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

Loader error on data request proxying raw fetch response #9691

Closed
nareshbhatia opened this issue Jul 2, 2024 · 7 comments · Fixed by #9693
Closed

Loader error on data request proxying raw fetch response #9691

nareshbhatia opened this issue Jul 2, 2024 · 7 comments · Fixed by #9693

Comments

@nareshbhatia
Copy link
Contributor

nareshbhatia commented Jul 2, 2024

Reproduction

https://github.com/nareshbhatia/remix-nested-layouts

npm ci
npm run dev
  1. Point your browser to http://localhost:3000/
  2. Click on the Movies tab. App navigates to http://localhost:3000/movies. However it crashes with error: TypeError: immutable.
  3. Refresh the browser without changing the URL. Page refreshes correctly showing a list of movies.

This indicates that the loader is reading the API_URL environment variable correctly for full page render, however when navigating from page to page, it can't read the environment variable.

This problem does not exist in production mode. See prod deployment at https://remix-nested-layouts-coral.vercel.app/

System Info

System:
    OS: macOS 13.6.7
    CPU: (4) x64 Intel(R) Core(TM) i5-7600K CPU @ 3.80GHz
    Memory: 55.42 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.10.0 - ~/.nvm/versions/node/v20.10.0/bin/node
    Yarn: 1.22.19 - ~/.yarn/bin/yarn
    npm: 10.2.3 - ~/.nvm/versions/node/v20.10.0/bin/npm
    Watchman: 2022.11.14.00 - /usr/local/bin/watchman
  Browsers:
    Chrome: 126.0.6478.127
    Safari: 17.5
  npmPackages:
    @remix-run/dev: ^2.10.0 => 2.10.0
    @remix-run/node: ^2.10.0 => 2.10.0
    @remix-run/react: ^2.10.0 => 2.10.0
    @remix-run/serve: ^2.10.0 => 2.10.0
    vite: ^5.3.2 => 5.3.2

Used Package Manager

npm

Expected Behavior

I should be able to navigate to any page, or click on various movies in the movie list and the app should not crash (in dev mode). The basic issue is that environment variables are not being read when navigating from page to page.

Actual Behavior

App crashes when navigating from page to page.

@brophdawg11
Copy link
Contributor

This isn't actually related to the ENV variable but looks like a stricter implementation of immutable fetch Response headers when using the built-native fetch implementation. Remix appends a custom header to ?_data requests and undici is throwing an error here because the headers on the fetch response being returned are considered immutable.

It looks like it's limited to directly returned fetch responses since we have lots of tests for returning json/new Response instances that don't run into this immutable issue. We can probably fix via response.clone() but I'm not sure we want to do that all the time so let me dig into this a bit.

For now, the best workaround is probably to read the response and return your own json response:

const res = await fetch(...);
const data = await res.json();
return json(data);

@nareshbhatia
Copy link
Contributor Author

nareshbhatia commented Jul 2, 2024

Thanks for digging in, @brophdawg11. I was intentionally returning the Response object because that was a strong recommendation in the docs. I will use the workaround in the meanwhile.

@brophdawg11
Copy link
Contributor

Yeah for sure - that will still be the recommendation in this type of BFF setup. It looks like undici is just a bit more strict than our old @remix-run/web-fetch implementation.

@brophdawg11 brophdawg11 linked a pull request Jul 2, 2024 that will close this issue
@brophdawg11 brophdawg11 changed the title Environment variables not working in dev mode when navigating to a different route Loader error on data request proxying raw fetch response Jul 2, 2024
@brophdawg11
Copy link
Contributor

Resolved by #9693 and will be available in the next release

@brophdawg11 brophdawg11 added the awaiting release This issue has been fixed and will be released soon label Jul 3, 2024
Copy link
Contributor

github-actions bot commented Jul 4, 2024

🤖 Hello there,

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

Copy link
Contributor

github-actions bot commented Jul 4, 2024

🤖 Hello there,

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

@nareshbhatia
Copy link
Contributor Author

Works like a charm! Thank you for a quick turnaround on this 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants