Skip to content

Commit

Permalink
Fix client filter case with redirects (#46317)
Browse files Browse the repository at this point in the history
Follow-up to #46283 this ensures we properly handle redirects with matchers at the base and we don't use the normalized redirects when generating the filter as the breaks with i18n. 

x-ref: [slack thread](https://vercel.slack.com/archives/C040J1CGLHH/p1677028542232619)

## Bug

- [x] Related issues linked using `fixes #number`
- [x] 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 authored Feb 23, 2023
1 parent 4eb34ef commit 8ff3d7f
Show file tree
Hide file tree
Showing 9 changed files with 40 additions and 13 deletions.
4 changes: 2 additions & 2 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -842,8 +842,8 @@ export default async function build(
]

if (config.experimental.clientRouterFilter) {
const nonInternalRedirects = redirects.filter(
(redir) => !(redir as any).internal
const nonInternalRedirects = (config._originalRedirects || []).filter(
(r: any) => !r.internal
)
const clientRouterFilters = createClientRouterFilter(
appPageKeys,
Expand Down
11 changes: 9 additions & 2 deletions packages/next/src/lib/create-router-client-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ export function createClientRouterFilter(
}
subPath = `${subPath}/${curPart}`
}
dynamicPaths.add(subPath)

if (subPath) {
dynamicPaths.add(subPath)
}
} else {
staticPaths.add(path)
}
Expand All @@ -53,7 +56,11 @@ export function createClientRouterFilter(
subPath = `${subPath}/${curPart}`
}

dynamicPaths.add(subPath)
// if redirect has matcher at top-level we don't include this
// as it would match everything
if (subPath) {
dynamicPaths.add(subPath)
}
} else {
staticPaths.add(path)
}
Expand Down
4 changes: 4 additions & 0 deletions packages/next/src/lib/load-custom-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,10 @@ async function loadRedirects(config: NextConfig) {
// they are still valid
checkCustomRoutes(redirects, 'redirect')

// save original redirects before transforms
if (Array.isArray(redirects)) {
;(config as any)._originalRedirects = redirects.map((r) => ({ ...r }))
}
redirects = processRoutes(redirects, config, 'redirect')
checkCustomRoutes(redirects, 'redirect')
return redirects
Expand Down
4 changes: 3 additions & 1 deletion packages/next/src/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,9 @@ export default class DevServer extends Server {
if (this.nextConfig.experimental.clientRouterFilter) {
clientRouterFilters = createClientRouterFilter(
Object.keys(appPaths),
this.customRoutes.redirects.filter((r) => !(r as any).internal)
((this.nextConfig as any)._originalRedirects || []).filter(
(r: any) => !r.internal
)
)

if (
Expand Down
7 changes: 5 additions & 2 deletions packages/next/src/shared/lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,7 @@ export default class Router implements BaseRouter {
const isQueryUpdating = (options as any)._h === 1

if (process.env.__NEXT_CLIENT_ROUTER_FILTER_ENABLED) {
const asNoSlash = removeTrailingSlash(as)
const asNoSlash = removeTrailingSlash(new URL(as, 'http://n').pathname)
const matchesBflStatic = this._bfl_s?.has(asNoSlash)
let matchesBflDynamic = false
const asNoSlashParts = asNoSlash.split('/')
Expand All @@ -1087,7 +1087,10 @@ export default class Router implements BaseRouter {
// if the client router filter is matched then we trigger
// a hard navigation
if (!isQueryUpdating && (matchesBflStatic || matchesBflDynamic)) {
handleHardNavigation({ url: as, router: this })
handleHardNavigation({
url: addBasePath(addLocale(as, options.locale || this.locale)),
router: this,
})
return false
}
}
Expand Down
5 changes: 5 additions & 0 deletions test/e2e/app-dir/app/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ module.exports = {
destination: 'https://example.vercel.sh',
permanent: false,
},
{
source: '/:path/to-redirect',
destination: 'https://example.vercel.sh',
permanent: false,
},
]
},
}
3 changes: 3 additions & 0 deletions test/e2e/middleware-redirects/app/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ module.exports = {
locales: ['en', 'fr', 'nl', 'es'],
defaultLocale: 'en',
},
experimental: {
clientRouterFilter: true,
},
redirects() {
return [
{
Expand Down
10 changes: 6 additions & 4 deletions test/e2e/middleware-redirects/app/pages/dynamic/[slug].js
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
export default function Account() {
export default function Account({ slug }) {
return (
<p id="dynamic" className="title">
Welcome to a /dynamic/[slug]
Welcome to a /dynamic/[slug]: {slug}
</p>
)
}

export function getServerSideProps() {
export function getServerSideProps({ params }) {
return {
props: {},
props: {
slug: params.slug,
},
}
}
5 changes: 3 additions & 2 deletions test/e2e/middleware-redirects/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ describe('Middleware Redirect', () => {
function tests() {
it('should redirect correctly with redirect in next.config.js', async () => {
const browser = await webdriver(next.url, '/')
await browser.eval('window.beforeNav = 1')
await browser.eval('window.next.router.push("/to-new")')
await browser.waitForElementByCss('#dynamic')
expect(await browser.eval('window.beforeNav')).toBe(1)
expect(await browser.elementByCss('#dynamic').text()).toBe(
'Welcome to a /dynamic/[slug]: new'
)
})

it('does not include the locale in redirects by default', async () => {
Expand Down

0 comments on commit 8ff3d7f

Please sign in to comment.