chore: update image domain handling in next config - #1980
Merged
Conversation
Collaborator
Author
|
Ah, okay.. eslint complains about not having them. Go figure lol. I'll get them placed back in. |
This reverts commit 1d8f6b5.
Collaborator
Author
|
I’m going to go ahead and merge this. Casper, I saw your reaction, so I’m assuming/hoping that’s an approval? 😂 We can always revert if needed, but there have been a few requests for this fix. Let me know if anything needs to change. |
Member
|
Thanks for this PR @whitigol! The code looks good, I'll make sure to test it out as well ASAP. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
Many users were experiencing images not loading. This was caused by Next.js deprecating
images.domainsin favor ofimages.remotePatterns. Our script was still written for the old API (adding a domain string via string replace), which no longer matches the currentnext.config.mjsstructure and could break or fail to add the deployment origin.Changes
scripts/create-images-domain.mjsimages.remotePatterns.protocol,hostname,pathname: "**") derived fromNEXT_PUBLIC_PROD_ORIGINinstead of appending a raw domain string.remotePatterns, the script exits without changing the config.remotePatternsarray and inserts the new entry before the closing]instead of using a fragile string replace.package.jsonnode scripts/create-images-domain.mjsbefore the Turbo build so the client’s Next config is updated with the correct image remote pattern for the deployment origin before building.No breaking changes
Formatted with Prettier
@casperiv0, sorry for the @, I know you're a busy guy! But,
I noticed in 1d8f6b5, it removed many parenthesized expressions that wrapped some nullish coalescing operators. I know it's best practice to wrap these in parenthesized expressions, but eslint/prettier complains about it and sees it as unnecessary. I briefly look through, and I personally don't see anything that could affect behavior, but I could be wrong. Let me know if you'd prefer to keep the parentheses and skip a full codebase format, or if you're okay with prettier removing them.EDIT: I reverted the format commit resulting in eslint errors. Let me know if this is good to merge 🙂