Skip to content

fix(static): url encoded name included in generated ext - #340

Merged
pi0 merged 4 commits into
nuxt:mainfrom
btkostner:filename-format
Jun 29, 2021
Merged

fix(static): url encoded name included in generated ext#340
pi0 merged 4 commits into
nuxt:mainfrom
btkostner:filename-format

Conversation

@btkostner

@btkostner btkostner commented Jun 25, 2021

Copy link
Copy Markdown
Contributor

This fixes an issue with the static provider on remote domains including query params in the nuxt generate filename outputs.

Example

Say I have domains: ['https://images.prismic.io'] in my nuxt config and passing an src like https://images.prismic.io/blog-system76/30e9de39-20a6-4caf-8728-0ec917942a0f_tumblr_4fef1ab602684355ef9596a0b9fdb122_810bfad7_1280.png?auto=compress,format&rect=56,0,700,700&w=800&h=800, the generated nuxt image file path would end up looking like dist/_nuxt/image/181d93.png%3Fauto=compress,format&rect=56,0,700,700&w=800&h=800.

This PR changes it to output dist/_nuxt/image/181d93.png by stripping anything after %3F which is the url encoded ?

@codecov-commenter

codecov-commenter commented Jun 25, 2021

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.81%. Comparing base (d9aeae2) to head (b44d891).
⚠️ Report is 970 commits behind head on main.

Files with missing lines Patch % Lines
src/utils.ts 0.00% 5 Missing ⚠️
src/generate.ts 0.00% 3 Missing ⚠️
src/runtime/image.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #340      +/-   ##
==========================================
- Coverage   59.36%   58.81%   -0.56%     
==========================================
  Files          26       26              
  Lines         598      607       +9     
  Branches      149      190      +41     
==========================================
+ Hits          355      357       +2     
- Misses        243      250       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@btkostner

Copy link
Copy Markdown
Contributor Author

An alternative solution for this is to loop over a list of known good formats ('.png', .jpg') and use '.startsWith()' on the ext string.

@pi0

pi0 commented Jun 29, 2021

Copy link
Copy Markdown
Member

Thanks for PR @btkostner. I've added a refactor to pass original src (input) to mapToStatic that we can use extracting name and ext before passed to provider without encoding issues. Also added a guessExt utility partially implementing your second idea to validate guessed ext against a simple regex.

@pi0 pi0 changed the title fix: url encoded name included in generated ext fix(static): url encoded name included in generated ext Jun 29, 2021
@btkostner

Copy link
Copy Markdown
Contributor Author

@pi0 Looks good! I wasn't sure how to add a test for this, but doing npm link confirms that this fixes my issue 🎊

@pi0
pi0 merged commit 58dd744 into nuxt:main Jun 29, 2021
@pi0

pi0 commented Jun 29, 2021

Copy link
Copy Markdown
Member

Thanks for confarming. Will trigger release after adding few more fixes.

@btkostner
btkostner deleted the filename-format branch June 29, 2021 17:46
procrates pushed a commit to procrates/nuxt-image that referenced this pull request Feb 21, 2023
Co-authored-by: Pooya Parsa <pyapar@gmail.com>
@github-actions github-actions Bot mentioned this pull request Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants