From 579e2daf8dd1816737d1bd253bf96c108a014061 Mon Sep 17 00:00:00 2001 From: Tony Sullivan Date: Mon, 22 Aug 2022 19:45:34 +0000 Subject: [PATCH] [@astrojs/image] Handle query params in remote image URLs during SSG builds (#4338) * fix: SSG builds should remove query params when building local image files * chore: add changeset * handling an edge case related to stripping extensions from a filename --- .changeset/chatty-ants-shop.md | 5 +++ .../integrations/image/src/utils/paths.ts | 22 +++++++++- .../basic-image/src/pages/index.astro | 4 +- .../integrations/image/test/image-ssg.test.js | 29 ++++++++++++- .../integrations/image/test/image-ssr.test.js | 42 ++++++++++++++++++- 5 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 .changeset/chatty-ants-shop.md diff --git a/.changeset/chatty-ants-shop.md b/.changeset/chatty-ants-shop.md new file mode 100644 index 000000000000..4c27d993c6dd --- /dev/null +++ b/.changeset/chatty-ants-shop.md @@ -0,0 +1,5 @@ +--- +'@astrojs/image': patch +--- + +When using remote images in SSG builds, query parameters from the original image source should be stripped from final build output diff --git a/packages/integrations/image/src/utils/paths.ts b/packages/integrations/image/src/utils/paths.ts index 6258d392da27..8521ac41fe9b 100644 --- a/packages/integrations/image/src/utils/paths.ts +++ b/packages/integrations/image/src/utils/paths.ts @@ -5,13 +5,31 @@ import type { TransformOptions } from '../loaders/index.js'; import { isRemoteImage } from './images.js'; import { shorthash } from './shorthash.js'; +function removeQueryString(src: string) { + const index = src.lastIndexOf('?'); + return index > 0 ? src.substring(0, index) : src; +} + +function removeExtname(src: string) { + const ext = path.extname(src); + + if (!ext) { + return src; + } + + const index = src.lastIndexOf(ext); + return src.substring(0, index); +} + export function ensureDir(dir: string) { fs.mkdirSync(dir, { recursive: true }); } export function propsToFilename({ src, width, height, format }: TransformOptions) { - const ext = path.extname(src); - let filename = src.replace(ext, ''); + // strip off the querystring first, then remove the file extension + let filename = removeQueryString(src); + const ext = path.extname(filename); + filename = removeExtname(filename); // for remote images, add a hash of the full URL to dedupe images with the same filename if (isRemoteImage(src)) { diff --git a/packages/integrations/image/test/fixtures/basic-image/src/pages/index.astro b/packages/integrations/image/test/fixtures/basic-image/src/pages/index.astro index 57b7fd97c9fc..f83897ddf59f 100644 --- a/packages/integrations/image/test/fixtures/basic-image/src/pages/index.astro +++ b/packages/integrations/image/test/fixtures/basic-image/src/pages/index.astro @@ -12,6 +12,8 @@ import { Image } from '@astrojs/image/components';

- + +
+ diff --git a/packages/integrations/image/test/image-ssg.test.js b/packages/integrations/image/test/image-ssg.test.js index 0b1fe192a608..47bbe76b785e 100644 --- a/packages/integrations/image/test/image-ssg.test.js +++ b/packages/integrations/image/test/image-ssg.test.js @@ -58,9 +58,10 @@ describe('SSG images', function () { }); describe('Remote images', () => { - // Hard-coding in the test! This should never change since the hash is based + // Hard-coding in the test! These should never change since the hash is based // on the static `src` string const HASH = 'Z1iI4xW'; + const HASH_WITH_QUERY = '18Aq0m'; it('includes attributes', () => { const image = $('#google'); @@ -79,6 +80,14 @@ describe('SSG images', function () { type: 'webp', }); }); + + it('removes query strings', () => { + verifyImage(`_image/googlelogo_color_272x92dp-${HASH_WITH_QUERY}_544x184.webp`, { + width: 544, + height: 184, + type: 'webp' + }); + }); }); }); @@ -174,6 +183,24 @@ describe('SSG images', function () { 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_272x92dp.png' ); }); + + it('keeps remote image query params', () => { + const image = $('#query'); + + const src = image.attr('src'); + const [route, params] = src.split('?'); + + expect(route).to.equal('/_image'); + + const searchParams = new URLSearchParams(params); + + expect(searchParams.get('f')).to.equal('webp'); + expect(searchParams.get('w')).to.equal('544'); + expect(searchParams.get('h')).to.equal('184'); + expect(searchParams.get('href')).to.equal( + 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_272x92dp.png?token=abc' + ); + }); }); }); }); diff --git a/packages/integrations/image/test/image-ssr.test.js b/packages/integrations/image/test/image-ssr.test.js index 37274c929aa6..9344dfc72f37 100644 --- a/packages/integrations/image/test/image-ssr.test.js +++ b/packages/integrations/image/test/image-ssr.test.js @@ -122,9 +122,31 @@ describe('SSR images - build', function () { expect(searchParams.get('f')).to.equal('webp'); expect(searchParams.get('w')).to.equal('544'); expect(searchParams.get('h')).to.equal('184'); - // TODO: possible to avoid encoding the full image path? expect(searchParams.get('href').endsWith('googlelogo_color_272x92dp.png')).to.equal(true); }); + + it('keeps remote image query params', async () => { + const app = await fixture.loadTestAdapterApp(); + + const request = new Request('http://example.com/'); + const response = await app.render(request); + const html = await response.text(); + const $ = cheerio.load(html); + + const image = $('#query'); + + const src = image.attr('src'); + const [route, params] = src.split('?'); + + expect(route).to.equal('/_image'); + + const searchParams = new URLSearchParams(params); + + expect(searchParams.get('f')).to.equal('webp'); + expect(searchParams.get('w')).to.equal('544'); + expect(searchParams.get('h')).to.equal('184'); + expect(searchParams.get('href').endsWith('googlelogo_color_272x92dp.png?token=abc')).to.equal(true); + }); }); }); @@ -216,5 +238,23 @@ describe('SSR images - dev', function () { 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_272x92dp.png' ); }); + + it('keeps remote image query params', () => { + const image = $('#query'); + + const src = image.attr('src'); + const [route, params] = src.split('?'); + + expect(route).to.equal('/_image'); + + const searchParams = new URLSearchParams(params); + + expect(searchParams.get('f')).to.equal('webp'); + expect(searchParams.get('w')).to.equal('544'); + expect(searchParams.get('h')).to.equal('184'); + expect(searchParams.get('href')).to.equal( + 'https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_272x92dp.png?token=abc' + ); + }); }); });