remix-run / remix

Build Better Websites. Create modern, resilient user experiences with web fundamentals.
https://remix.run
MIT License
29.79k stars 2.51k forks source link

Trailing slash causes redirect loop #7529

Open joelvh opened 1 year ago

joelvh commented 1 year ago

What version of Remix are you using?

2.0.0

Are all your remix dependencies & dev-dependencies using the same version?

Steps to Reproduce

  1. Create a page, ideally a nested path such as app/routes/folder.file._index.tsx (URL path /folder/file)
  2. Set up a reverse proxy (e.g. Shopify App Proxy) pointing to that path (e.g. https://www.example.com/folder/file)
  3. Update remix.config.js to set publicPath = "/folder/file/build/ so assets link through the reverse proxy
  4. Visit the proxy path (e.g. https://example.myshopify.com/folder/file) which reverse proxies to www.example.com
  5. Observe: browser path does not have a trailing slash, but the request from the reverse proxy to the Remix app has a trailing slash (e.g https://www.example.com/folder/file/)
  6. Observe: browser reloads infinitely with error message in browser console that says Initial URL (/folder/file/) does not match URL at time of hydration (/folder/file), reloading page...

Expected Behavior

Expect the refresh behavior to realize that a trailing slash should be ignored.

Temporary fix is to place the following in entry.client.tsx:

  const initialPathname = window.__remixContext.url
  const hydratedPathname = window.location.pathname

  /**
   * Fix initial pathname to match browser if we're dealing with trailing slashes.
   */
  if (
    initialPathname !== hydratedPathname &&
    initialPathname.replace(/\/+$/, '') === hydratedPathname.replace(/\/+$/, '')
  ) {
    window.__remixContext.url = hydratedPathname
  }

Actual Behavior

Refresh doesn't realize the two paths are effectively the same.

UPDATE: It turns out this is also an issue with links and causes the inverse issue. The Link will have a path such as /folder/file and the path in the browser becomes /folder/file/, which I believe is updated by the router. I've updated the script that fixes this (above). I've also updated the steps to reproduce to call out that we are using a custom publicPath.

brophdawg11 commented 3 months ago

This should be resolved via #9695 and available in the next release 👍

github-actions[bot] commented 2 months ago

🤖 Hello there,

We just published version 2.11.0-pre.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

github-actions[bot] commented 2 months ago

🤖 Hello there,

We just published version 2.11.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

brophdawg11 commented 2 months ago

We had to revert the fix in #9695 so re-opening this. We should be able to remove this check entirely instead in #9890