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 breaks app when used in Vercel Edge #5915

Closed
1 task
pilcrowonpaper opened this issue Jan 20, 2023 · 22 comments · Fixed by #6085
Closed
1 task

SolidJS integration breaks app when used in Vercel Edge #5915

pilcrowonpaper opened this issue Jan 20, 2023 · 22 comments · Fixed by #6085
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@pilcrowonpaper
Copy link
Contributor

What version of astro are you using?

1.9.0

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?

MacOS

Describe the Bug

When using Solid components in an app hosted on Vercel edge, it throws the following error:

TypeError: Failed to execute 'encode' on 'TextEncoder': parameter 1 is not of type 'String'.
    at (entry.mjs:41:5485)
    at (entry.mjs:93:13)

Link to Minimal Reproducible Example

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

Participation

  • I am willing to submit a pull request for this issue.
@matthewp matthewp added - P4: important Violate documented behavior or significantly impacts performance (priority) - P3: minor bug An edge case that only affects very specific usage (priority) and removed - P4: important Violate documented behavior or significantly impacts performance (priority) labels Jan 26, 2023
@jmn
Copy link

jmn commented Jan 28, 2023

I ran into the same thing with Preact just now.

@bholmesdev
Copy link
Contributor

Thanks for reporting! Wouldn't be the first component issue on Vercel edge. @pilcrowonpaper @jmn have either of you been able to repro with Astro 2.0?

@jmn
Copy link

jmn commented Jan 30, 2023

Yes. I'm on Astro 2.0

@helmisatria
Copy link

I ran into the same thing, not using any specific UI framework/library (only .astro) and deployed on Cloudflare pages

@AirBorne04
Copy link
Contributor

I narrowed it down to the chunkToByteArray function beeing called with an object (with type: 'head') which is causing the issue. This is an issue happening besides ui framework and adapters just the vercel adapter does error out on it.

export function chunkToByteArray(

@AirBorne04
Copy link
Contributor

@pilcrowonpaper is there a way to test vercel edge runtime locally?

@pilcrowonpaper
Copy link
Contributor Author

@AirBorne04 I don't think so. I know it's based on Cloudflare workers but I'm not sure if there's a way to run that locally as well.

@AirBorne04
Copy link
Contributor

I managed to get what i wanted. Pretty funky all this 😄 Cloudflare has wrangler which runs the project locally. For vercel i managed now to upload the prebuilt and debug with the error messages. I have two minor issues so far and reached a render issue with Solid-js which we did fix for the cloudflare adapter, but vercel is building different and therefore i need to check how we can fix this as well.

@HiDeoo
Copy link
Member

HiDeoo commented Jan 31, 2023

is there a way to test vercel edge runtime locally?

@AirBorne04 I guess the closest should be https://edge-runtime.vercel.app/getting-started

@wkelly17
Copy link

Also getting this on Astro/cloudflare 6.1.1 and Astro 2.0.4. (Also using Solid, 1.6.10) Seems to happen whether adapter is in directory or advanced mode. Wrangler preview says " To use the new ReadableStream() constructor, enable the streams_enable_constructors feature flag," which is a little odd since CF says they are default on https://developers.cloudflare.com/workers/platform/compatibility-dates/#change-history.

@bholmesdev
Copy link
Contributor

bholmesdev commented Jan 31, 2023

Seems related to #4039. Also getting an "X not supported by the browser, returning undefined." Errors thrown are a red herring from improper handling of undefined HTML

@AirBorne04
Copy link
Contributor

is there a way to test vercel edge runtime locally?

@AirBorne04 I guess the closest should be https://edge-runtime.vercel.app/getting-started

I have been trying those, but did not manage to connect their addEventListener to play nicely with the astro generated code.

Also getting this on Astro/cloudflare 6.1.1 and Astro 2.0.4. (Also using Solid, 1.6.10) Seems to happen whether adapter is in directory or advanced mode. Wrangler preview says " To use the new ReadableStream() constructor, enable the streams_enable_constructors feature flag," which is a little odd since CF says they are default on https://developers.cloudflare.com/workers/platform/compatibility-dates/#change-history.

That is different, i have been running the same code on Cloudflare and it works as expected. For the flags you need to add it in the toml or via command line if you use wrangler locally. I also remember that they should be default by now, not sure what is going on there.

For this issue here i think there a slight changes in the implementation across vercel edge and cloudflare worker, or the compilation result are not 100% comparable, though the errors I could find do not seem to be discussable much. Here is what I have found and would address with a PR shortly.

@wkelly17
Copy link

wkelly17 commented Jan 31, 2023

@AirBorne04 - To use the new ReadableStream() constructor, enable the streams_enable_constructors feature flag," is a different error, but I primarily meant to say that I was also getting the OP's --

Failed to execute 'encode' on 'TextEncoder': parameter 1 is not of type 'String'.

The build doesn't break, but it just renders a blank page live. I used CF's real time functions logs, which, show success when hitting the functions endpoints directly, but throw the encoding error when trying to return HTML... The bit about streams_enable_contructors flag on wrangler preview was just some extra info that I wasn't sure if it were relevant or not.

@bholmesdev
Copy link
Contributor

bholmesdev commented Jan 31, 2023

@AirBorne04 Found something! Using target: "webworker" seems to break bundling in Vite 4. Removing this (to fall in-line with our other edge renderers) solves the issue 👍 Curious if there's implications removing this option though. Will check with core

@AirBorne04
Copy link
Contributor

Seems related to #4039. Also getting an "X not supported by the browser, returning undefined." Errors thrown are a red herring from improper handling of undefined HTML

This is actually the error message from SolidJS when the server code of (solidjs/web) is run from a browser context. The import handling is automagic and not easy to control during build.

@AirBorne04 - To use the new ReadableStream() constructor, enable the streams_enable_constructors feature flag," is a different error, but I primarily meant to say that I was also getting the OP's -- `Failed to execute 'encode' on 'TextEncoder': parameter 1 is not of type 'String'. The build doesn't break, but it just renders a blank page live. I used CF's real time functions logs, which, show success when hitting the functions endpoints directly, but throw the encoding error when trying to return HTML... The bit about streams_enable_contructors flag on wrangler preview was just some extra info that I wasn't sure if it were relevant or not.

Ah alright, the streams_enabled has nothing to do with it. I did not see the error in locally, but it makes sense.

@AirBorne04 Found something! Using target: "webworker" seems to break bundling in Vite 4. Removing this (to fall in-line with our other edge renderers) solves the issue 👍 Curious if there's implications removing this option though. Will check with core

It has implications, because other packages that are imported need this setting to webworker, so that the correct bundle is imported, for cloudflare we do have a two step compile approach which is fixing that problem, maybe we need to adjust here too.

@bholmesdev
Copy link
Contributor

@AirBorne04 Spoke with @matthewp and I agree adding an esbuild step to Vercel is a good approach. This would mirror our cloudflare fix, and they're more or less the same deployment target 👍

@AirBorne04
Copy link
Contributor

Do we think it makes sense to somehow / somewhere consolidate those build settings for the edge runtimes? If even possible? I will add the step to the vercel adapter as well in the meantime.

@bholmesdev
Copy link
Contributor

bholmesdev commented Jan 31, 2023

@AirBorne04 That's a good question! We'd probably need a shared "adapter utils" package (or some equivalent) to keep the packages separate. Not a bad idea though. Could possibly split out cloudflare utils for Vercel to depend on too 🤔

@AirBorne04
Copy link
Contributor

Maybe some sort of a edge runtime adapter base / util package.

@bholmesdev
Copy link
Contributor

@AirBorne04 Feel free to experiment in a PR commit if you want! Otherwise, happy to explore in a separate PR

@bholmesdev
Copy link
Contributor

@AirBorne04 Confirming if you plan to PR a fix! Otherwise, I'm happy to pick it up.

@AirBorne04
Copy link
Contributor

Yes i will put something together, also managed to get the edge-runtime running locally, maybe something for the preview option for the vercel adapter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)
Projects
None yet
8 participants