Fix storefrontRedirect on soft navigations
#1880
Conversation
blittle
force-pushed
the
bl-fix-redirect
branch
from
cc8ebc5
to
7f7c628
Compare
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
Also change redirect status code from 301 to 302
blittle
force-pushed
the
bl-fix-redirect
branch
from
7f7c628
to
6733dad
Compare
| const redirectFrom = | ||
| pathname + (filteredSearchParams ? '?' + isSoftNavigation : ''); |
There was a problem hiding this comment.
I think this is wrong? Isn't redirectFrom getting ?true here?
Maybe you meant pathname + (isSoftNavigation ? '?' + filteredSearchParams : '') instead?
There was a problem hiding this comment.
Also, maybe we could simplify this to:
const url = new URL(request.url);
const isSoftNavigation = url.searchParams.has('_data');
url.searchParams.delete('_data');
const redirectFrom = url.toString().replace(url.origin, '');There was a problem hiding this comment.
Hmmm, this is odd that it worked. I'm going to add to the unit tests and figure out why the API request didn't fail.
There was a problem hiding this comment.
It didn't fail my unit test because the test didn't have an extra query parameter. I added another test to verify that scenario.
| '@shopify/hydrogen': patch | ||
| --- | ||
|
|
||
| Fix bug where `storefrontRedirect` would return an error on soft page navigations. Also change the redirect status code from 301 to 302. |
There was a problem hiding this comment.
Thinking about it, should we consider 301 => 302 a breaking change? 🤔 Maybe not...
There was a problem hiding this comment.
I argue it was a bug in the first place 🙈
* Fix `storefrontRedirect` on soft navigations Also change redirect status code from 301 to 302 * Add test and PR feedback
WHY are these changes introduced?
Fix
storefrontRedirecton soft navigations (click a link in the app to transition to a route that has a redirect). Also change the redirect status code from 301 to 302. 302 is not permanent and safer.WHAT is this pull request doing?
_datasearch parameter when querying if a redirect exists.HOW to test your changes?
<Link to="/doesNotExist?return_to=%2Fcollections">Redirect</Link>/collections.Post-merge steps
Checklist