fix(cloudflare): preserve user-defined image service across all image… - #16654
fix(cloudflare): preserve user-defined image service across all image…#16654greatjourney589 wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 5f6d386 The changes in this PR will be included in the next version bump. 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 |
This comment was marked as low quality.
This comment was marked as low quality.
… image service is set
Hi, @adamchal , |
|
@greatjourney589 confirmed this works very well. This is really smart sleuthing and great work! Thanks again for taking the time on this. |
ematipico
left a comment
There was a problem hiding this comment.
The changeset needs to be reworded. I added more comments regarding code and tests
| Preserve user-defined image services when the Cloudflare adapter is used. Previously, the adapter's `imageService` mode (including the default `'cloudflare-binding'` and `'compile'`) would silently overwrite a custom `image.service` configured in `astro.config.*`, replacing it with the workerd image service. Custom services (e.g., third-party CDNs) are now preserved across all modes, matching the behavior of the explicit `'custom'` mode. | ||
|
|
||
| Additionally, the workerd prerenderer no longer hard-swaps `globalThis.astroAsset.imageService` to `astro/assets/services/sharp` for byte-level static image generation when a custom service is in use. Previously the `'compile'` mode bypassed the preserved service during the Node-side image generation pass; it now uses the user's configured service throughout the build. |
There was a problem hiding this comment.
This changeset is too lenghtly and too focused on the technical side of the fix. Changesets are meant for end users, not maintainers. Follow this guide https://contribute.docs.astro.build/docs-for-code-changes/changesets/#tips-and-examples
| // effect or `'compile'`/`'cloudflare-binding'` is selected. | ||
| const hasCustomService = config.service.entrypoint !== 'astro/assets/services/sharp'; | ||
| if (hasCustomService && buildService !== 'custom') { | ||
| logger.info( |
There was a problem hiding this comment.
We already check for custom in the switch, we just need to add hasCustomService check there and fallback to sharp otherwise.
| function createNoopLogger(): AstroIntegrationLogger { | ||
| const noop = () => {}; | ||
| const logger = { | ||
| options: {} as never, | ||
| label: 'test', | ||
| fork: () => logger, | ||
| info: noop, | ||
| warn: noop, | ||
| error: noop, | ||
| debug: noop, | ||
| }; | ||
| return logger as unknown as AstroIntegrationLogger; | ||
| } |
There was a problem hiding this comment.
This isn't needed, you can reduce this stub to
const spyLogger = new SpyLogger();
const logger = spyLogger.forkIntegrationLogger('test-spy');this is what we use in the mdx tests
| for (const mode of [ | ||
| undefined, // default → 'cloudflare-binding' | ||
| 'passthrough', | ||
| 'cloudflare', | ||
| 'cloudflare-binding', | ||
| 'compile', | ||
| { build: 'compile' as const }, | ||
| { build: 'compile' as const, runtime: 'cloudflare-binding' as const }, | ||
| ]) { |
There was a problem hiding this comment.
Shouldn't we test custom too? That's essentially where the PR is fixing things
|
Hey @greatjourney589 are you still interested in working on this or should we close this PR? |
|
@greatjourney589 I’m happy to take this over and see it through if you are strapped. It’s a real sore spot on my projects. |
|
I submitted a separate PR #17099 that directly addresses the issue #16201. This PR should be updated to:
|
|
Thanks @adamchal. Ok in that case I'll close this one. |
Changes
image.servicewith@astrojs/cloudflare/image-service-workerdwheneverimageServicewas unset (default'cloudflare-binding') or set to'compile'/'cloudflare-binding'/'cloudflare'/'passthrough'. Custom services (e.g., a third-party CDN service) are now preserved across all modes.setImageConfigthat detects a non-defaultimage.service(entrypoint ≠astro/assets/services/sharp) and returns the config untouched, mirroring the precedent in the existing fallbackdefault:branch and the explicit'custom'mode.logger.infoline when this preservation kicks in, telling the user the override was skipped and pointing them atimageService: 'custom'to silence the notice..changeset(@astrojs/cloudflarepatch).Testing
packages/integrations/cloudflare/test/image-config.test.ts(10 cases) covering preservation acrossundefined,'passthrough','cloudflare','cloudflare-binding','compile', and the compound{ build: 'compile' }/{ build: 'compile', runtime: 'cloudflare-binding' }configs. Also asserts the existing override of default sharp in'cloudflare-binding'and'compile'is preserved (no regression) and that explicit'custom'continues to work.external-image-service.test.ts(2/2),binding-image-service.test.ts(7/7),compile-image-service.test.ts(5/5).pnpm run buildsucceeds;tsc -bclean.Docs
No user-facing docs changes needed. Behavior aligns with what the existing docs already imply (a custom
image.serviceshould be respected); this PR brings the Cloudflare adapter into line. Worth a brief note in the Cloudflare adapter README that custom services are preserved alongsideimageServicemodes — happy to follow up in withastro/docs if maintainers prefer.Closes #16201