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

CDN Causing infinite reload #6703

Closed
1 task done
yarbsemaj opened this issue Jun 27, 2023 · 12 comments · Fixed by #6707
Closed
1 task done

CDN Causing infinite reload #6703

yarbsemaj opened this issue Jun 27, 2023 · 12 comments · Fixed by #6707
Labels
bug Something isn't working

Comments

@yarbsemaj
Copy link

yarbsemaj commented Jun 27, 2023

What version of Remix are you using?

1.18.0

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

  • Yes

Steps to Reproduce

  1. Place your site behind a CDN, and don't include query params in the cache key, but forward them to the origin
  2. Visit a page with a cache header with query param, then again without
  3. Page gets stuck in a infinite reload loop

Issue caused by this PR
#6409

Expected Behavior

Page should not get stuck in a loop and should load normally

Actual Behavior

Page gets stuck in an infinite reload loop as the url on the cache page does not match the URL of the page you are loading

@n8agrin
Copy link
Contributor

n8agrin commented Jun 27, 2023

@brophdawg11 I think we should drop the search comparison part of #6409.

@yarbsemaj that would fix your issue, right?

I can open a pr today

@yarbsemaj
Copy link
Author

It should do, yes

n8agrin added a commit to n8agrin/remix that referenced this issue Jun 27, 2023
…wnloading asset bundles

Resolves remix-run#6703

Previous implementation used the pathname and search parts of a url to determine if route changed, which causes some conflict when CDNs / cache proxies are placed between the Remix app and the client. This change allows remix to reload the page if only the path changes. There may be some issues with how loaders are handled when configured behind a CDN, since Remix expects to trigger loaders when the search part of a url changes, but the total impact here is dependent on how a developer is configuring the network topology on top of a Remix server.
@n8agrin
Copy link
Contributor

n8agrin commented Jun 27, 2023

@yarbsemaj see comment here: #6707 (comment)

@tatemz
Copy link

tatemz commented Jun 29, 2023

I also have seen infinite reloads when upgrading to 1.18, but I have not yet been able to confirm why. I see infinite reloading when running locally and not behind a CDN. As a result, my current project will remain on lower versions until I can figure out why 1.18 caused this issue

@brophdawg11
Copy link
Contributor

I think #6707 is the correct fix here for CDN's ignoring search params so we're going to look into a patch release with that in it.

@tatemz Please reply if you can figure out your issue. Turn on Preserve log in the Look in the Chrome dev tools console settings and look for this type of message indicating Remix is reloading due to a mismatch:

Screenshot 2023-06-29 at 11 14 35 AM



Another root cause we identified was mismatched versions of @remix-run/server-runtime and @remix-run/react so ensure that those are the same version and you don't have duplicates (via npm ls [package])

@github-actions
Copy link
Contributor

🤖 Hello there,

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

@brophdawg11 brophdawg11 added bug Something isn't working awaiting release This issue has been fixed and will be released soon and removed bug:unverified labels Jun 29, 2023
@brophdawg11
Copy link
Contributor

This should be available in 1.18.1 once released, but can be tested in 1.18.1-pre.0 currently.

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-0657c16-20230630 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!

@tatemz
Copy link

tatemz commented Jun 30, 2023

I think #6707 is the correct fix here for CDN's ignoring search params so we're going to look into a patch release with that in it.

@tatemz Please reply if you can figure out your issue. Turn on Preserve log in the Look in the Chrome dev tools console settings and look for this type of message indicating Remix is reloading due to a mismatch:
Screenshot 2023-06-29 at 11 14 35 AM

Another root cause we identified was mismatched versions of @remix-run/server-runtime and @remix-run/react so ensure that those are the same version and you don't have duplicates (via npm ls [package])

Thanks @brophdawg11. My project is definitely showing this error in the console. Thanks for the lead. I am using Shopify Hydrogen and will have to track down the deps resolution through their repo

@brophdawg11
Copy link
Contributor

@tatemz yeah we chatted with the Hydrogen team yesterday about that. The issue boils down to hydrogen having an internal dependency on 1.17.1 but if you then update to 1.18.0 in your package.json, you can get a mismatch which causes this issue. For now just make sure you've installed the same remix versions as hydrogen is using internally. And then bump to 1.18.0 when they do. We discussed plans to avoid this type of mismatch in the future as well.

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.18.1-pre.2 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
Copy link
Contributor

🤖 Hello there,

We just published version 1.18.1 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 brophdawg11 removed the awaiting release This issue has been fixed and will be released soon label Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants