Skip to content

Commit

Permalink
Update router filter to be less sensitive (#46515)
Browse files Browse the repository at this point in the history
This skips including dynamic redirects in the client router filter as it
causes extra unexpected hard navigations.

x-ref: [slack
thread](https://vercel.slack.com/archives/C04MEB9L9RQ/p1677521159911179?thread_ts=1677255545.574209&cid=C04MEB9L9RQ)

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)
  • Loading branch information
ijjk committed Feb 28, 2023
1 parent 9ad1f32 commit 82f4fd3
Show file tree
Hide file tree
Showing 6 changed files with 13 additions and 25 deletions.
2 changes: 1 addition & 1 deletion packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ import { webpackBuild } from './webpack-build'
import { NextBuildContext } from './build-context'
import { normalizePathSep } from '../shared/lib/page-path/normalize-path-sep'
import { isAppRouteRoute } from '../lib/is-app-route-route'
import { createClientRouterFilter } from '../lib/create-router-client-filter'
import { createClientRouterFilter } from '../lib/create-client-router-filter'

export type SsgRoute = {
initialRevalidateSeconds: number | false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { Token } from 'next/dist/compiled/path-to-regexp'
import { BloomFilter } from '../shared/lib/bloom-filter'
import { isDynamicRoute } from '../shared/lib/router/utils'
import { removeTrailingSlash } from '../shared/lib/router/utils/remove-trailing-slash'
import { Redirect } from './load-custom-routes'
import { tryToParsePath } from './try-to-parse-path'

const POTENTIAL_ERROR_RATE = 0.02

Expand Down Expand Up @@ -42,26 +44,14 @@ export function createClientRouterFilter(
for (const redirect of redirects) {
const { source } = redirect
const path = removeTrailingSlash(source)
let tokens: Token[] = []

if (path.includes(':') || path.includes('(')) {
let subPath = ''
const pathParts = path.split('/')

for (let i = 1; i < pathParts.length + 1; i++) {
const curPart = pathParts[i]

if (curPart.includes(':') || curPart.includes('(')) {
break
}
subPath = `${subPath}/${curPart}`
}
try {
tokens = tryToParsePath(source).tokens || []
} catch (_) {}

// if redirect has matcher at top-level we don't include this
// as it would match everything
if (subPath) {
dynamicPaths.add(subPath)
}
} else {
if (tokens.every((token) => typeof token === 'string')) {
// only include static redirects initially
staticPaths.add(path)
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ import { CachedFileReader } from '../future/route-matcher-providers/dev/helpers/
import { DefaultFileReader } from '../future/route-matcher-providers/dev/helpers/file-reader/default-file-reader'
import { NextBuildContext } from '../../build/build-context'
import { logAppDirError } from './log-app-dir-error'
import { createClientRouterFilter } from '../../lib/create-router-client-filter'
import { createClientRouterFilter } from '../../lib/create-client-router-filter'

// Load ReactDevOverlay only when needed
let ReactDevOverlayImpl: FunctionComponent
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/shared/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,7 @@ export default class Router implements BaseRouter {
new URL(curAs, 'http://n').pathname
)
const asNoSlashLocale = addBasePath(
addLocale(curAs, locale || this.locale)
addLocale(asNoSlash, locale || this.locale)
)

if (
Expand Down
2 changes: 0 additions & 2 deletions test/e2e/app-dir/app/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,8 @@ createNextDescribe(
{ pathname: '/redirect-1' },
{ pathname: '/redirect-2' },
{ pathname: '/blog/old-post' },
{ pathname: '/redirect-3/some/value' },
{ pathname: '/redirect-3/some' },
{ pathname: '/redirect-4' },
{ pathname: '/redirect-4/another' },
])(
'should match redirects in pages correctly $path',
async ({ pathname }) => {
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/app-dir/app/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ module.exports = {
permanent: false,
},
{
source: '/redirect-3/some/:path*',
source: '/redirect-3/some',
destination: 'https://example.vercel.sh',
permanent: false,
},
{
source: '/redirect-4/:path*',
source: '/redirect-4',
destination: 'https://example.vercel.sh',
permanent: false,
},
Expand Down

0 comments on commit 82f4fd3

Please sign in to comment.