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

@astro/solid-js does not work with @astro/cloudflare adapter #4766

Closed
1 task
AirBorne04 opened this issue Sep 15, 2022 · 7 comments · Fixed by #4815
Closed
1 task

@astro/solid-js does not work with @astro/cloudflare adapter #4766

AirBorne04 opened this issue Sep 15, 2022 · 7 comments · Fixed by #4815
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@AirBorne04
Copy link
Contributor

What version of astro are you using?

1.2.4

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

Cloudflare

What package manager are you using?

npm

What operating system are you using?

Windows

Describe the Bug

SSR is enforced due to Cloudflare adapter. But SSR is not working with Solid in the Cloudflare environment (Worker).

Error: Ct is not supported in the browser, returning undefined
Here Ct is the minified name of { renderToString } from "solid-js/web". So Astro seems to call the server function for SSR but Solid recognizes the environment of a browser and does not seem to support it.

There are some potential solutions to work around this:

  • do not use a UI Framework or apply client:only to all components and loose pre-render completely
  • do not use the Cloudflare adapter but instead build static (build API functions native for Cloudflare)

What astrojs could do:

  • bring up a message stating that SSR mode for renderes is not supported with the Cloudflare adapter
  • statically pre-render for a worker environment

Link to Minimal Reproducible Example

https://github.com/AirBorne04/minimal-astro-cloudflare-solid

Participation

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

Have you reached out to the SolidJS people about this? I'm betting they'd like to make it work in Cloudflare.

@matthewp matthewp added the - P3: minor bug An edge case that only affects very specific usage (priority) label Sep 15, 2022
@matthewp
Copy link
Contributor

Did a little research and the reason is because of this line:

I think this is configuring Vite to prefer web entrypoints which results in the non-SSR code being bundled for Solid.

@nrgnrg wrote the Cloudflare adapter, do you know why you used this setting?

@matthewp matthewp changed the title @astro/cloudflare + @astro/solid-js = 🔥 @astro/solid-js does not work with @astro/cloudflare adapter Sep 15, 2022
@AirBorne04
Copy link
Contributor Author

I think the target: 'webworker' is generally correct and probably needed for other ui frameworks that support webworker rendering? (when i was testing with react that worked, but i am not sure of the implications on the rendering itself)

So there are 2 questions than:

  • can solid-js adopt to that setting?
  • can an astro ui plugin opt out of it?

@ryansolid, @nksaraf can you maybe give us a pointer, on how solid is meant to work in an webworker environment in regards to SSR / SSG. I have seen the SolidJS Guide mentioning streaming to work in a Cloudflare worker, but did not manage to get it to work. Is there some sort of example repo?

@ryansolid
Copy link
Contributor

We can add an export condition to support this. We just need to figure out which. Is it the "worker" condition? Generally speaking we have nothing Node specific so we've been just using "node" condition and it's been fine. But there are reasons not the set the "node" condition for workers.

@AirBorne04
Copy link
Contributor Author

Some more digging revealed to me that the target webworker forces a single file build. But this was not the real issue, rather the esbuild configuration. With the following 2 changes the issues were gone.

and that works for react and solid-js, I am not sure about svelte and vue though. Can somebody help test those out?

@ryansolid I guess there is no need for a condition since it works perfectly with the node condition.

@ryansolid
Copy link
Contributor

I think the worker condition has value because not all node APIs are supported and some libraries may expect that if you set the condition to node.

Platform browser could be really awkward if it overrides things. We have browser field too so that people can have an environment work in browser mode, but browser mode is way different than server rendering in a worker.

@AirBorne04
Copy link
Contributor Author

Spot on, just had my issues finding how to configure and fallback. Found the correct settings though, to support worker and fallback to node and created a PR #4815 to address it.

@bluwy bluwy linked a pull request Sep 20, 2022 that will close this issue
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
Development

Successfully merging a pull request may close this issue.

3 participants