Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Surface thrown response headers to ancestor boundaries #6425

Merged
merged 5 commits into from
May 19, 2023

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented May 18, 2023

We have a gap in the API currently where you cannot access headers from child-route-thrown-responses because we only run headers() from "renderable" matches (which means root down to the boundary). So if you throw new Response() from /parent/child and catch it in /parent - you have no way to send any child headers back from the /parent headers() function.

This PR adds a new errorHeaders parameter to headers() that holds the headers from any thrown Response triggering the error boundary.

If the route that throws the response is also the boundary route, then errorHeaders will be the same as what's already passed as loaderHeaders/actionHeaders.

We also only expose the errorHeaders to the leaf renderable match headers() function to avoid duplicating the header value through parentHeaders.

Closes #5356

@changeset-bot
Copy link

changeset-bot bot commented May 18, 2023

🦋 Changeset detected

Latest commit: a2725e3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 18 packages
Name Type
@remix-run/server-runtime Minor
@remix-run/cloudflare Minor
@remix-run/deno Minor
@remix-run/dev Minor
@remix-run/node Minor
@remix-run/react Minor
@remix-run/cloudflare-pages Minor
@remix-run/cloudflare-workers Minor
create-remix Minor
@remix-run/css-bundle Minor
@remix-run/architect Minor
@remix-run/express Minor
@remix-run/netlify Minor
@remix-run/serve Minor
@remix-run/testing Minor
@remix-run/vercel Minor
remix Minor
@remix-run/eslint-config Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@markdalgleish
Copy link
Member

This API makes sense to me 👍

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-3e093e5-20230520 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants