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

SolidJS integration on Vercel Edge breaks with "Ua is not supported in the browser" #6989

Closed
1 task
Coretteket opened this issue May 4, 2023 · 9 comments
Closed
1 task
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) pkg: solid Related to Solid (scope)

Comments

@Coretteket
Copy link

What version of astro are you using?

2.3.4

Are you using an SSR adapter? If so, which one?

Vercel Edge

What package manager are you using?

pnpm

What operating system are you using?

Windows

What browser are you using?

Chrome, Firefox

Describe the Bug

It appears that issue #5915 has re-appeared recently. Using versions astro@^2.3.4, @astrojs/solid-js@^2.1.1, @astrojs/vercel@^3.3.0, I'm getting the following error in Vercel logs, and an empty SolidJS component on client. Note that this error only appears when deployed.

Error: Ua is not supported in the browser, returning undefined
    at (entry.mjs:56:32412)
    at (entry.mjs:56:32522)
    at (entry.mjs:56:33846)
    at (entry.mjs:42:1044)

The minimal reproducible example is identical to that of issue #5915, except for the updated packages as described above.

Link to Minimal Reproducible Example

https://github.com/pilcrowOnPaper/astro-vercel-solid

Participation

  • I am willing to submit a pull request for this issue.
@wkelly17
Copy link

wkelly17 commented May 4, 2023

I don't have a min example to contribute, but I think I'm getting the same thing on Cloudflare as well. Same deps cause the same blank issue, and locking to
astro/cloudflare 6.2.2 and @astrojs/solid-js 2.1 seems to fix it. So, not sure if was the bump to 2.1.1 in astro/solid or some shared logic for the edge betwen 6.2.2 and 6.2.4, but seems like it was solid-js 2.1 is my guess.

@johannesspohr
Copy link
Contributor

Same issue here on Cloudflare, can confirm that 2.1.1 is the breaking version.

It looks like because solid is not externalized any more, the worker build includes the web version of solid-js, which doesn't contain renderToString but a mock version of it, which just throws the error message.

With disabled minification, the error message is renderToString2 is not supported in the browser, returning undefined.

@johannesspohr
Copy link
Contributor

johannesspohr commented May 5, 2023

It's related to the module resolution done by the final esbuild step. Since it's currently set to target browser environment, the client version of solid-js is included in the worker bundle. This is fixed by #6915.

@bluwy bluwy added pkg: solid Related to Solid (scope) - P4: important Violate documented behavior or significantly impacts performance (priority) labels May 8, 2023
@bluwy bluwy self-assigned this May 15, 2023
@bluwy
Copy link
Member

bluwy commented May 15, 2023

While #7092 fixes the case for cloudflare, I think there was an unintentional breaking change somewhere. Gonna look into this again this week

@bluwy
Copy link
Member

bluwy commented May 16, 2023

Looks like #4888 is causing quite some issues:

  1. I don't think the solid integration should change ssr.target to node. The deployment platform, in this case, vercel edge and cloudflare, should always set this strictly as webworker.
  2. Update Solid ecosystem package logic to include packages with peerDep… #6934 undid the fix in adjusting cloudflare adapter and solid ssr to work together #4888. That's why it caused the regression.

So I'm thinking of a plan of:

  1. Remove the solid integration ssr.target = 'node' setting
  2. Make vercel edge and cloudflare always set ssr.target = 'webworker'
  3. Set cloudflare condition to workerd and worker: Add workerd and worker to cloudflare adapter bundling #7092
  4. Set vercel edge condition to edge-light (spec/proposal) and worker (though not documented, this is the only way to make solid-js work) (NOTE: research link)

@bluwy
Copy link
Member

bluwy commented May 17, 2023

Step 1-4 should be done now, so updating to the latest version of @astrojs/vercel and @astrojs/cloudflare should fix it when it's released. Unfortunately this still leaves solidjs in a state of a regression (if you dont upgrade the adapters), but we've done changes like this in the past so I'm leaning towards "upgrading packages to latest" as the fix for now.

Marking this closed as the stuff to do is completed, the next release should come around today or tomorrow for Astro 2.5 + the other packages.

@bluwy bluwy closed this as completed May 17, 2023
@pilcrowonpaper
Copy link
Contributor

OK so I was getting the same error after updating everything to the latest version. Took quite a while to debug but I needed to remove the noExternal config that was used to fix the same issue in an older version:

export default defineConfig({
	 vite: {
		ssr: {
			noExternal: ["solid-js"]
		}
	}
});

@sncalvo
Copy link

sncalvo commented Aug 31, 2023

Currently getting this error with @astrojs/deno and @astrojs/cloudflare after upgrading all packages to latest. Has anyone else experienced this after Astro 3.0 or upgrade of a related library?

@johannesspohr
Copy link
Contributor

My cloudflare deployment using astro 3 and latest SolidJS integration is working fine, so I assume it's something unrelated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) pkg: solid Related to Solid (scope)
Projects
None yet
Development

No branches or pull requests

6 participants