Skip to content

Conversation

sommeeeer
Copy link
Collaborator

@sommeeeer sommeeeer commented Jun 19, 2025

fixes #731

Copy link

changeset-bot bot commented Jun 19, 2025

🦋 Changeset detected

Latest commit: 1316689

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@opennextjs/cloudflare Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

pkg-pr-new bot commented Jun 19, 2025

Open in StackBlitz

npm i https://pkg.pr.new/@opennextjs/cloudflare@740

commit: 1316689

Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

Thanks - you are on fire 🔥

I think there might be a bug when there is no local pattern?

@vicb
Copy link
Contributor

vicb commented Jun 19, 2025

image

not a 🦊 ??

@vicb
Copy link
Contributor

vicb commented Jun 19, 2025

Also would be nice to add tests from https://github.com/vercel/next.js/blob/canary/test/unit/image-optimizer/match-local-pattern.test.ts in images.spec.ts as we have for remote patterns.
Thx

@sommeeeer
Copy link
Collaborator Author

sommeeeer commented Jun 19, 2025

Alright, so I added a fix commit. Sorry about the confusion!

edit2: I guess we wrap pathname and hostname with makeRe() in our test cause Next uses the same when writing the manifest here? However, why does Next use it again inside matchRemotePattern() and matchLocalPattern()? They are also using the arrays from the config.

Say Hello to Snipp! She is the cutest most active cat on this planet. Unfortunently she got sold to a happy customer in London! I hope she is doing fine there (my family owns a cat breeding business 🐈).

@vicb
Copy link
Contributor

vicb commented Jun 20, 2025

However, why does Next use it again inside matchRemotePattern() and matchLocalPattern()?

Those methods are call from the image optimizer that passes in the next config and not the manifest

Snipp is so lovely 🥰

Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

A couple more nits and should be good to 🚀

@sommeeeer sommeeeer requested a review from vicb June 20, 2025 06:46
Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!

@vicb vicb merged commit d8150d2 into opennextjs:main Jun 20, 2025
7 checks passed
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.

implement local patterns for images

2 participants