Fix fallback false

[conico974]

Dynamic route with fallback false can wrongly intercept and return 404 for routes that intersect with them.

Fix #677 and opennextjs/opennextjs-cloudflare#611

[pkg-pr-new]

Open in StackBlitz

pnpm add https://pkg.pr.new/@opennextjs/aws@858

commit: ccdb493

[vicb]

could that cause issue with minification?

[conico974]

I'll need to test it. Not sure what we can use if we can't use that, i don't think we can use instanceof

[conico974]

It does work in cloudflare in preview

[khuezy]

If this is a concern, there is a keep_classname option in the webpack config.

[changeset-bot]

🦋 Changeset detected

Latest commit: ccdb493

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

This PR includes changesets to release 3 packages

Name	Type
@opennextjs/aws	Patch
app-pages-router	Patch
app-router	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

[lauri865]

App router's export const dynamicParams = false; seems to be broken still :(

[sommeeeer]

App router's export const dynamicParams = false; seems to be broken still :(

what version of OpenNext are you on? when you say broken, what do you mean?

[lauri865]

Cloudflare@latest (=1.0.4)

Same issue as the referenced issue:
opennextjs/opennextjs-cloudflare#611

[lauri865]

Sorry, correction, the original issue seems to be fixed. Repro doesn't reproduce it anymore, but we seem to be hitting a new issue it seems (fallback rewrite doesn't get triggered, and we end up with 404 in a different scenario). I will see if I can create a new reproduction soon.

[sommeeeer]

Sorry, correction, the original issue seems to be fixed. Repro doesn't reproduce it anymore, but we seem to be hitting a new issue it seems (fallback rewrite doesn't get triggered, and we end up with 404 in a different scenario). I will see if I can create a new reproduction soon.

@lauri865 Alright. The reproduction you linked here doesn't have any dynamicParams in it. It seems to be missing a commit.

[lauri865]

Sorry, correction, the original issue seems to be fixed. Repro doesn't reproduce it anymore, but we seem to be hitting a new issue it seems (fallback rewrite doesn't get triggered, and we end up with 404 in a different scenario). I will see if I can create a new reproduction soon.

@lauri865 Alright. The reproduction you linked here doesn't have any dynamicParams in it. It seems to be missing a commit.

I corrected that in the meanwhile as well, was missing a commit indeed. Thanks for pointing out.

[lauri865]

New reproduction committed: https://github.com/lauri865/opennext-routes-repro

pnpm dev / start work just fine (shows this Github issue when you navigate to any non-existing page, e.g. http://localhost:3000/abc123

If export const dynamicParams = false; is present:
pnpm preview shows 404 also for static path /test that should exist, and the fallback rewrite in next.config doesn't work for any non-existing path.

So, it seems like it's impossible to opt out from dynamicParams, and if you try to do that, it breaks static params as well.

[conico974]

@lauri865 You need an incrementalCache in your open-next.config.ts

[lauri865]

We havestaticAssetsIncrementalCache in our production config, but facing the same issue. Do we need something else? Our site us fully static.

Updated that repro with staticAssetsIncrementalCache and it has the same issue.

[conico974]

@lauri865 Create an issue with a link to the repro, then

[lauri865]

@conico974, I did, it's referenced as fixed by this PR. Which it seems to be not. Should I open another issue?

[conico974]

@lauri865 Yeah, that's not the same issue:

Sorry, correction, the original issue seems to be fixed. Repro doesn't reproduce it anymore

[lauri865]

Opened a new issue here with a reproduction: opennextjs/opennextjs-cloudflare#682