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

feat: add support for query param url syntax of IMAGE CDN #566

Merged
merged 12 commits into from
Mar 10, 2023

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Feb 23, 2023

Summary

gatsbyjs/gatsby#35160 changed the URL structure for image cdn. This PRs add support for new structure:

  1. ODBs are cached just on path ignoring query params. New syntax allows for multiple images sharing same path and differ just with query params. To support it, permanent redirect is added that moves query params into path. That redirect doesn't match old structure and I don't think there's a way to match it there
  2. The added redirect doesn't match the older structure (I don't think it's possible to actually match it with existing API given some encoding changes need to happen), so IPX handler wrapper now also converts event to match old structure. This means no changes are needed in @netlify/ipx. (I'm yet to figure out if this will make it possible to hit problems that caused Gatsby to change the url structure, but I'm hopeful it could be fine, because most references I found related to fs limitation when using IMAGE CDN api without actual CDN support for it, as Gatsby treats those then as local image processing jobs)

Test plan

  1. Visit the Deploy Preview (image-cdn demo page)

Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal

Fixes https://github.com/netlify/pod-ecosystem-frameworks/issues/389

Standard checks:

  • Check the Deploy Preview's Demo site for your PR's functionality
  • Add docs when necessary

🧪 Once merged, make sure to update the version if needed and that it was
published correctly.

@netlify
Copy link

netlify bot commented Feb 23, 2023

Deploy Preview for netlify-plugin-gatsby-demo ready!

Name Link
🔨 Latest commit 20f8eed
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-gatsby-demo/deploys/640b533fe153f8000884b750
😎 Deploy Preview https://deploy-preview-566--netlify-plugin-gatsby-demo.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Feb 24, 2023

Deploy Preview for netlify-plugin-gatsby-demo-v5 ready!

Name Link
🔨 Latest commit 20f8eed
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-gatsby-demo-v5/deploys/640b533f54a11400089391fb
😎 Deploy Preview https://deploy-preview-566--netlify-plugin-gatsby-demo-v5.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@pieh pieh marked this pull request as ready for review March 3, 2023 08:29
@pieh pieh requested a review from a team March 3, 2023 08:29
Copy link
Contributor

@sarahetter sarahetter left a comment

Choose a reason for hiding this comment

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

Looks great! Thank you!

@kodiakhq kodiakhq bot merged commit 5874f67 into main Mar 10, 2023
@kodiakhq kodiakhq bot deleted the fix/gatsby-5-image-cdn branch March 10, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants