Potential upstream problem in solid for users of Astro + solid + cloudflare?
Summary: The lastest versions of Astro, their cloudflare adapters, and solid-js are breaking buildings during hydration, and then when a component uses onMount. Here are the referenced issues. https://github.com/withastro/adapters/issues/224 and then https://github.com/withastro/adapters/issues/263. Both have minimal reproductions attached.
I'm not sure who might be helpful to ping on any of this, and I tried to look around for any related issues in this discord, but wasn't sure. Can someone from the solid team perhaps weigh in on these issues? Specifically, you'll see https://github.com/withastro/adapters/issues/263#issuecomment-2117106206, with suggests this might be the upstream issue?
I'm not sure who might be helpful to ping on any of this, and I tried to look around for any related issues in this discord, but wasn't sure. Can someone from the solid team perhaps weigh in on these issues? Specifically, you'll see https://github.com/withastro/adapters/issues/263#issuecomment-2117106206, with suggests this might be the upstream issue?
GitHub
Cloudflare v10 + Solid breaks some components and scripts · Issue #...
Astro Info Astro v4.5.16 Node v20.12.0 System Linux (x64) Package Manager bun Output server Adapter @astrojs/cloudflare Integrations unocss @astrojs/solid-js @swup/astro @vite-pwa/astro-integration...
GitHub
Issues · withastro/adapters
Home for Astro's core maintained adapters. Contribute to withastro/adapters development by creating an account on GitHub.
GitHub
Cloudflare + Solid-js hydration / build errors. · Issue #263 · wi...
Astro Info Astro v4.8.4 Node v21.5.0 System macOS (arm64) Package Manager pnpm Output server Adapter @astrojs/cloudflare Integrations @astrojs/solid-js If this issue only occurs in one browser, whi...
42 Replies
@core-team - if you have a second, any thoughts on this perhaps or if it'd help to move the conversation elsewhere?
I see.. this is actually the most relevant thread: https://github.com/solidjs/solid-start/issues/263
GitHub
Cloudflare adapter disables known cloudflare worker "compatible" de...
Summary The adapter solid-start-cloudflare-workers pulls in "node" exports of dependencies by default. This makes sense for solid-js framework and possibly precompiled Solid Components in...
It's annoying because while we could fix it.. I'm almost positive it will break Jest.. Most people involved in the conversation agreed the behavior was unexpected but no one was interested in fixing it.
hmmm. Breaking jest does sound significant. But, wouldnt this also keep solidstart from being able to deploy to cloudflare and other edge runtimes too? I'm out of league with publishing packages, but what would be needed to move the ball forward on it or would be ideal?
I'm not sure.. there are a bunch of open issues from when this conversation started. It hadn't impacted us much until now that I guess they are trying to remove compatibility.
I think the shortest path is see if newer Jest works without the browser field and see if we can remove it.
I think the hardest part of this is that we can't make such a move on a patch release. We'd have to do a minor most likely otherwise risking back compat.
Well.. I can't speak for the astro core team, but I know that a previous verison of their adapter still works on cloudflare, but I don't think it's like immediate obselence for anyone still on the old version of their cloudflare adapter. I'm not sure if there's anything I can do (since it seems like a pretty large and complicated issue), but I'd be happy to run any of the stuff on my end that broke to see if something resolves
@ryansolid not sure if you remind, but we met in Amsterdam Devworld. I just came here too. I'm maintaining the astro cloudflare adapter. We did had an additional esbuild bundling step in the past, which we removed, that's why we discover a lot of different limiations, which were hidden to us before. We think the most correct approach is to fix it in Solid's vite plugin, but happy to discuss other solutions if you have any idea
I also came here for the same reason. I would really appreciate it if someone from the @core-team can help us move this forward.
Hey just got back from vacation. It's tricky as I said because in a sense we rely on the behavior being the way it is for Jest to work. And when we suggested that the way many tools were handling things was odd and incorrect people tended to agree but pointed at another tool that had this behavior and basically wouldn't change it. I'm curious @alex (he/him) if there is something to be done at the vite-plugin level but my understanding was this had to do with browser field resolution of the
solid-js package itself.That's a good question, I'm still trying to debug if it is only the resolution issue or something else. In general we see two types of errors:
First:
✘ [ERROR] Uncaught (in response) TypeError: Cannot set properties of undefined (setting 'noHydrate')Second:
onClick functionality is not working, but in my case moving from <>...</> to <div>...</div> solved the issue.@ryansolid I wonder if the
vite-plugin-solid's vite config (https://github.com/solidjs/vite-plugin-solid/blob/main/src/index.ts#L238-L253), could be adapted to make it work 🤔 (maybe adding conditions or aliases as outlined in https://github.com/solidjs/vite-plugin-solid/issues/153)The first error suggests there is a place where sharedConfig.context isn't defined. I did see something in Astro/Solid Adapter that surprised me where it was using the
<NoHydrate> wrapper. I'm gathering to reduce output size but I think there are somebugs with it and Suspense. We haven't used it ourselves for that and to be fair.. Astro didn't originally support async Solid rendering on the server so Suspense wasn't an issue. So I think that looks like just a Solid bug/adapter bug that should be addressable.
The second issue does also seem like a top level fragment issue. I've been chasing these down for a while. It is probably something odd in the Solid/Astro adapter code that we don't hit typically in apps.
My understanding of the issues were things that were specifically related to cloudflare. If those are, I'm a bit surprised, but otherwise those just seem like specific code bugs and not bundling issues.Yeah those can be code bugs, we had a dedicated esbuild bundle step in an earlier version, which we now removed thanks to updates to the platform. So we now use output from
astro build (vite output) for cloudflare. I haven't seen any reports for other adapters, but that doesn't mean they don't exist there.
One difference between cloudflare adapter and other adapters is that we do need to set vite.ssr.target = 'webworker' & vite.ssr.noExternal = true, which leads to a different output of the build step. Maybe we also need to check if @astrojs/solid (https://github.com/withastro/astro/tree/main/packages/integrations/solid) needs an change or update, but I'm not familar with that integration. We do have an pending core update, which is supposed to fix chunking issues, so I hope it will fix all those solid issues, but I'm not 100% sure yet. It's hard for me to debug, if you can tell me what you would need to debug this on solids side, I can try to get those infos & reproductions from users.My understanding is that the browser version of Solid is getting pulled in on the server because while the
webworker export condition is properly handled for some reason unknown to me they then run it through the toplevel browser mapping. Which remaps it back to the browser build. It feels like a bug in all the modern tool stack but no one seems interested in fixing it.
I would expect the browser mapping either to happen first or not happen when export conditions are involved. But to have both run and the browser mapping to run afterwards makes zero sense. Like we need the browser mapping or old tools like Jest which didn't support export conditions. That was the only way to get the browser version in node. But to have it also apply on top of things that get mapped to the server via export conditions is just nonsensical.Okay I took another look, the only error left is this one:
✘ [ERROR] Uncaught (in response) TypeError: Cannot set properties of undefined (setting 'noHydrate')I'm not sure if that is a bundler/bundling issue anymore, or if that is something else..?
I just wanted to thank you both, @alex (he/him) and @ryansolid, for all your efforts!
That is an odd one. it generally means server code is running at a point where there is no server context which shouldn't be possible. Like something outside of a render or not sure.
Yeah I haven't figured it out, still trying to debug this. 🤔
@alex (he/him) Any progress on this?
Not from our side, I still don’t think we can fix it without changes in solids packages
I haven't had a chance to verify that legacy packages like Jest can still work without using the browser field.
@ryansolid -> Does this matter in terms of jest support?
https://jestjs.io/blog/2022/04/25/jest-28#packagejson-exports
I tried to read through the linked issues, but not sure I just quite grasp if this is relevant.
I tried to set up a small Jest + Solid repo and run some tests with pathching solid to remove the browser export altogether, but was getting some error about solid-js/web not providing an needed export.
I'll link it here in case it is a starting point (but idk if its helpful or not). https://github.com/solidjs/solid/releases
I'll link it here in case it is a starting point (but idk if its helpful or not). https://github.com/solidjs/solid/releases
GitHub
Releases · solidjs/solid
A declarative, efficient, and flexible JavaScript library for building user interfaces. - solidjs/solid
Jest 28: Shedding weight and improving compatibility 🫶 · Jest
Jest 28 is finally here, and it comes with some long requested features such as support for sharding a test run across multiple machines, package exports and the ability to customize the behavior of fake timers. These are just some personal highlights, and we'll be highlighting more in this blog post.
This is good..What we are looking for. However the error you saw sounds exactly like what would happen if it wasn't working. If it can't resolve to the client version you'd get a complaint about solid-js/web missing a needed export.
It might just be a more minor config thing though
Yeah, I figured it was a "bundler can get the right thing" type of error, but I also figured there are plenty of very unfamiliar config things to me (I'm not a jest user, and normally still work at the level that a lot of the tooling is wired together (i.e astro adapters, vite plugins) etc;). But I thought that Jest release sounded like it was a fix for this, and so yeah, maybe there's a flag somehwere or other way to indicate which export fields to point at for Jest. Idk if there's any other common legacy tools (i.e. don't support exports fileds) supported that previously didn't support package.json exports, but I'm willing to at least poke around and see what might be doable if there are other things that could use investigating
@ryansolid -> Sorry one more ping here. I forgot to link the repo, but I played with it a bit more, and I think it works. (I.e. have a solidjs component test passing after pnpm patching solid and deleting the browser field) I'd love for you to check it out and see if this has now resolved upstream, and if there's anyone else that might also be an upstream blocker.
https://github.com/wkelly17/Solid-Jest-repro
I noticed it should be configurable with Jest here:
https://jestjs.io/docs/29.4/configuration#testenvironmentoptions-object
GitHub
GitHub - wkelly17/Solid-Jest-repro
Contribute to wkelly17/Solid-Jest-repro development by creating an account on GitHub.
Configuring Jest · Jest
The Jest philosophy is to work great by default, but sometimes you just need more configuration power.
Thanks this will be helpful
@ryansolid -> Not wanting to be pushy, but is there anything else I can help out with on thiss or an idea of when you might could look at it? I'm just trying keeping tabs on it to keep it from falling forgotten. I know there's a number of folks who do use the the cloudflare + solid + astro combo.
It requires a minor release I believe otherwise I could "break" existing projects so that will take a bit of coordination.
I'm getting this weird case, the code inside onMount is executing (twice) in the server, and this is happening only in Cloudflare adapter and in production (it doesn't in dev mode).
The code inside
onMount should be executing 0 times on the server.Yea, this issue is making solid integration with astro unusable
For this to happen suggests the browser build is getting on the server somehow possibly.
onMount on the server is literally function onMount() {} https://github.com/solidjs/solid/blob/main/packages/solid/src/server/reactive.ts#L154So to sum up the current state of things: For folks using Cloudflare + Astro + Solid the immediate fix is use the cloudflare adapter version 9.2.1 or use client:only directives, the long term solution is that solid has found the solution but we are waiting on a minor version release and will need to update to that? Any ETA on the minor version release?
Well, and depending on your dependencies this is a solution still:
https://discord.com/channels/830184174198718474/1239920931510554655/1249724228794585178
I think as long as you don't have other dependencies that break when manually setting the resolve fields for Vite, then this is fine. If I understand, this is what the PR to change solid will do anyway is get Vite and some other bundlers to stop mapping backwards from package.json exports to the browser field just because it's a worker..
We are reviewing the features going into 1.9 right now. https://discord.com/channels/722131463138705510/780502110772658196/1280551507774672906
If I do have other dependencies that break does this mean the fix in core is going to break those same things then? I am using supabase and from what I can tell from the error messages it does not get along with these vite settings.
What errors are you getting?
The package management / bundling is a bit outside my comfort zone, but what should be happening with that fix I linked is that it's preventing Vite from mapping backwards from
workers conditional export to the browser field (which is the client build of solid which is breaking when done in SSR)
I'm not sure if it would be breaking or not from core. I don't think so. This workaround may be exlcluding some other conditions needed to get the correct supabase bundleVariations on this:
This seems like someone importing a node only library in one of their deps. We don't use node-fetch internally.
Maybe.. https://github.com/supabase/postgrest-js/issues/465
But also, it says,
Consider disabling ssr.noExternal or remove the built-in dependency.. Do you have it set to no external ?GitHub
1.8.1 @supabase/node-fetch dependencies breaks browser builds · Iss...
Bug report I confirm this is a bug with Supabase, not with my own application. I confirm I have searched the Docs, GitHub Discussions, and Discord. Describe the bug After installing the latest depe...
No I currently don't have it set so it is whatever is default for Astro the specific project is open source so feel free to poke around https://github.com/datagrove/marketplace-us . For now its working with the cloudflare adapter 9.2.1 so I may spend some more time hunting this down when I have the chance or maybe 1.9 will be the answer 🤷♀️
GitHub
GitHub - datagrove/marketplace-us
Contribute to datagrove/marketplace-us development by creating an account on GitHub.
@Rachael - took one more look at this:
Here's is the supabase's fork of node-fetch:
https://github.com/supabase/node-fetch/blob/2.x/package.json#L7...
They have a main field, and a browser field. The fix I gave you above removes
browser from the vite resolution conditions to fix it for solid, bc solid does not want the implicit mapping of worker -> back to browser that some of these bundlers do. (Bc for solid, worker != browser. The server version of solid is what runs in workers). (The default for Vite ['browser', 'module', 'jsnext:main', 'jsnext']), so you're getting the server build of node-fetch, which is importing node's stream, which is what is throwing your error.
The fix in solid shouldn't break this I wouldn't, cause the proposed fix thus far is to just remove that browser field from solid only, (which shouldnt' be needed tbh, but we can't change the bundlers. Imo, if the bundler shouldn't do a backwards mapping to the older browser field is a newer module.exports resolves to a valid identifier, but that's out of solid core's hands, but of course then that'd break other things from the bundlers' ends at this point.). Breakage would occur when a tool is trying to resolve solid's exports itself but don't support the modules.exports fields, which shouldn't be too many scenarios I wouldn't think. Most folks are probably compiling through vite / rollup / webpack / etc which can all read module.exportsGitHub
node-fetch/package.json at 2.x · supabase/node-fetch
A light-weight module that brings the Fetch API to Node.js - supabase/node-fetch
Awesome, this was very helpful thanks!