Repository navigation
fix(core): use external image services for stored thumbnails - #2175
masonjames wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 024660b The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 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 |
@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-test
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-loader
@emdash-cms/registry-moderation
@emdash-cms/registry-verification
upgrade-emdash
@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: |
There was a problem hiding this comment.
This is a sensible, well-scoped bug fix: HEIC/HEIF files are undisplayable in browsers, so converting them client-side before they hit the existing local-media upload flow is the right layer to solve it. The change is additive (no break), the CSP-compatible heic-to/csp import is appropriate, the changeset is user-facing, and no user-visible strings were added. Tests cover conversion behavior, browser renderability, and end-to-end flow through the media API.
I read the changed files and traced the two call-sites (MediaLibrary and MediaPickerModal). The only meaningful gap is that the same HEIC fix is not applied to uploadToProvider, so external-provider uploads can still store an undisplayable original. The rest of the code looks solid.
|
I'm a bit reluctant to add a big new WASM dependency for this. It seems like something the image server should be handling. |
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. |
|
This PR has been inactive for 14 days. It will be closed automatically in 7 days if there is no further activity. If you're still working on this, please push an update or leave a comment. |
|
Refreshed onto current Local validation is clean: full build, 64 focused core media tests, 10 admin thumbnail tests, package typecheck, zero-diagnostic type-aware lint, formatting, and |
|
This PR is waiting on its author because it has merge conflicts with |
Avoid sending stored-media requests to Astro’s local-only image endpoint when an external service is configured. Preserve public source versions and mutable cache headers while leaving local transforms and upload acceptance unchanged.
8783bc8 to
024660b
Compare
danielmlr
left a comment
There was a problem hiding this comment.
Thanks, this is a good fix, and it's great that the image service does the work now! It needs two small changes before we can merge it.
- Please only send the new redirect when someone is signed in (
ctx.locals.user), and change itsCache-Controlfrompublictoprivate. Only the admin uses these/_imagelinks, and the admin is always signed in. Right now, though, anyone can ask for a link to any file name at any size. On an image service that charges for each new image, a stranger could run up the site owner's bill this way, and on a service that protects its links with a signature, anyone could get a valid one. Withprivate, a shared cache can't pass a signed-in user's redirect on to someone else. Visitors who aren't signed in won't notice any difference. - Please rewrite the changeset in plain words and say that the fix needs a storage
publicUrl. People read it in the changelog when they upgrade, so it should tell them whether the fix applies to their site. For example: "Fixes media library thumbnails on Node sites that use an external Astro image service and a storagepublicUrl. The image service now makes the thumbnails, including HEIC if it supports it, so the admin no longer loads the full original."
Thanks again!
What does this PR do?
Stored-media thumbnails on Node fail when Astro uses an external image service: EmDash forwards the request to Astro's generic endpoint, which requires a local transformer. The endpoint now passes the storage adapter's public URL through the configured service's
validateOptionsandgetURL, retaining replacement versions and revalidation headers. This allows existing HEIC media to render when the service supports it.The fix requires a source URL the image service can reach. Upload acceptance is unchanged. It follows the maintainer's request to use the image server.
Type of change
Your connection to this change
This comes from stored-media previews on my EmDash site using Node, S3-compatible R2 storage, and an external image service.
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runAI assistance disclosure
Screenshots / test output
pnpm format, rootpnpm build,pnpm typecheck,pnpm lint:quick, andpnpm lintpass. Type-aware JSON lint reports zero diagnostics.pnpm --dir docs buildandgit diff --checkpass.sips; this is not a hosted-provider or production-deployment claim.