fix: Respect trailing slash config for _next/image route in worker

[sommeeeer]

Should close #877

When you have trailingSlash enabled in next config, the rendered <img>'s href from the <Image> component is with a trailing slash. We need to ensure this is respected in our worker's fallback route for /_next/image.

<img href= Without trailingSlash enabled:
/_next/image?url=%2F_next%2Fstatic%2Fmedia%2Fimage.40ec4ba1.jpeg&w=3840&q=75

<img href= With trailingSlash enabled:
/_next/image/?url=%2F_next%2Fstatic%2Fmedia%2Fimage.40ec4ba1.jpeg&w=3840&q=75

Notice the slash ^

[changeset-bot]

🦋 Changeset detected

Latest commit: 8f9c456

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

[pkg-pr-new]

Open in StackBlitz

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

commit: 8f9c456

[vicb]

LGTM, thanks @sommeeeer !

Would you have time to add an e2e test in i.e. playground15?

[sommeeeer]

Thanks for the reviews! I'll look into adding the E2E later. It will break signal.test.ts in playground15. Also looks like it breaks a lot of the other apps aswell if you enable trailingSlash (e.g. examples/e2e/app-router).

[vicb]

Thanks for the reviews! I'll look into adding the E2E later. It will break signal.test.ts in playground15. Also looks like it breaks a lot of the other apps aswell if you enable trailingSlash (e.g. examples/e2e/app-router).

Interesting, it would be interesting to understand why.

Maybe it's only that the test should be updated - IIUC requests to an url without a trailing slash will be re-directed to the / ending URL and that might break the tests.

But if there is something to change in the adapter, we should create an issue (I don't think it would be high priority to fix that for now).

[sommeeeer]

Interesting, it would be interesting to understand why.

Maybe it's only that the test should be updated - IIUC requests to an url without a trailing slash will be re-directed to the / ending URL and that might break the tests.

But if there is something to change in the adapter, we should create an issue (I don't think it would be high priority to fix that for now).

I added the E2E now. To fix the signal.test.ts I only needed to add an ending / in the revalidatePath() there. This behavior was the same in next start.

When it comes to e2e/app-router I took another look, and your right it turns out the tests needs an update. Also some of the logic inside the middleware. That is expected. I guess we dont have any plans to enable it there anyways. Especially not in this PR.

We do have trailingSlash: true in pages-router and app-pages-router over at AWS BTW.

When you have time, would you mind taking another look on this one? Should be ready to go now.

[vicb]

LGTM, Thanks a lot for the changes and investigating the tests 🙏

I added the E2E now. To fix the signal.test.ts I only needed to add an ending / in the revalidatePath() there. This behavior was the same in next start.

Nice!

When it comes to e2e/app-router I took another look, and your right it turns out the tests needs an update. Also some of the logic inside the middleware. I guess we dont have any plans to enable it there anyways. Especially not in this PR.

Thanks for checking here - great to hear that the code is fine, only the tests would need to be updated.

And yes, we should keep the config of e2e/ apps with aws otherwise it will be much harder to sync the updates.