-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fixes an issue where view transitions to the 404-page did not work #9642
Conversation
🦋 Changeset detectedLatest commit: 931b7bd The changes in this PR will be included in the next version bump. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me! Thanks for adding the simple to understand test as well.
Co-authored-by: Florian Lefebvre <[email protected]>
Co-authored-by: Arsh <[email protected]>
response = new Response(response.body, { ...response, status }); | ||
response = new Response(response.body, { | ||
status: status, | ||
headers: response.headers | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can confirm that ...response
seems to do nothing here 👍 We could also copy over statusText
, but not a big deal I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was it originally. I advised against it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, looks like you're already one step ahead 😄
Changes
Changed the cloning of a Response in route.ts to retain the original headers.
With the headers the content type of the 404 page was lost.
The view transition loader expected text/html and did a full reload due to the missing header.
Closes #9629
Testing
new e2e test to confirm that we can transition to 404 / undefined pages without reloads
Docs
n.a. / bug fix