chore: externalize cloudflare:sockets imports

[alexanderniebuhr]

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

• closes Consider treating cloudflare:sockets as external in rollupOptions set in @astrojs/cloudflare #407
• I still don't know if it would work with astro dev, without using wrangler, because in a local runtime the import wouldn't work, but at least the config wouldn't complain anymore.

Testing

• existing tests

Docs

• bugfix not needed

[changeset-bot]

🦋 Changeset detected

Latest commit: 2da4e18

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages

Name	Type
@astrojs/cloudflare	Patch
@test/astro-cloudflare-astro-dev-platform	Patch
@test/astro-cloudflare-astro-env	Patch
@test/astro-cloudflare-compile-image-service	Patch
@test/astro-cloudflare-external-image-service	Patch
@test/astro-cloudflare-wasm	Patch
@test/astro-cloudflare-no-output	Patch
@test/astro-cloudflare-routes-json	Patch
@test/astro-cloudflare-with-solid-js	Patch
@test/astro-cloudflare-wrangler-preview-platform	Patch

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

[ematipico]

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

[Fryuni]

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.

[irvinebroque]

Added one more that should be externalized, thanks for this @alexanderniebuhr !!!

[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

[ematipico]

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.

[alexanderniebuhr]

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 astro.config.js.

cloudflare: imports can't be bundled, since those are runtime features AFAIK. So before if the user did not externalize those imports manually, they ended up with an error during astro build 🤔

But happy to discuss this further.

[ematipico]

I see. Sounds like more a bugfix then.

[Fryuni]

Code seems great. Changeset needs update.

I agree with Ema that it's a bugfix.