From 153f9e2b007b712e40beb532f230d32a649f7310 Mon Sep 17 00:00:00 2001 From: Klein Torres <39591589+kleintorres@users.noreply.github.com> Date: Tue, 13 Dec 2022 05:02:06 +0100 Subject: [PATCH] Fix browser navigation buttons not working with shallow routing and middleware (#43919) This PR fixes #41064. In some particular cases, while using a middleware and shallow routing the navigation get stucks and stop refreshing the page. After futher investigation it seems that a line of code was added that causes the router pathname to be incorrect and then making Next believe it's the same page that is loading. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [x] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm build && pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md) Co-authored-by: JJ Kasper --- packages/next/shared/lib/router/router.ts | 16 ++++--- .../middleware-shallow-link/app/middleware.js | 9 ++++ .../app/pages/index.js | 26 ++++++++++++ .../app/pages/page2.js | 25 +++++++++++ .../e2e/middleware-shallow-link/index.test.ts | 42 +++++++++++++++++++ 5 files changed, 109 insertions(+), 9 deletions(-) create mode 100644 test/e2e/middleware-shallow-link/app/middleware.js create mode 100644 test/e2e/middleware-shallow-link/app/pages/index.js create mode 100644 test/e2e/middleware-shallow-link/app/pages/page2.js create mode 100644 test/e2e/middleware-shallow-link/index.test.ts diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts index 6f718b7d1d26863..cc9e43dd9301b51 100644 --- a/packages/next/shared/lib/router/router.ts +++ b/packages/next/shared/lib/router/router.ts @@ -1456,15 +1456,13 @@ export default class Router implements BaseRouter { // we don't attempt resolve asPath when we need to execute // middleware as the resolving will occur server-side - const isMiddlewareMatch = await matchesMiddleware({ - asPath: as, - locale: nextState.locale, - router: this, - }) - - if (options.shallow && isMiddlewareMatch) { - pathname = this.pathname - } + const isMiddlewareMatch = + !options.shallow && + (await matchesMiddleware({ + asPath: as, + locale: nextState.locale, + router: this, + })) if (isQueryUpdating && isMiddlewareMatch) { shouldResolveHref = false diff --git a/test/e2e/middleware-shallow-link/app/middleware.js b/test/e2e/middleware-shallow-link/app/middleware.js new file mode 100644 index 000000000000000..dd6b55cae5e94fc --- /dev/null +++ b/test/e2e/middleware-shallow-link/app/middleware.js @@ -0,0 +1,9 @@ +import { NextResponse } from 'next/server' + +export async function middleware() { + return NextResponse.next() +} + +export const config = { + matcher: '/(.*$)', +} diff --git a/test/e2e/middleware-shallow-link/app/pages/index.js b/test/e2e/middleware-shallow-link/app/pages/index.js new file mode 100644 index 000000000000000..97b4c0b8699d66b --- /dev/null +++ b/test/e2e/middleware-shallow-link/app/pages/index.js @@ -0,0 +1,26 @@ +import Link from 'next/link' +import React from 'react' + +const Page = () => { + return ( + <> +

Content for page 1

+
+ + Shallow push + +
+
+ + Go to page 2 + +
+ + ) +} + +export default Page diff --git a/test/e2e/middleware-shallow-link/app/pages/page2.js b/test/e2e/middleware-shallow-link/app/pages/page2.js new file mode 100644 index 000000000000000..a8de1b6f668295d --- /dev/null +++ b/test/e2e/middleware-shallow-link/app/pages/page2.js @@ -0,0 +1,25 @@ +import Link from 'next/link' +import React from 'react' + +const Page = () => { + return ( + <> +

Content for page 2

+ + Shallow replace + +
+ +
+ + ) +} + +export default Page diff --git a/test/e2e/middleware-shallow-link/index.test.ts b/test/e2e/middleware-shallow-link/index.test.ts new file mode 100644 index 000000000000000..b97cc5ee955603c --- /dev/null +++ b/test/e2e/middleware-shallow-link/index.test.ts @@ -0,0 +1,42 @@ +import { createNext, FileRef } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import webdriver from 'next-webdriver' +import { join } from 'path' + +describe('browser-shallow-navigation', () => { + let next: NextInstance + + beforeAll(async () => { + next = await createNext({ + files: { + pages: new FileRef(join(__dirname, 'app/pages')), + 'middleware.js': new FileRef(join(__dirname, 'app/middleware.js')), + }, + }) + }) + + afterAll(() => next.destroy()) + + it('should render the correct page', async () => { + const browser = await webdriver(next.url, '/') + + /// do shallow push + await browser.elementByCss('[data-next-shallow-push]').click() + await browser.waitForElementByCss('[data-next-page]') + + // go to another page + await browser.elementByCss('[data-next-page]').click() + await browser.waitForElementByCss('[data-next-shallow-replace]') + + // do shadow replace + await browser.elementByCss('[data-next-shallow-replace]').click() + await browser.waitForElementByCss('[data-go-back]') + + // go back using history api + await browser.elementByCss('[data-go-back]').click() + + // get page h1 + let title = await browser.elementByCss('h1').text() + expect(title).toContain('Content for page 1') + }) +})