Skip to content

fix: follow server redirects in client navigation #605

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

alecmarcus
Copy link

@alecmarcus alecmarcus commented Jul 17, 2025

Closes #601

  • Client navigation is now expected to be initialized with initClient({ spaMode: true })
  • This flag is used to alter the behavior of fetch transport to follow redirects in the response recursively
  • For backwards compatibility, adds a check to avoid initializing client navigation more than once
  • Update the type of onNavigate to be a promise

I dug into this without seeing #603 😅. But if I understand correctly, my approach is different and as a result, client side rendering remains possible across same origin redirects.

I also figured that given cross-origin server redirects are acceptable with server navigation, it would make sense to gracefully fall back to server redirects when we encounter them. This is now done by catching the CORS error and falling back to browser navigation.

Unfortunately there's no way to see the location of a cross origin redirect in a fetch response, so the check is for a generic TypeError: Failed to fetch. However the fallback behavior is simply to reload the window at the originally requested url on the site's origin, so I don't see this being an issue.

This is working nicely from my perspective as a dev user. But if this doesn't make sense as a contribution I'm happy to make changes.

CleanShot.2025-07-16.at.21.17.24.mp4

@justinvdm
Copy link
Collaborator

Thank you for this @alecmarcus!

We've released the fix to #601 here: https://github.com/redwoodjs/sdk/releases/tag/v0.1.24

Now to figure out where to go for this PR here. It looks like we took quite different directions, and so my take is: lets figure out if there's anything here that we haven't covered in #603, so we can figure out what to do about it.

Along those lines, there was mention of cross origin redirects. @peterp Do you think this is handled already with #603 now landed?

Anything else covered here and not in #603?

@alecmarcus
Copy link
Author

alecmarcus commented Jul 23, 2025

My pleasure!

As I understand it, the main and only difference is that this approach gives continuous client side rendering for all same-origin requests. You can see in the video that instead of falling back to a full reload via browser navigation, we're able to maintain the diff-and-rerender SPA behavior for any number of redirects the server might respond with, whereas I believe #603 immediately falls back to a full page reload at the next location when the server returns a redirect.

That being the case, #603 should handle cross origin request just fine/the same way this change does

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

Successfully merging this pull request may close these issues.

Server redirects break client navigation
2 participants