-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix(cloudflare): preserve user-defined image service across all image… #16654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@astrojs/cloudflare': patch | ||
| --- | ||
|
|
||
| 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. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,19 @@ export function setImageConfig( | |
| ) { | ||
| const { buildService, runtimeService } = normalizeImageServiceConfig(service); | ||
|
|
||
| // If the user has configured a non-default image service, preserve it across | ||
| // all `imageService` modes. This matches the explicit `'custom'` mode and the | ||
| // fallback branch below, and prevents the adapter from silently overriding a | ||
| // user-defined service (e.g., a third-party CDN) when the default mode is in | ||
| // effect or `'compile'`/`'cloudflare-binding'` is selected. | ||
| const hasCustomService = config.service.entrypoint !== 'astro/assets/services/sharp'; | ||
| if (hasCustomService && buildService !== 'custom') { | ||
| logger.info( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already check for |
||
| `Detected a custom image service (${config.service.entrypoint}). Preserving it instead of applying the '${buildService}' mode override. Set \`imageService: 'custom'\` to silence this notice.`, | ||
| ); | ||
| return { ...config }; | ||
| } | ||
|
|
||
| switch (buildService) { | ||
| case 'passthrough': | ||
| return { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import * as assert from 'node:assert/strict'; | ||
| import { describe, it } from 'node:test'; | ||
| import type { AstroConfig, AstroIntegrationLogger } from 'astro'; | ||
| import { setImageConfig } from '../src/utils/image-config.ts'; | ||
|
|
||
| const SHARP_ENTRYPOINT = 'astro/assets/services/sharp'; | ||
| const CUSTOM_ENTRYPOINT = 'my-app/image-services/cdn'; | ||
| const WORKERD_ENTRYPOINT = '@astrojs/cloudflare/image-service-workerd'; | ||
|
|
||
| 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; | ||
| } | ||
|
Comment on lines
+10
to
+22
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
|
||
| function makeConfig(entrypoint: string): AstroConfig['image'] { | ||
| return { service: { entrypoint, config: {} } } as AstroConfig['image']; | ||
| } | ||
|
|
||
| describe('setImageConfig — custom image service preservation', () => { | ||
| 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 }, | ||
| ]) { | ||
|
Comment on lines
+29
to
+37
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we test |
||
| it(`preserves a user-defined service when imageService = ${JSON.stringify(mode)}`, () => { | ||
| const customConfig = makeConfig(CUSTOM_ENTRYPOINT); | ||
| const result = setImageConfig(mode as any, customConfig, 'build', createNoopLogger()); | ||
| assert.equal( | ||
| result.service.entrypoint, | ||
| CUSTOM_ENTRYPOINT, | ||
| `Expected custom service to be preserved for mode ${JSON.stringify(mode)}`, | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
| it("does not preserve when user has the default sharp service (mode 'cloudflare-binding')", () => { | ||
| const sharpConfig = makeConfig(SHARP_ENTRYPOINT); | ||
| const result = setImageConfig('cloudflare-binding', sharpConfig, 'build', createNoopLogger()); | ||
| assert.equal( | ||
| result.service.entrypoint, | ||
| WORKERD_ENTRYPOINT, | ||
| 'Expected adapter to override default sharp for cloudflare-binding mode', | ||
| ); | ||
| }); | ||
|
|
||
| it("does not preserve when user has the default sharp service (mode 'compile')", () => { | ||
| const sharpConfig = makeConfig(SHARP_ENTRYPOINT); | ||
| const result = setImageConfig('compile', sharpConfig, 'build', createNoopLogger()); | ||
| assert.equal( | ||
| result.service.entrypoint, | ||
| WORKERD_ENTRYPOINT, | ||
| 'Expected adapter to override default sharp for compile mode', | ||
| ); | ||
| }); | ||
|
|
||
| it("respects an explicit 'custom' mode regardless of service", () => { | ||
| const customConfig = makeConfig(CUSTOM_ENTRYPOINT); | ||
| const result = setImageConfig('custom', customConfig, 'build', createNoopLogger()); | ||
| assert.equal(result.service.entrypoint, CUSTOM_ENTRYPOINT); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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