withastro / adapters

Home for Astro's core maintained adapters
47 stars 26 forks source link

feat(cloudflare): Exclude prerender and unused chunks from server bundle #222

Closed Fryuni closed 2 months ago

Fryuni commented 3 months ago

Changes

I'm okay with putting this behind a flag (experimental or not), the wiring is quite trivial to make configurable.

Testing

I added an example of an offending project that includes all content collections in the server bundle even though those are only used in pre-rendered pages, causing unnecessary bloat.

Docs

I don't think this needs any docs changes, but happy to document it if others think it does.

changeset-bot[bot] commented 3 months ago

🦋 Changeset detected

Latest commit: fc0ff552e9007cc9bf0d8f16644cbf9ba406116f

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

This PR includes changesets to release 9 packages | Name | Type | | ------------------------------------------------ | ----- | | @astrojs/cloudflare | Minor | | @test/astro-cloudflare-astro-dev-platform | Patch | | @test/astro-cloudflare-external-image-service | Patch | | @test/astro-cloudflare-no-output | Patch | | @test/astro-cloudflare-prerender-optimizations | Patch | | @test/astro-cloudflare-routes-json | Patch | | @test/astro-cloudflare-wasm | 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 2 months ago

Some more context/findings:

https://discord.com/channels/830184174198718474/845430950191038464/1226408296672395264 https://discord.com/channels/830184174198718474/845430950191038464/1226411660114133032

alexanderniebuhr commented 2 months ago

!preview cf-no-prerender-chunks

github-actions[bot] commented 2 months ago

Snapshots have been released for the following packages:

🦋  warn ===============================IMPORTANT!===============================
🦋  warn Packages will be released under the experimental--cf-no-prerender-chunks tag
🦋  warn ----------------------------------------------------------------------
🦋  info npm info @astrojs/cloudflare
🦋  info npm info @astrojs/netlify
🦋  info @astrojs/cloudflare is being published because our local version (0.0.0-cf-no-prerender-chunks-20240412140922) has not been published on npm
🦋  warn @astrojs/netlify is not being published because version 5.2.0 is already published on npm
🦋  info Publishing "@astrojs/cloudflare" at "0.0.0-cf-no-prerender-chunks-20240412140922"
🦋  success packages published successfully:
🦋  @astrojs/cloudflare@0.0.0-cf-no-prerender-chunks-20240412140922
🦋  Creating git tag...
🦋  New tag:  @astrojs/cloudflare@0.0.0-cf-no-prerender-chunks-20240412140922

Build Log ``` > root@0.0.0 build /home/runner/work/adapters/adapters > turbo run build --filter="@astrojs/*" • Packages in scope: @astrojs/cloudflare, @astrojs/netlify, @astrojs/test-utils • Running build in 3 packages • Remote caching disabled ::group::@astrojs/netlify:build cache miss, executing a68730fc97f3487c > @astrojs/netlify@5.2.0 build /home/runner/work/adapters/adapters/packages/netlify > tsc ::endgroup:: ::group::@astrojs/cloudflare:build cache miss, executing 12fa5edd1cdf35df > @astrojs/cloudflare@0.0.0-cf-no-prerender-chunks-20240412140922 build /home/runner/work/adapters/adapters/packages/cloudflare > tsc ::endgroup:: Tasks: 2 successful, 2 total Cached: 0 cached, 2 total Time: 3.63s ```
ascorbic commented 3 weeks ago

I'm looking at this in reference to https://github.com/withastro/astro/issues/10552 and I'm wondering if this would be better to be in core rather than adapters. The same thing seems to happen for all adapters, and it shouldn't really be the responsibility of an adapter to clean up the build liek this.

Fryuni commented 3 weeks ago

We started discussing how to better solve this in core. It is not on a proper thread, just scattered across the #contribute channel, but here is one message related to the topic

alexanderniebuhr commented 3 weeks ago

@ascorbic take a look at https://github.com/withastro/astro/pull/11245, I'm not totally sure if that fixes the https://github.com/withastro/astro/issues/10552 issue, but it should make this workaround inside the adapter not needed anymore.

ascorbic commented 3 weeks ago

@alexanderniebuhr ooh, nice. Thanks for the pointer