Skip to content

Commit

Permalink
Fix browser navigation buttons not working with shallow routing and m…
Browse files Browse the repository at this point in the history
…iddleware (#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 <jj@jjsweb.site>
  • Loading branch information
kleintorres and ijjk committed Dec 13, 2022
1 parent 43680e3 commit 153f9e2
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 9 deletions.
16 changes: 7 additions & 9 deletions packages/next/shared/lib/router/router.ts
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions 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: '/(.*$)',
}
26 changes: 26 additions & 0 deletions 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 (
<>
<h1>Content for page 1</h1>
<div>
<Link
data-next-shallow-push
shallow
href={{ query: { params: 'testParams' } }}
>
Shallow push
</Link>
</div>
<div>
<Link data-next-page href="/page2">
Go to page 2
</Link>
</div>
</>
)
}

export default Page
25 changes: 25 additions & 0 deletions 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 (
<>
<h1>Content for page 2</h1>
<Link
data-next-shallow-replace
replace
shallow
href={{ query: { params: 'testParams' } }}
>
Shallow replace
</Link>
<div>
<button data-go-back onClick={() => window.history.back()}>
Go Back
</button>
</div>
</>
)
}

export default Page
42 changes: 42 additions & 0 deletions 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')
})
})

0 comments on commit 153f9e2

Please sign in to comment.