fix(cloudflare): inject headers for caching the assets folder#16571
Conversation
🦋 Changeset detectedLatest commit: c48d4f0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 37 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 |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
8b568f9 to
32acd0e
Compare
ematipico
left a comment
There was a problem hiding this comment.
Blocking because it's unclear what this PR does. It it's fixing a bug, there should be at least a reproduction and the PR should explain the fix, not "the changes"
|
This brings the Cloudflare adapter to parity with other official adapters. Unfortunately it's more complicated because CF doesn't have a nicer way for us to provide headers |
florian-lefebvre
left a comment
There was a problem hiding this comment.
I think it looks pretty good! Ideally it should only include unit tests to speed things up. Do you think you could update the PR to do that? That likely requires extracting the whole logic to a dedicated function so it can be tested in isolation (and accepting some things like readFile) as arguments. See https://github.com/withastro/astro/blob/main/CONTRIBUTING.md#making-code-testable
Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
|
@florian-lefebvre Good idea! Just did, |
|
@ematipico can you have another look? |
Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
ematipico
left a comment
There was a problem hiding this comment.
We should probably put more emphasis in the changeset
Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
|
@MA2153 can you resolve the conflicts? |
# Conflicts: # packages/integrations/cloudflare/src/index.ts # packages/integrations/cloudflare/test/with-base.test.ts
|
@florian-lefebvre Done |
Fixes #16692
Changes
Cache-Control: public, max-age=31536000, immutablefor hashed Astro assets (/_astro/*) into Cloudflare's_headersfile at build time, so browsers cache assets across deploys without extra user configuration.build.assetsPrefixis set (assets are served from a different origin)._headersalready has aCache-Controlrule whose URL pattern matches the assets path — Cloudflare merges duplicate matching rules' headers with a comma, which would produce contradictory cache directives.baseconfig: the injected pattern is prefixed accordingly (e.g./blog/_astro/*)._headerswrite (write to.tmp, then rename) to avoid leaving the file truncated on a mid-write crash.Testing
headersFileHasCacheControlForPathutility inpackages/integrations/cloudflare/test/headers.test.ts, covering: empty files, exact patterns, global splat (/*),! Cache-Controldetach, non-matching rules, placeholder patterns (:name), host-prefixed patterns, comments/blank lines, base-prefixed paths, and case-insensitivity.with-baseintegration test to assert the injected block is present with the correct base prefix, that user-defined headers are preserved, and that the injected block is placed before user headers.Docs
No documentation changes needed — this is an internal build-time improvement for Cloudflare deployments. The behavior is automatic and the skip conditions (logged via
logger.info) are observable in build output. No new configuration options were added.Other Info
Live preview of the fix: https://with-fix-astro-assets-cf-repro.ma2153.workers.dev/