fix(@astrojs/cloudflare): fix imageService: 'compile' silent noop with prerenderEnvironment: 'node' - #17347
Conversation
… node (#17346) When `imageService: 'compile'` was used with `prerenderEnvironment: 'node'`, images were silently copied without optimization (PNG bytes in .webp files). The root cause was that `collectStaticImages` — which installs sharp for build-time transforms — only ran in the workerd prerenderer path. The node prerender path used the workerd passthrough stub instead. Fix: wrap the default prerenderer with a `collectStaticImages` method that installs sharp (or the user's custom service) before image generation runs, and restore the default prerender entrypoint that gets skipped when `settings.prerenderer` is set.
🦋 Changeset detectedLatest commit: 9d47fc6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 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 |
adamchal
left a comment
There was a problem hiding this comment.
@ematipico embarrassed I missed this one. Got a little fixated on the workerd prerenderer!
Read through the fix and it seems logical and the tests look good.
I also tested this fix locally to make sure that it does not regress anything in #17099 or #17285 on the workerd side.
|
Hi, giving this a gentle bump in case someone has a minute to review! LGTM. 🙏 |
alexanderniebuhr
left a comment
There was a problem hiding this comment.
@ematipico I wanted to approve this, but saw you assigned yourself, so I just left some comments after my review. Would love if we can get this merged :)
Implementation is solid and works as expected, just the tests have some minor nits AFAIK
|
@alexanderniebuhr it's all yours |
|
Thanks, I'll handle the changes and merge :) |
|
@alexanderniebuhr do you still plan on resolving this one? Thanks! |
Without cache isolation these tests pass on assets-cache hits from the earlier workerd suites (which generate the identical transforms into the shared node_modules/.astro cache), so they could not fail even with the fix reverted.
…overage With a user-configured image.service the Node prerender bundle already loads the user service via virtual:image-service, so this scenario worked before the fix — the test documents behavior rather than guarding the #17346 regression.
…lSharp The ERR_DLOPEN_FAILED rationale documented the skipRealSharp constant but was left above the build-time generation suite when the constant moved to the top of the file.
The restored entrypoint specifier and config shape duplicate the skip condition in packages/astro/src/core/build/vite-build-config.ts; note that they must stay in lockstep since drift silently reintroduces the unoptimized-image failure mode this fix removes.
…s as defensive With a user-configured image.service the prerender bundle already loads the service via virtual:image-service; the re-import exists for symmetry with the workerd prerenderer, not correctness.
e18e dependency analysisNo dependency warnings found. |
…taticImages The raw entrypoint import can fail where the bundled service works (e.g. TypeScript entrypoints on Node versions without type stripping), and any registered static image implies the bundled service is already cached in globalThis. Only import when the cache is empty — meaning no image was rendered and the service goes unused — and never fail the build over it.
Summary
Fixes a bug where
imageService: 'compile'produced unoptimized images (byte-for-byte source copies with incorrect extensions, e.g. PNG bytes in a.webpfile) whenprerenderEnvironmentwas set to'node'. No warning or error was emitted — the build completed silently.Closes #17346
Root Cause
Two interacting issues:
collectStaticImagesonly existed on the workerd prerenderer. This method installs sharp (or a user-configured image service) intoglobalThis.astroAsset.imageServicebefore the image generation pipeline runs. WhenprerenderEnvironment: 'node', the default Node prerenderer was used instead — which had nocollectStaticImages, so image transforms fell back to the workerd passthrough stub, returning input buffers unchanged.The default prerender entrypoint was skipped when
setPrerendererwas called. Astro skips setting the prerender entrypoint whensettings.prerendereris truthy, so restoring it manually was also required.Fix
In
packages/integrations/cloudflare/src/index.ts:astro:build:starthook — Added anelse if (hasBuildImageService)branch forprerenderEnvironment: 'node'. Wraps the default prerenderer with acollectStaticImagesmethod that installs sharp (or the user's custom service) before image generation runs, mirroring the existing workerd prerenderer behavior.astro:build:setuphook — Restores the default prerender entrypoint whenprerenderEnvironment: 'node'andhasBuildImageServiceare both true, since it gets skipped whensetPrerendereris called.Before:
(before: 12kB, after: 12kB)— output.webpis actually PNG bytesAfter:
(before: 12kB, after: 0kB)— proper WebP output (~494 bytes)Testing
Added
packages/integrations/cloudflare/test/compile-image-service.test.tswith tests covering:imageService: 'compile'withprerenderEnvironment: 'node'(the bug scenario)prerenderEnvironment: 'node'All existing tests continue to pass.