Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix IPX static generation issue when using multiple format options #673

Closed

Conversation

jimhlad
Copy link
Contributor

@jimhlad jimhlad commented Dec 3, 2022

This fixes an issue with the IPX provider when multiple formatting options are applied.

Modified playground sample data as a minimal reproduction.

To test:

  1. Make the change to playground/providers.ts shown here: https://github.com/nuxt/image/pull/673/files#diff-cdfcb4bb4c632950a1a834b483f874c9c3f5090efb60a1467e9e994403361276
  2. Run npm run dev:generate and npx serve playground/.output/public
  3. Observe colors image does not load on IPX page
  4. Now apply the changes from this PR and run npm run dev:generate and npx serve playground/.output/public
  5. Color image now loads

@jimhlad jimhlad changed the title Fix IPX issue with multiple format options Fix IPX static generation issue when using multiple format options Dec 3, 2022
@marvincaspar
Copy link

Thanks, this fixes my issue with not generating all my image sizes. Hope that it will be merged soon

@jimhlad
Copy link
Contributor Author

jimhlad commented Dec 9, 2022

@pi0 For your review

@@ -12,7 +12,7 @@ const operationsGenerator = createOperationsGenerator({
quality: 'q',
background: 'b'
},
joinWith: ',',
joinWith: '_',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this breaks placeholder generation for IPX as it generates the following URL when using the placeholder attribute:

/_ipx/q_50,s_10x10/images/slightly_smiling_face_3d.png

which is a valid IPX URL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's possible. If there's a better solution feel free to close this out.

Note: In addition to fixing the described bug, this PR also made it so that the downloaded directory names don't have commas or ampersands in them. Not sure if there's a better way to accomplish this, but to me it feels a bit hacky to have these characters in folder names. Underscores or hyphens seem cleaner. Also not sure if there are any SEO implications to consider here.

@pi0
Copy link
Member

pi0 commented Jan 27, 2023

This should be fixed by #725. Please ping me to reopen if not. Also thanks for PRs ❤️

@pi0 pi0 closed this Jan 27, 2023
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.

4 participants