Add configurable Cloudflare image transforms#1390
Conversation
🦋 Changeset detectedLatest commit: 6ed3a7e The changes in this PR will be included in the next version bump. This PR includes changesets to release 14 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 |
|
All contributors have signed the CLA ✍️ ✅ |
Scope checkThis PR changes 537 lines across 23 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Summary
This PR introduces configurable media transforms for EmDash's file-serving route, with a Cloudflare Images adapter. The architecture is sound: a platform-neutral MediaTransformDescriptor interface, virtual-module wiring that follows the same pattern as storage/auth adapters, and graceful fallback to the original file when transforms fail or time out. The approach fits EmDash's existing adapter model well.
What I checked:
- The full diff against
origin/main, all 23 changed files - Virtual module generation, type declarations, and integration config pipeline
- The media file route (
[...key].ts) transform wiring - The Cloudflare runtime adapter (
image-transforms-runtime.ts) - Test coverage and AGENTS.md conventions (changeset present, no i18n/RTL concerns in this server-only change)
Headline conclusion:
The implementation is solid, but there is one real correctness bug in the Accept-header format negotiation. The imageFormatForRequest helper uses a naive substring match (includes("image/avif")) that can incorrectly select AVIF when the client explicitly rejects it with q=0. For a feature advertised as "negotiating AVIF or WebP from the Accept header," this is straightforward content-negotiation logic that should respect quality values.
The route also buffers the entire file into memory for every request that matches the transformable content types, even when the adapter ends up returning null or throwing. This changes the original streaming behavior to an in-memory buffer. For large images on memory-constrained Workers this is a performance concern, though it's an inherent trade-off of the current MediaTransformInput interface.
Finally, the @emdash-cms/cloudflare package has tests for the config-time helper but no tests for the actual runtime createMediaTransform function, timeout handling, or binding resolution.
Verdict: comment — one correctness bug to fix, plus two non-blocking suggestions.
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
|
I think this should be using the native Astro images feature rather than inventing something itself. That supports the images binding, but also other adapters |
|
I'm going to look at this, as I think there are a few subtleties that need to be addressed |
|
Superseded by #1438 |
What does this PR do?
Adds configurable media transforms for files served through EmDash's media file route, plus a Cloudflare Images adapter. Core stays platform-neutral via a media transform descriptor and virtual module; Cloudflare consumers can opt into
cloudflareImageTransforms({ binding: \"IMAGES\" }). Transform failures, missing bindings, and timeouts fall back to the original stored file.Closes #
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runmessages.pochanges except in translation PRs — a workflow extracts catalogs on merge tomain.AI-generated code disclosure
Screenshots / test output
pnpm buildpnpm format:checkpnpm --filter emdash typecheckpnpm --filter @emdash-cms/cloudflare typecheckpnpm --filter emdash exec vitest run tests/unit/astro/integration/virtual-modules.test.ts tests/unit/astro/media-file-transform.test.ts tests/unit/astro/routes.test.tspnpm --filter @emdash-cms/cloudflare exec vitest run tests/media/image-transforms.test.tspnpm --filter emdash checkpnpm --filter @emdash-cms/cloudflare checkpnpm lint