[@astrojs/image] Handle query params in remote image URLs during SSG builds #4338
Conversation
🦋 Changeset detectedLatest commit: e4ac0c0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
github-actions
bot
added
the
pkg: integration
|
Just confirming that this doesn’t strip them from the requested image, and that example.com/image.png?id=1 doesn’t overwrite or conflict with |
|
@FredKSchott that's correct - the querystring is kept when pulling the original image by URL, it's only stripped off when building the |
| let filename = src.replace(ext, ''); | ||
| let filename = removeQueryString(src); | ||
| const ext = path.extname(filename); | ||
| filename = filename.replace(ext, ''); |
There was a problem hiding this comment.
probably unlikely to happen but this replaces the first substring of the extension so if you had foo.js.bar.js it would become foo.bar.js. Using lastIndexOf to find where the extension is located and then substringing up to that point would be more technically correct I think.
There was a problem hiding this comment.
Great catch, I handled that for stripping the querystring but forgot to move that over to lastIndexOf + substring for the extension
fixed in the latest commit 👍
tony-sull
force-pushed
the
fix/image-ssg-querystring
branch
from
64619a1 to
bf20f96
Compare
Changes
Closes #4317
When building optimized images for SSG builds, any query params used for remote images should be stripped out from the final built HTML
Note that during
astro devand for SSR builds the query params are still needed to make sure the remote image can be fetched properlyTesting
Added
devandbuildtest coverage for remote image query params in SSG and SSRDocs
Bug fix only