Skip to content

Add configurable Cloudflare image transforms#1390

Closed
mvvmm wants to merge 6 commits into
emdash-cms:mainfrom
mvvmm:media/cloudflare-image-transforms
Closed

Add configurable Cloudflare image transforms#1390
mvvmm wants to merge 6 commits into
emdash-cms:mainfrom
mvvmm:media/cloudflare-image-transforms

Conversation

@mvvmm

@mvvmm mvvmm commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

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

  • Bug fix
  • Feature
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable). Do not include messages.po changes except in translation PRs — a workflow extracts catalogs on merge to main.
  • I have added a changeset (if this PR changes a published package)
  • New features link to an approved Discussion: https://github.com/emdash-cms/emdash/discussions/...

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: OpenCode + GPT-5.5

Screenshots / test output

  • pnpm build
  • pnpm format:check
  • pnpm --filter emdash typecheck
  • pnpm --filter @emdash-cms/cloudflare typecheck
  • pnpm --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.ts
  • pnpm --filter @emdash-cms/cloudflare exec vitest run tests/media/image-transforms.test.ts
  • pnpm --filter emdash check
  • pnpm --filter @emdash-cms/cloudflare check
  • pnpm lint

@changeset-bot

changeset-bot Bot commented Jun 8, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6ed3a7e

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

This PR includes changesets to release 14 packages
Name Type
emdash Minor
@emdash-cms/cloudflare Minor
@emdash-cms/sandbox-workerd Patch
@emdash-cms/fixture-perf-site Patch
@emdash-cms/perf-demo-site Patch
@emdash-cms/cache-demo-site Patch
@emdash-cms/admin Minor
@emdash-cms/auth Minor
@emdash-cms/blocks Minor
@emdash-cms/gutenberg-to-portable-text Minor
@emdash-cms/x402 Minor
create-emdash Minor
@emdash-cms/auth-atproto Patch
@emdash-cms/plugin-embeds 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

@github-actions github-actions Bot added the review/needs-review No maintainer or bot review yet label Jun 8, 2026
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Scope check

This 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.

@pkg-pr-new

pkg-pr-new Bot commented Jun 8, 2026

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@1390

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@1390

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/@emdash-cms/auth-atproto@1390

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@1390

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@1390

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/contentful-to-portable-text@1390

emdash

npm i https://pkg.pr.new/emdash@1390

create-emdash

npm i https://pkg.pr.new/create-emdash@1390

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@1390

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/@emdash-cms/plugin-cli@1390

@emdash-cms/plugin-types

npm i https://pkg.pr.new/@emdash-cms/plugin-types@1390

@emdash-cms/registry-client

npm i https://pkg.pr.new/@emdash-cms/registry-client@1390

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/@emdash-cms/registry-lexicons@1390

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/@emdash-cms/sandbox-workerd@1390

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@1390

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@1390

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@1390

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@1390

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@1390

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@1390

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/@emdash-cms/plugin-field-kit@1390

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@1390

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@1390

commit: 6ed3a7e

@mvvmm mvvmm changed the title Add configurable Cloudflare image transforms WIP: Add configurable Cloudflare image transforms Jun 8, 2026
@mvvmm mvvmm marked this pull request as draft June 8, 2026 23:25
@mvvmm

mvvmm commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Jun 8, 2026
@mvvmm mvvmm marked this pull request as ready for review June 9, 2026 00:34
@mvvmm mvvmm changed the title WIP: Add configurable Cloudflare image transforms Add configurable Cloudflare image transforms Jun 9, 2026

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/cloudflare/src/media/image-transforms-runtime.ts Outdated
Comment thread packages/core/src/astro/routes/api/media/file/[...key].ts
Comment thread packages/cloudflare/src/media/image-transforms-runtime.ts
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Overlapping PRs

This PR modifies files that are also changed by other open PRs:

This may cause merge conflicts or duplicated work. A maintainer will coordinate.

@github-actions github-actions Bot added review/awaiting-author Reviewed; waiting on the author to respond and removed review/needs-review No maintainer or bot review yet labels Jun 9, 2026
@github-actions github-actions Bot added review/needs-rereview Author pushed changes since the last review and removed review/awaiting-author Reviewed; waiting on the author to respond labels Jun 9, 2026
@github-actions github-actions Bot added review/approved Approved; no new commits since and removed review/needs-rereview Author pushed changes since the last review labels Jun 9, 2026
@ascorbic

Copy link
Copy Markdown
Collaborator

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

@ascorbic

Copy link
Copy Markdown
Collaborator

I'm going to look at this, as I think there are a few subtleties that need to be addressed

@ascorbic

Copy link
Copy Markdown
Collaborator

Superseded by #1438

@ascorbic ascorbic closed this Jun 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants