chore: externalize cloudflare:sockets imports
#409
Conversation
🦋 Changeset detectedLatest commit: 2da4e18 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
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 |
.changeset/good-rivers-add.md
Outdated
| '@astrojs/cloudflare': minor | ||
| --- | ||
|
|
||
| Improves default config to support `cloudflare:sockets` imports |
There was a problem hiding this comment.
nit: since it's a minor, maybe the changeset should contain more information e.g. what it means for the users and they can do now
There was a problem hiding this comment.
Is it even a breaking change? I don't see how this would break any case that was working. It might change the error in some scenarios, but not add new ones.
There was a problem hiding this comment.
Added one more that should be externalized, thanks for this @alexanderniebuhr !!!
|
I changed the approach to use a vite plugin to externalize the imports based on resolveId, this should be more flexible and work for all modes, which follows this suggestion |
There was a problem hiding this comment.
The code looks good to me. I think we should craft a proper changeset, and maybe consider updating the docs, since now all cloudflare: specifiers are treated differently.
And maybe @Fryuni is onto something, because we actually change the behaviour of the build under hood. If want to avoid a breaking change, maybe this new behaviour should be opt-in, with a new config, and then make a major where it will be the default one.
|
I still don't think this is a breaking change or that we need to update the docs, because without this change, user had to add the code manually before, in there
But happy to discuss this further. |
|
I see. Sounds like more a bugfix then. |
UPDATE: I changed the approach to use a vite plugin to externalize the imports based on
resolveId, this should be more flexible and work for all modes.Changes
cloudflare:socketsas external in rollupOptions set in @astrojs/cloudflare #407astro dev, without usingwrangler, because in a local runtime the import wouldn't work, but at least the config wouldn't complain anymore.Testing
Docs