Cloudflare v10 breaks cloudflare build / wrangler preview with React #211
Comments
dallyh
changed the title
alexanderniebuhr
added
pkg: cloudflare
- P4: important
|
Thanks for reporting the issue. It should work, this is a regression. I was able to reproduce it and am working on a fix :) |
|
Just checking in :) I was able to reproduce it, but still haven't found a fix, since it only happens for prerendered pages 🤔 |
|
As already mentioned in the Discord, this also applies to Solid & possibly other frameworks. @alexanderniebuhr Have you been able to find out more or get to the bottom of the problem? |
|
@deeprobin Can you link the related Discord message which shows that this also applies to Solid |
alexanderniebuhr
added
- P5: urgent
|
@alexanderniebuhr It will be this one https://discord.com/channels/595317990191398933/1223238165918384269, I was following this thread too. |
|
So I found a workaround, I know it's not create but if you have at least one page which is SSR using a React component with |
alexanderniebuhr
added
- P4: important
|
For me the issues with version 10 are currently too much hassle, so I'm personally staying on v9. Here is documentation for the older version, for anyone else still on that version. I believe that v10 will be great eventually, it really fits with the recent Cloudflare changes like that wrangler.toml can now be used for configuring a Pages project, however it uncovered some upstream issues, some of which are a little bit bigger and will probably take some time to properly fix. Still, thanks for your hard work guys! Really appreciate it. |
|
That's totally fair and also the reason why we are still maintaining v9 with security fixes and urgent bug fixes for some weeks. We hope that all the upstream issues are fixed by the time v9 maintenance ends. |
|
I was looking at the built chunks, and i may have found something. When the page is built without const noop = () => {};
export const R = noop;
export const r = noop;
export const t = noop;Thus next this variable When the page is built with The minification makes it hard, but I have found that the object is getting set in the code var react_production_min = {};
...
W={ReactCurrentDispatcher:U,ReactCurrentBatchConfig:V,ReactCurrentOwner:K}
...
react_production_min.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED=W;
...
{
react.exports = react_production_min;
}
var reactExports = react.exports;
...
export { React as R, reactExports as r, test as t };So basically where there is no on demand page with react and client directive, the imports leads to nowhere? |
|
@dallyh That is actually very helpful, thank you. We now have something to look into more detail. |
|
Adding this to the Astro config adds all of the React stuff into single outputted chunk file, then everything looks to work fine. So I'm just dropping this here as a better workaround than setting up an ondemand page with React and client directive :) vite: {
build: {
rollupOptions: {
output: {
manualChunks: (id) => {
//console.log(id);
if (id.includes("node_modules/react")) {
return "react-chunk";
}
},
},
},
},
}, |
|
@dallyh That was the missing piece! Thanks so much! ❤️ Please try |
|
@alexanderniebuhr Same as in #213 (comment) :( |
|
I've dug deeper and the issue with the build seems to lie here https://github.com/withastro/astro/blob/ecb44351e98cfba575447699689b0ace49cd3c9d/packages/astro/src/runtime/server/escape.ts and here with this file https://github.com/WebReflection/html-escaper/blob/master/cjs/index.js in dependency Changing the manual chunks to this fixes the build, and React seems to work fine. manualChunks: (id) => {
if (id.includes("node_modules") && !id.includes("node_modules/astro") && !id.includes("node_modules/html-escaper")) return "vendor";
}, |
|
Ok this is very strange. I have tested the build locally with your reproduction example and it worked without issues, I'll update the branch, but I'm afraid that depending on what users use in there project, that there might be more cases like this in the future. Anyways it's super hard to reproduce for me. |
|
It could potentially be differences between operating systems, or may be that the edit: As you said that there will be more cases, i¨ve tried the recent version in my project and got: With integration sanity-astro :( Maybe a better way of handling this would be to pick some of the packages like React, and create chunks for them, instead of picking everything in the node_modules directory? |
|
Can you show me the line of
Then it's just the other way around, we would start with React, Solid, e.g. I'm trying to think about a solution, and hopefully we can cut a release early next week, with a version which does work good enough for now. |
|
@dallyh sorry for all this back and forth, but can you try this version with your real project?
|
|
You could ping me at discord, @dally.h for some testing, if required. However, I tried it on the example React repo which works fine, then I tried it on #213 this issue/repo and my other projects, and it did not work because of the same errors with Shard and its dependencies. But again for whatever reason, adding "sharp" to manual chunks seems to do the trick. adapter: cloudflare({
imageService: "compile",
platformProxy: {
enabled: true,
},
experimental: {
manualChunks: ["sharp"]
}
}),With this tho the "sharp" chunk is created but not referenced anywhere, so then Wrangler just probably does not pick it up. So looks like we solved this one, but now #220 arises as all the chunks are still included, but just not used server side. |
|
Ok so maybe adding "sharp" by default makes sense. For #220, there is already a fix, but we need to wait for an upstream fix, otherwise we can't merge the adapter fix, but I don't know when that will land, or we need to find a workaround for the time being. Let me think about that 🤔 |
|
I am getting this error when trying with a React Component. |
Fwiw, I was having a very similar issue when trying to pre-render pages and was able to workaround the problem by trying the I've been around the houses on this as, on the latest version of the Cloudflare adapter (@10.2.4), I was actually hit with a different issue which precluded me from seeing the In version 10.2.4, if I try and pre-render a page, the built output in Downgrading to 10.0.0 was what caused me to bump into this issue and, with the test package, I've been able to move forward. |
|
Yeah this import issue with prerendering has it's origins in Astro core 😢 |
Astro Info
Describe the Bug
When using v10 version of the adapter, hybrid output mode and React on a prerendered path with
client:loaddirective, preview with wrangler fails. This was not the case with previous version of the adapter. I haven't tried building the site on Cloudflare, but my guess is if the wrangler fails, then the build will fail too.Steps to reproduce
What's the expected result?
React should be usable on a prerendered path as it was like this before. If this is not the case, it should be mentioned in the docs that React should be used only with SSR enabled endpoint/path.
Link to Minimal Reproducible Example
https://github.com/dallyh/astro-cf-10-issues/tree/react-issue
Participation
The text was updated successfully, but these errors were encountered: