fix: correctly enable edge images #1631
Conversation
✅ Deploy Preview for next-hp-edge-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-export-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-edge-middleware ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-canary ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-rsc-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for nextjs-plugin-custom-routes-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
packages/runtime/src/helpers/edge.ts
Outdated
| process.env.NEXT_DISABLE_EDGE_IMAGES !== '1' && | ||
| process.env.NEXT_DISABLE_EDGE_IMAGES !== 'true' && | ||
| process.env.NEXT_DISABLE_NETLIFY_EDGE !== 'true' && | ||
| process.env.NEXT_DISABLE_NETLIFY_EDGE !== '1' |
There was a problem hiding this comment.
[sand] It's a shame I wasn't able to get my IPX work in before these changes because I'm adding the isEnvSet method that we also have in the Gatsby plugin to help make logic like this a little easier on the eyes.
You're free to include the method in your PR if you'd like to use here and I can resolve the conflicts in my branch, but no worries if you're a bit pressed for time and would rather leave this as is for the time being.
There was a problem hiding this comment.
@ericapisani I've updated it to use destr
There was a problem hiding this comment.
@ascorbic Would we rather use this going forward everywhere else? Can be done as a follow up, but we'd likely want to add it to the project board for someone to eventually pick up as an easy win if so
There was a problem hiding this comment.
Yeah, but bear in mind we can't use it inside the patched code
| } | ||
|
|
||
| if (middlewareManifest?.middleware && Object.keys(middlewareManifest.middleware).length !== 0) { | ||
| usingEdge = true | ||
| if (process.env.NEXT_DISABLE_NETLIFY_EDGE === 'true' || process.env.NEXT_DISABLE_NETLIFY_EDGE === '1') { |
There was a problem hiding this comment.
Just realised I accidentally removed the logic in this
Summary
When we changed the logic so that EFs are used by default for middleware, we accidentally chnaged back the logic so that image content negotiation was no longer done with them by default. This PR fixes that, and EFs are used for image content negotiation unless disabled.
Fixes #1648
Test plan
/imagepage in the default demo and ensure that the generated images are not redirected to/_ipxRelevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.