Skip to content

Commit

Permalink
parallel routes: fix catch all route support (vercel#58215)
Browse files Browse the repository at this point in the history
This PR fixes a bug where parallel routes would not apply appropriately on navigation when used within slots.

The following scenarios:
```
/foo
   /bar
   /@slot
     /[...catchAll]
```

or 

```
/foo
  /[...catchAll]
  /@slot
     /bar
```

will now function correctly when accessing /foo/bar, and Next.js will render both /bar and the catchall slots.

The issue was that the tree constructed by `next-app-loader` for a given path, /foo/bar in the example, would not include the paths for the catch-all files at build time. The routing was done 1-1 when compiling files, where a path would only match one file, with parallel routes, a path could hit a defined path but also a catch all route at the same time in a different slot.

The fix consists of adding another normalisation layer that will look for all catch-all in `appPaths` and iterate over the other paths and add the relevant information when needed.

The tricky part was making sure that we only included the relevant paths to the loader: we don't want to overwrite a slot with a catch all if there's already a more specific subpath in that slot, i.e. if there's /foo/@slot/bar/page.js, no need to inject /foo/@slot/bar/[...catchAll]. 

One thing that is not supported right now is optional catch all routes, will add later.

fixes vercel#48719
fixes vercel#49662
  • Loading branch information
feedthejim committed Nov 9, 2023
1 parent b740fe8 commit 4024b25
Show file tree
Hide file tree
Showing 7 changed files with 72 additions and 3 deletions.
4 changes: 4 additions & 0 deletions packages/next/src/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
import { isStaticMetadataRouteFile } from '../lib/metadata/is-metadata-route'
import { RouteKind } from '../server/future/route-kind'
import { encodeToBase64 } from './webpack/loaders/utils'
import { normalizeCatchAllRoutes } from './normalize-catchall-routes'

export function sortByPageExts(pageExtensions: string[]) {
return (a: string, b: string) => {
Expand Down Expand Up @@ -545,6 +546,9 @@ export async function createEntrypoints(
)
}

// TODO: find a better place to do this
normalizeCatchAllRoutes(appPathsPerRoute)

// Make sure to sort parallel routes to make the result deterministic.
appPathsPerRoute = Object.fromEntries(
Object.entries(appPathsPerRoute).map(([k, v]) => [k, v.sort()])
Expand Down
53 changes: 53 additions & 0 deletions packages/next/src/build/normalize-catchall-routes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { AppPathnameNormalizer } from '../server/future/normalizers/built/app/app-pathname-normalizer'

/**
* This function will transform the appPaths in order to support catch-all routes and parallel routes.
* It will traverse the appPaths, looking for catch-all routes and try to find parallel routes that could match
* the catch-all. If it finds a match, it will add the catch-all to the parallel route's list of possible routes.
*
* @param appPaths The appPaths to transform
*/
export function normalizeCatchAllRoutes(
appPaths: Record<string, string[]>,
normalizer = new AppPathnameNormalizer()
) {
const catchAllRoutes = [
...new Set(Object.values(appPaths).flat().filter(isCatchAllRoute)),
]

for (const appPath of Object.keys(appPaths)) {
for (const catchAllRoute of catchAllRoutes) {
const normalizedCatchAllRoute = normalizer.normalize(catchAllRoute)
const normalizedCatchAllRouteBasePath = normalizedCatchAllRoute.slice(
0,
normalizedCatchAllRoute.indexOf('[')
)

if (
// first check if the appPath could match the catch-all
appPath.startsWith(normalizedCatchAllRouteBasePath) &&
// then check if there's not already a slot value that could match the catch-all
!appPaths[appPath].some((path) => hasMatchedSlots(path, catchAllRoute))
) {
appPaths[appPath].push(catchAllRoute)
}
}
}
}

function hasMatchedSlots(path1: string, path2: string): boolean {
const slots1 = path1.split('/').filter((segment) => segment.startsWith('@'))
const slots2 = path2.split('/').filter((segment) => segment.startsWith('@'))

if (slots1.length !== slots2.length) return false

for (let i = 0; i < slots1.length; i++) {
if (slots1[i] !== slots2[i]) return false
}

return true
}

function isCatchAllRoute(pathname: string): boolean {
return pathname.includes('[...') || pathname.includes('[[...')
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { RouteKind } from '../../route-kind'
import { FileCacheRouteMatcherProvider } from './file-cache-route-matcher-provider'

import { DevAppNormalizers } from '../../normalizers/built/app'
import { normalizeCatchAllRoutes } from '../../../../build/normalize-catchall-routes'

export class DevAppPageRouteMatcherProvider extends FileCacheRouteMatcherProvider<AppPageRouteMatcher> {
private readonly expression: RegExp
Expand Down Expand Up @@ -56,6 +57,8 @@ export class DevAppPageRouteMatcherProvider extends FileCacheRouteMatcherProvide
else appPaths[pathname] = [page]
}

normalizeCatchAllRoutes(appPaths)

const matchers: Array<AppPageRouteMatcher> = []
for (const filename of routeFilenames) {
// Grab the cached values (and the appPaths).
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Baz() {
return 'baz slot'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Foo() {
return 'bar slot'
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ export default function Layout({ children, slot }) {
<div>
<Link href="/parallel-catchall/bar">catchall bar</Link>
</div>
<div>
<Link href="/parallel-catchall/baz">catchall baz</Link>
</div>
</div>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ createNextDescribe(
await browser.elementByCss('[href="/parallel-catchall/bar"]').click()
await check(
() => browser.waitForElementByCss('#main').text(),
'main catchall'
'bar slot'
)
await check(
() => browser.waitForElementByCss('#slot-content').text(),
Expand All @@ -328,14 +328,14 @@ createNextDescribe(
'foo slot'
)

await browser.elementByCss('[href="/parallel-catchall/bar"]').click()
await browser.elementByCss('[href="/parallel-catchall/baz"]').click()
await check(
() => browser.waitForElementByCss('#main').text(),
'main catchall'
)
await check(
() => browser.waitForElementByCss('#slot-content').text(),
'slot catchall'
'baz slot'
)
})

Expand Down

0 comments on commit 4024b25

Please sign in to comment.