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

fetcher.load is causing the whole page to reload #1220

Closed
12 tasks
ryanflorence opened this issue Dec 23, 2021 · 4 comments
Closed
12 tasks

fetcher.load is causing the whole page to reload #1220

ryanflorence opened this issue Dec 23, 2021 · 4 comments
Labels
bug Something isn't working

Comments

@ryanflorence
Copy link
Member

ryanflorence commented Dec 23, 2021

Which Remix packages are impacted?

  • remix (Remix core)
  • create-remix
  • @remix-run/architect
  • @remix-run/cloudflare-workers
  • @remix-run/dev
  • @remix-run/express
  • @remix-run/netlify
  • @remix-run/node
  • @remix-run/react
  • @remix-run/serve
  • @remix-run/server-runtime
  • @remix-run/vercel

What version of Remix are you using?

1.1.1 (1.0.6 also)

What version of Node are you using? Minimum supported version is 14.

16

Steps to Reproduce

  let movies = useFetcher();
  useEffect(() => {
    movies.load(`/api/movies?q=Lord+of+the+rings`);
  }, []);

Expected Behavior

The document shouldn't reload

Actual Behavior

The document is reloading!

Whether I call load from a combobox input or simply on mount in a useEffect, the whole document reloads.

I've added a default export to the route, and also removed it, thinking maybe it had to do with our resource routes, but in both cases it reloads.

It might have something to do with our auto window.reload if an asset isn't found and maybe my build hashes got out of whack? Not sure yet.

image

@ryanflorence ryanflorence added the bug Something isn't working label Dec 23, 2021
@ryanflorence
Copy link
Member Author

Oh yep, I'm getting into this code:

} catch (error) {
// User got caught in the middle of a deploy and the CDN no longer has the
// asset we're trying to import! Reload from the server and the user
// (should) get the new manifest--unless the developer purged the static
// assets, the manifest path, but not the documents 😬
window.location.reload();
return new Promise(() => {
// check out of this hook cause the DJs never gonna re[s]olve this
});

Not sure why yet.

@kentcdodds
Copy link
Member

Would this cause useFetcher to follow redirects? (Which I didn't think they were supposed to do): #1204

@ryanflorence
Copy link
Member Author

Found it. It's because I had server code in my movies route so the try/catch around loading the module reloaded the page. Once I got that out it worked fine.

Our work to automatically mark our route entries as side-effect free (or the better error message) that we're already working on will make this a non-issue.

Anyway, solution is like any other time you get server code in browser bundles, move the imports into a separate file (and if you feel like, it, tack on *.server.js to the file.

@kentcdodds useFetcher().load should not follow redirects in the fetch, but instead redirect the app (especially for unauthenticated calls), but a raw fetch to a resource route will follow the redirect for the fetch but not redirect the app.

@nickjs
Copy link

nickjs commented Dec 23, 2021

@ryanflorence Thanks for the follow up and clarification, glad you found your issue 😄. Just to make sure I'm understanding correctly, useFetcher() should navigate the app if a loader or action explicitly return redirect()s?

If that's correct, then I believe #1204 is still a valid RFC to improve UX for users without javascript and DX for developers who want to use fetchers.

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
Development

No branches or pull requests

3 participants