Invariant: attempted to hard navigate to the same URL (Rewrites inside next.config.mjs)

[grayaustinc]

Verify canary release

• I verified that the issue exists in the latest Next.js canary release

Provide environment information

Deployed to Vercel

      Platform: win32
      Arch: x64
      Version: Windows 10 Pro
    Binaries:
      Node: 16.13.1
      npm: N/A
      Yarn: N/A
      pnpm: N/A
    Relevant packages:
      next: 12.2.6-canary.0
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0

What browser are you using? (if relevant)

Not relevant

How are you deploying your application? (if relevant)

Vercel

Describe the Bug

When creating rewrites from next.config.mjs, that include a trailing slash in the source, and/or rewriting from a source to a destination with a slug { source: "/test/", destination: "/test/1/" } will cause errors in production that will cause the page to reload and throws an error.

Errors:
Failed to lookup route: /
The provided 'as' value (/test/) is incompatible with the 'href' value (/test/[slug])
Invariant: attempted to hard navigate to the same URL /

On the minimal reproduction, errors can be seen when clicking on the home button twice. For the first group of links 1,3,4 (links 2, 2.5 apparently work because /test.js file actually exists) It appears as though a root index file is needed in a folder that has a dynamic route in order to prevent the errors from occurring even though the path should be getting rewritten to the dynamic destination.

The second "fix" is from removing the trailing slash from the source. I have no idea why removing the trailing slash from the rewrite source prevents the errors above from happening but I wouldn't really call that a solution to the problem as you can't really remove the trailing slash from the home page '/'. You can see that removing the trailing slash does not throw errors in the reproduction with the second group of links.

In development, the network tab shows that the router is requesting the wrong JavaScript file for the page and seems to be ignoring the rewrite from the manifest (Cannot be seen in Vercel reproduction).
I believe whatever is causing this bug is the same bug for #38171 as it follows a similar premise using rewrites and in development, an extra page.js fetch can be seen before the page reload with a Failed to load script: /_next/static/chunks/pages/test.js error.

Expected Behavior

Should work the same way as 12.1.4 where rewrites do not throw errors on page load.

Link to reproduction

https://next-12-2-4-rewrite-bug.vercel.app

To Reproduce

Minimal reproduction repo: https://github.com/grayaustinc/next-12.2.4-rewrite-bug (is now on version: next@canary)
Create rewrites in next.config.mjs where the source does not contain the slug (with a trailing slash) and the destination with the slug included.
{ source: "/test/", destination: "/test/1/" }
More simply, a source/destination for the home page
{ source: "/", destination: "/home/" }

[ijjk]

Hi, it looks like the above reproduction is 404ing, please update it so we can take a look!

[grayaustinc]

Hi, it looks like the above reproduction is 404ing, please update it so we can take a look!

Should be fixed now, was set to private

[msampsontruecar]

I've been seeing the same issue. I tracked it down to this change made in 12.2.1.
https://github.com/vercel/next.js/pull/38282/files#diff-c63f939297887175342bae28f4c0e69f1e6b10149cb43795390356d1d771a6f2R38

Adding (/)? to the end of the source pattern if config is set for trailingSlash: true

    const matcher = getPathMatch(
      rewrite.source + (process.env.__NEXT_TRAILING_SLASH ? '(/)?' : ''),
      {
        removeUnnamedParams: true,
        strict: true,
      }
    )

Reverting the change would fix it for us but we would reintroduce the issue for people who don't have trailing slashes on the end of their rewrite sources and have trailingSlash: true.

I'll have to look into it more but there may be some issue with using (/)? with pathToRegexp since the delimiter is set to /.

One possible solution that I think covers all use cases and avoids adding extra pattern matching. Although I'm not sure it is safe to make assumptions about what a rewrite source will end with so more testing is needed.

    const matcher = getPathMatch(
      !rewrite.source.endsWith('/') && process.env.__NEXT_TRAILING_SLASH
        ? rewrite.source + '/'
        : rewrite.source.endsWith('/') && !process.env.__NEXT_TRAILING_SLASH
        ? removeTrailingSlash(rewrite.source)
        : rewrite.source,
      {
        removeUnnamedParams: true,
        strict: true,
      }
    )

If no one gets to it before me I can investigate further and work on a PR next week.

[grayaustinc]

I think your onto something as I've also just looked into it using what you've found but I have yet to determine if (/)? is what is actually causing the problem.

if my path is /test/ and the regex being passed into the getPathMatch is /test/(/)? simply testing the regex will return true. I believe the problem lies somewhere inside the getPathMatch function, or more specifically the pathToRegexp function.
printing out the returned value for pathToRegexp shows that the above regex is turned into /^/test(?:/(/))?$/i which means that the matching pathnames HAS to be /test// with an extra slash in order to match.

const matcher = getPathMatch(removeTrailingSlash(rewrite.source) + (process.env.__NEXT_TRAILING_SLASH ? '(/?)' : ''), {
     removeUnnamedParams: true,
     strict: true
});

I believe this is the correct change. I've tried this change in all my repositories and they have functioned as they were suppose to. I will run a few more tests but I believe this is equivalent to the solution that you have.

[msampsontruecar]

I originally tried the same solution of removeTrailingSlash on rewrite.source. I just tested it again and it doesn't work for me. I still get the error in the case where the rewrite source ends in a trailing slash and trailingSlash: true. I don't think I ever got that case working if (/)? is included in the pattern match.

[msampsontruecar]

I looked into path-to-regex that getPathMatch uses.
strict When true the regexp won't allow an optional trailing delimiter to match. (default: false)
So we have the strict option set as true which seems to conflict with our attempts to pattern match an optional trailing slash. Which probably causes some unexpected behavior.

[grayaustinc]

I actually just tried it without the removeTrailingSlash from my example and it worked just the same. Did you try with the matching inside the parenthesis? (/?) it seems that putting the ? inside of the parenthesis gets around the strict matching. I see that you put in a pull request, but wanted to see if this change works for your examples as well. I don't know what kind of side effects there could be on other projects/examples when changing from strict: true to strict: false

strict: false change does work for my examples

[msampsontruecar]

Thanks for pointing out that you moved the ? inside the parens. I didn't notice that change. I just tried your example out (with and without the removeTrailingSlash. I'm not able to get it to work in all cases either way. I'm hoping when a code owner reviews my PR they can shine some light on why strict was added in the first place.

[msampsontruecar]

@grayaustinc I just pushed a unit test to my PR that can be used to test that these changes cover all cases. You can use that test to verify potential fixes. You may have to set process.env.__NEXT_TRAILING_SLASH in the test if your solution relies on that.

[msampsontruecar]

@grayaustinc after some more testing I got your solution to work with one small change.

    const matcher = getPathMatch(
      removeTrailingSlash(rewrite.source) +
        (process.env.__NEXT_TRAILING_SLASH === 'true' ? '(/?)' : ''),
      {
        removeUnnamedParams: true,
        strict: true,
      }
    )

Since process.env.__NEXT_TRAILING_SLASH is a string it could be undefined or "false".

[grayaustinc]

@msampsontruecar That's surprising to me that the env variable would work like that as even in the current canary the env variable is used as a boolean.

All of my cases work with the changes you made above with process.env.__NEXT_TRAILING_SLASH === 'true' so that seems to be the best solution. My cases also work with or without the removeTrailingSlash function included.

[grayaustinc]

Updated the reproduction to use Next 13 and continue to have same issues.
A temporary fix is to add {/}? to end of sources (As explained on issue #40549 ).

[phatify]

I am facing the same issue with Next.js 13 latest when redirect to another page not found in 404 route. This error still exists till now with no solution :(