From a2b66c754969af4ce98bb10654286a4445cb0999 Mon Sep 17 00:00:00 2001 From: Oskar Baumann <62597223+panwauu@users.noreply.github.com> Date: Thu, 6 Oct 2022 17:53:11 +0200 Subject: [PATCH] [@astrojs/image]: Fix component with query string (#4997) * remove query params from file extension * Revert changes to make change as small as possible * moving the removeQueryParam check to basename() * chore: adding a changeset * adding SSR test coverage for new picture test cases Co-authored-by: Tony Sullivan --- .changeset/large-bananas-prove.md | 5 +++ .../integrations/image/src/utils/paths.ts | 4 +-- ...ducing-astro.jpg => introducing astro.jpg} | Bin .../basic-picture/src/pages/index.astro | 5 +++ .../image/test/picture-ssg.test.js | 29 +++++++++++++++++- .../image/test/picture-ssr-build.test.js | 14 +++++++++ .../image/test/picture-ssr-dev.test.js | 16 ++++++++++ 7 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 .changeset/large-bananas-prove.md rename packages/integrations/image/test/fixtures/basic-picture/src/assets/blog/{introducing-astro.jpg => introducing astro.jpg} (100%) diff --git a/.changeset/large-bananas-prove.md b/.changeset/large-bananas-prove.md new file mode 100644 index 000000000..da3467950 --- /dev/null +++ b/.changeset/large-bananas-prove.md @@ -0,0 +1,5 @@ +--- +'@astrojs/image': patch +--- + +Fixes a bug that lost query parameters for remote images in the `` component diff --git a/packages/integrations/image/src/utils/paths.ts b/packages/integrations/image/src/utils/paths.ts index cd21da95d..556505704 100644 --- a/packages/integrations/image/src/utils/paths.ts +++ b/packages/integrations/image/src/utils/paths.ts @@ -18,7 +18,7 @@ export function extname(src: string) { return ''; } - return src.substring(src.length - (base.length - index)); + return base.substring(index); } function removeExtname(src: string) { @@ -32,7 +32,7 @@ function removeExtname(src: string) { } function basename(src: string) { - return src.replace(/^.*[\\\/]/, ''); + return removeQueryString(src.replace(/^.*[\\\/]/, '')); } export function propsToFilename(transform: TransformOptions) { diff --git a/packages/integrations/image/test/fixtures/basic-picture/src/assets/blog/introducing-astro.jpg b/packages/integrations/image/test/fixtures/basic-picture/src/assets/blog/introducing astro.jpg similarity index 100% rename from packages/integrations/image/test/fixtures/basic-picture/src/assets/blog/introducing-astro.jpg rename to packages/integrations/image/test/fixtures/basic-picture/src/assets/blog/introducing astro.jpg diff --git a/packages/integrations/image/test/fixtures/basic-picture/src/pages/index.astro b/packages/integrations/image/test/fixtures/basic-picture/src/pages/index.astro index 98aac062d..7cc20d65f 100644 --- a/packages/integrations/image/test/fixtures/basic-picture/src/pages/index.astro +++ b/packages/integrations/image/test/fixtures/basic-picture/src/pages/index.astro @@ -1,5 +1,6 @@ --- import socialJpg from '../assets/social.jpg'; +import introJpg from '../assets/blog/introducing astro.jpg'; import { Picture } from '@astrojs/image/components'; --- @@ -10,6 +11,8 @@ import { Picture } from '@astrojs/image/components';
+ +

@@ -19,5 +22,7 @@ import { Picture } from '@astrojs/image/components';
+
+ diff --git a/packages/integrations/image/test/picture-ssg.test.js b/packages/integrations/image/test/picture-ssg.test.js index a997b57f1..0b99e4933 100644 --- a/packages/integrations/image/test/picture-ssg.test.js +++ b/packages/integrations/image/test/picture-ssg.test.js @@ -30,6 +30,13 @@ describe('SSG pictures - dev', function () { query: { f: 'jpg', w: '506', h: '253' }, alt: 'Social image', }, + { + title: 'Filename with spaces', + id: '#spaces', + url: '/@astroimage/assets/blog/introducing astro.jpg', + query: { f: 'jpg', w: '768', h: '414' }, + alt: 'spaces' + }, { title: 'Inline imports', id: '#inline', @@ -116,6 +123,13 @@ describe('SSG pictures with subpath - dev', function () { query: { f: 'jpg', w: '506', h: '253' }, alt: 'Social image', }, + { + title: 'Filename with spaces', + id: '#spaces', + url: '/@astroimage/assets/blog/introducing astro.jpg', + query: { f: 'jpg', w: '768', h: '414' }, + alt: 'spaces' + }, { title: 'Inline imports', id: '#inline', @@ -199,6 +213,13 @@ describe('SSG pictures - build', function () { size: { width: 506, height: 253, type: 'jpg' }, alt: 'Social image', }, + { + title: 'Filename with spaces', + id: '#spaces', + regex: /^\/assets\/introducing astro.\w{8}_\w{4,10}.jpg/, + size: { width: 768, height: 414, type: 'jpg' }, + alt: 'spaces', + }, { title: 'Inline images', id: '#inline', @@ -244,7 +265,13 @@ describe('SSG pictures - build', function () { const srcset = source.attr('srcset'); for (const src of srcset.split(',')) { - const [pathname, width] = src.split(' '); + const segments = src.split(' '); + + // filenames may have a space in them, pop the last item for the + // width and join the other segments back for the filepath + const width = segments.pop(); + const pathname = segments.join(' '); + const widthNum = parseInt(width.substring(0, width.length - 1)); verifyImage(pathname, { diff --git a/packages/integrations/image/test/picture-ssr-build.test.js b/packages/integrations/image/test/picture-ssr-build.test.js index f70543def..b76585240 100644 --- a/packages/integrations/image/test/picture-ssr-build.test.js +++ b/packages/integrations/image/test/picture-ssr-build.test.js @@ -23,6 +23,13 @@ describe('SSR pictures - build', function () { query: { f: 'jpg', w: '506', h: '253', href: /^\/assets\/social.\w{8}.jpg/ }, alt: 'Social image', }, + { + title: 'Filename with spaces', + id: '#spaces', + url: '/_image', + query: { w: '768', h: '414', f: 'jpg', href: /^\/assets\/introducing astro.\w{8}.jpg/ }, + alt: 'spaces', + }, { title: 'Inline imports', id: '#inline', @@ -127,6 +134,13 @@ describe('SSR pictures with subpath - build', function () { query: { f: 'jpg', w: '506', h: '253', href: /^\/docs\/assets\/social.\w{8}.jpg/ }, alt: 'Social image', }, + { + title: 'Filename with spaces', + id: '#spaces', + url: '/_image', + query: { w: '768', h: '414', f: 'jpg', href: /^\/docs\/assets\/introducing astro.\w{8}.jpg/ }, + alt: 'spaces', + }, { title: 'Inline imports', id: '#inline', diff --git a/packages/integrations/image/test/picture-ssr-dev.test.js b/packages/integrations/image/test/picture-ssr-dev.test.js index 98be066d7..10d8f65cd 100644 --- a/packages/integrations/image/test/picture-ssr-dev.test.js +++ b/packages/integrations/image/test/picture-ssr-dev.test.js @@ -33,6 +33,14 @@ describe('SSR pictures - dev', function () { contentType: 'image/jpeg', alt: 'Social image', }, + { + title: 'Filename with spaces', + id: '#spaces', + url: '/@astroimage/assets/blog/introducing astro.jpg', + query: { w: '768', h: '414', f: 'jpg' }, + contentType: 'image/jpeg', + alt: 'spaces', + }, { title: 'Inline imports', id: '#inline', @@ -154,6 +162,14 @@ describe('SSR pictures with subpath - dev', function () { contentType: 'image/jpeg', alt: 'Social image', }, + { + title: 'Filename with spaces', + id: '#spaces', + url: '/@astroimage/assets/blog/introducing astro.jpg', + query: { w: '768', h: '414', f: 'jpg' }, + contentType: 'image/jpeg', + alt: 'spaces', + }, { title: 'Inline imports', id: '#inline',