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

Add support for replace() redirects #11811

Merged
merged 6 commits into from
Jul 18, 2024
Merged

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Jul 16, 2024

Builds off of #11100 and adds the new replace() redirect API

Closes #10606

Copy link

changeset-bot bot commented Jul 16, 2024

🦋 Changeset detected

Latest commit: cbf49e8

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

This PR includes changesets to release 5 packages
Name Type
@remix-run/router Minor
react-router Minor
react-router-dom Minor
react-router-dom-v5-compat Minor
react-router-native 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

Comment on lines +1628 to +1632
export const replace: RedirectFunction = (url, init) => {
let response = redirect(url, init);
response.headers.set("X-Remix-Replace", "true");
return response;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to redirectDocument - this is just a wrapper with a custom internal header

@brophdawg11 brophdawg11 merged commit 01d0f41 into v6 Jul 18, 2024
3 checks passed
@brophdawg11 brophdawg11 deleted the brophdawg11/redirect-replace branch July 18, 2024 12:47
Copy link
Contributor

🤖 Hello there,

We just published version 6.26.0-pre.0 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!

Copy link
Contributor

github-actions bot commented Aug 1, 2024

🤖 Hello there,

We just published version 6.26.0 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!

@DreierF
Copy link

DreierF commented Aug 2, 2024

I gave it a try, but it does not quite behave like I would have expected. Not sure though if this is a misunderstanding on my side or a bug.

Minimal reproduction: https://stackblitz.com/edit/github-efe1i6?file=src%2Fapp.tsx

Steps to reproduce:

  • Open the preview in a new tab
  • Click on "Go to 1"
  • Click on "Go to 2"
  • Observe that the filter search param is added as expected
  • Then navigate back in the browser

Expected
I get back to the previous page that shows "Go to 2"

Actual
I get back to the initial "Go to 1" because the replace replaces the history entry I'm coming from instead of the entry that I'm navigating to/loading data for.

@brophdawg11 Could you take a look whether this is expected?
I can also file a bug ticket if this is not expected.

@brophdawg11
Copy link
Contributor Author

This is functioning as expected because loaders run before the history stack is updated. So the loader for /2 is running while history is on /1 and thus by using replace you are replacing the /1 entry in the history stack. This is done intentionally so that if you are redirect-ing you don't see the URL flickering between intermediate states and just goes from the original URL to the final redirected URL.

If you want the history stack to be / -> /1 -> /2?filter=abc then you don't need to use replace and you can just use a normal redirect and then the back button will go to /1.

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.

3 participants