withastro / adapters

Home for Astro's core maintained adapters
70 stars 41 forks source link

chore: externalize `cloudflare:sockets` imports #409

Closed alexanderniebuhr closed 3 weeks ago

alexanderniebuhr commented 1 month ago

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

Testing

Docs

changeset-bot[bot] commented 1 month ago

🦋 Changeset detected

Latest commit: 2da4e184914f35b51468a74b0c0d1e13c2a44069

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

alexanderniebuhr commented 1 month ago

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

alexanderniebuhr commented 1 month ago

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 commented 1 month ago

I see. Sounds like more a bugfix then.