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

Tags: URL rewrites for tag pages are overridden when query params change #452

Closed
mikemurray opened this issue Dec 7, 2018 · 1 comment
Closed
Assignees
Milestone

Comments

@mikemurray
Copy link
Member

Type: minor

Describe the bug
Using a proxy like nginx or skipper to do rewrites of URL's for the tag pages do not stick. They are overridden and revert back to the system defined route when query params change.

To Reproduce
Steps to reproduce the behavior:

  1. Setup the full Reaction Platform
  2. Add rewrite rewrite rule for from: /tags/example/product to: /tags/example-product
  3. Navigate to http://localhost:<proxy-port>/tags/example/product
  4. See that you do get the correct page
  5. After a second or two, see that the URL has changed to http://localhost:<proxy-port>/tags/example-product?limit=20

Expected behavior

URL should remain in the same style as the rewrite.

http://localhost:<proxy-port>/example/product?limit=20

@mikemurray mikemurray self-assigned this Dec 7, 2018
@machikoyasuda machikoyasuda added this to the 🏔 Shavano milestone Dec 10, 2018
@mikemurray
Copy link
Member Author

This issue can be resolved, but the commitment to disabling single page app (SPA) links must be fully realized.

At the moment, if you disable SPA links, the first request to a new tag or PDP page will force a window.location. Filtering lists or selecting variants on those pages would still trigger via pushState.

To be able to use rewrite rules to have arbitrary links to pages like these, then they cannot be allowed to modify the route via pushState.

They must also "know" the URL the proxy expects, e.g. /example/product instead of that the next.js app expects, which is /tag/example-product.

Otherwise, next.js or the underlying express server must be made aware of routes like /example/product so it can render the correct page or state.


What I did to get to this conclusion

  • set ENABLE_SPA_LINKS=false in .env
  • Modify router.js to indeed force every link to perform a window.location
  • Disable updates to the URL on componentDidMount of the tag page. This is bad and causes an infinite loop.
  • Disable updates to the UIStore for filtering so pages don't change, then change again when the browser reloads.
  • Set up a rewrite in Reaction /example/product => /tag/shirts
  • Navigate to a tag page
  • Hard code /example/product into Router.setSearch, so simulate "knowing" the rewrite URL.
  • Use page limit control
  • Success!! URL is now /example/product?limit=60

Anything short of that caused the URL to revert from /example/product to /tag/example-product?limit=60

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

No branches or pull requests

2 participants