Skip to content

Commit

Permalink
[@astrojs/image] Handle query params in remote image URLs during SSG …
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
Tony Sullivan authored Aug 22, 2022
1 parent 2c40fba commit 579e2da
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 5 deletions.
5 changes: 5 additions & 0 deletions .changeset/chatty-ants-shop.md
Original file line number Diff line number Diff line change
@@ -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
22 changes: 20 additions & 2 deletions packages/integrations/image/src/utils/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { Image } from '@astrojs/image/components';
<br />
<Image id="google" src="https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_272x92dp.png" width={544} height={184} format="webp" />
<br />
<Image id='inline' src={import('../assets/social.jpg')} width={506} />
<Image id="inline" src={import('../assets/social.jpg')} width={506} />
<br />
<Image id="query" src="https://www.google.com/images/branding/googlelogo/2x/googlelogo_color_272x92dp.png?token=abc" width={544} height={184} format="webp" />
</body>
</html>
29 changes: 28 additions & 1 deletion packages/integrations/image/test/image-ssg.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <img> attributes', () => {
const image = $('#google');
Expand All @@ -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'
});
});
});
});

Expand Down Expand Up @@ -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'
);
});
});
});
});
42 changes: 41 additions & 1 deletion packages/integrations/image/test/image-ssr.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
});

Expand Down Expand Up @@ -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'
);
});
});
});

0 comments on commit 579e2da

Please sign in to comment.