bugfix: rewrite lit.js package resolution to use node export for Cloudflare SSR #6915
Conversation
🦋 Changeset detectedLatest commit: 80d5074 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
bot
added
the
pkg: integration
chaffinated
changed the title
|
The same happened for Also, the vercel edge integration basically uses the same esbuild config and will probably suffer from the same issue (as described in #6989), I guess the same fix could be applied there as well. |
ematipico
changed the title
|
@chaffinated great! In #7092 I added a test for solid, do you think it makes sense to also include sth like that for lit? |
|
@johannesspohr Yup! And I just found the |
to reflect "neutral" esbuild setting, instead of "node"
Stick with the standard CloudFlare convention of using `browser: "platform"` but with added export condition support for "worker" type environments; for SSR-specific package exports (which usually use `platform: "node"`), we instead will use hard-coded resolve rewrites to point to the appropriate exports for Lit.js libraries with added support for custom rewrite options passed to the CloudFlare adapter
chaffinated
changed the title
|
The new changes looks good to me. I've also commented at lit/lit#1983 (comment) to hear what their thoughts are on this before we move on with the patch. But I'm happy to move on with this if there's not much feedback. |
|
Gonna merge this one once the CI passes. I don't think it'll be fixed in Lit soon, but I'll be following lit/lit#3907 for any updates there. |
|
Looks like running two wrangler CLIs caused the CI to be flaky (even before my cleanup). Gonna have to investigate it a bit before merging. |
|
I don't think we should accept this changes as is. I really don't want the Cloudflare adapter to have special-case code for one framework. That doesn't scale. Instead I think it would be fine if the Cloudflare adapter had some API where its esbuild configuration could be extended, and the Lit adapter implemented support for that. |
|
We operate on consensus. The approach I think we should take here is:
|
|
@bluwy @chaffinated any progress? |
|
Since this is quite a bit outdated, will need a new PR. |
|
So what's the right way to go about this, since all related issues and PRs seem to have been closed after a lack of consensus or direction? As a user, how can I register with the Astro team that this would be a nice feature? |
|
@dengel29 My comment here is still true: #6915 (comment) The Cloudflare adapter is community maintained by the maintainer is extremely active. If you brought up this issue and asked for such a config I bet he would be willing to work with you. That adapter lives here: https://github.com/withastro/adapters |
Changes
fix withastro/adapters#42
Rewrite lit packages entrypoint paths to work in Cloudflare SSR
Testing
Added test in cloudflare integration
Docs
No documentation was added because this should not directly affect developer experience.