Remove the cache property from calls to new Request(...) in React
#771
Conversation
🦋 Changeset detectedLatest commit: 1f5539f The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
🧪 Prereleases are available for testing 🧪 @cloudflare/next-on-pagesYou can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/9067847845/npm-package-next-on-pages-771@cloudflare/eslint-plugin-next-on-pagesYou can install this latest build in your project with: npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/next-on-pages/runs/9067847845/npm-package-eslint-plugin-next-on-pages-771 |
|
Thanks a lot for the PR @james-elicx 😄 But it seems like the e2es are failing, and the failures do seem legitimate:
Could you please have a look? 🙏 |
|
argh, i was hoping a rerun might fix them. welp. that's irritating |
yeah no, I already did try rerunning them 🥲 Also usually they kind of either all run or all (or most) fail, when just a few tests fail like now I think that does indicate a real regression/issue 😅 |
james-elicx
force-pushed
the
james/fix-cache-request-init
branch
from
5850426
to
ec5e9ae
Compare
james-elicx
force-pushed
the
james/fix-cache-request-init
branch
2 times, most recently
from
2e5bcb1
to
d81eaff
Compare
james-elicx
force-pushed
the
james/fix-cache-request-init
branch
from
d81eaff
to
63cb2fe
Compare
james-elicx
force-pushed
the
james/fix-cache-request-init
branch
from
e6faca1
to
1f5539f
Compare
|
What's the status of this PR? I'm having this issue with Cloudflare pages and next-firebase-auth-edge library. |
i think it's probably fine to merge personally. the area of caution is we haven't done any tests about if this would cause a regression for how the fetch cache in nextjs works. i feel like it would probably be okay though. |
In the React server code, there is a ternary where they create a new Request object and forward all properties on
RequestInitto it. This won't work in workerd due to thecacheproperty being present. Therefore, we need to overwrite their ternary with a way for us to strip thecacheproperty from theRequestInitobject.https://github.com/vercel/next.js/blob/9ec37c12/packages/next/src/compiled/react/cjs/react.react-server.production.js#L87
Additionally, once this patch has been added, it looks like the error can then happen in our patched fetch. Therefore, also adding similar logic there to strip out the
cacheproperty from theRequestInitobject.fixes #719