Skip to content

Commit 5f95b6b

Browse files
shudingtimneutkensijjk
authored
Improved route resolution in next-app-loader (vercel#40109)
This PR implements the logic to make next-app-loader able to match multiple routes. The app loader is refactored to construct the tree recursively instead of within a loop, as there could be multiple branches. Similarly, when entering a new layout level or branch, we resolve both the slot name (defaults to `"children"`) and the segment. In order to make that work, the loader has to know all matched app paths. This is passed in as the `appPaths` loader option, which is gathered when creating the entrypoint. ## Bug - [ ] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have helpful link attached, see `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` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have helpful link attached, see `contributing.md` ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm lint` - [ ] The examples guidelines are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing.md#adding-examples) Co-authored-by: Tim Neutkens <tim@timneutkens.nl> Co-authored-by: JJ Kasper <jj@jjsweb.site>
1 parent 5c2faad commit 5f95b6b

File tree

26 files changed

+434
-154
lines changed

26 files changed

+434
-154
lines changed

‎.github/workflows/build_test_deploy.yml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ jobs:
5151
run: sudo ethtool -K eth0 tx off rx off
5252

5353
- name: Check non-docs only change
54-
run: echo ::set-output name=DOCS_CHANGE::$(node scripts/run-for-change.js --not --type docs --exec echo 'nope')
54+
run: echo "::set-output name=DOCS_CHANGE::$(node scripts/run-for-change.js --not --type docs --exec echo 'nope')"
5555
id: docs-change
5656

5757
- run: echo ${{steps.docs-change.outputs.DOCS_CHANGE}}
@@ -124,7 +124,7 @@ jobs:
124124
path: ./*
125125
key: ${{ github.sha }}-${{ github.run_number }}
126126

127-
- run: echo ::set-output name=SWC_CHANGE::$(node scripts/run-for-change.js --type next-swc --exec echo 'yup')
127+
- run: echo "::set-output name=SWC_CHANGE::$(node scripts/run-for-change.js --type next-swc --exec echo 'yup')"
128128
id: swc-change
129129

130130
- run: echo ${{ steps.swc-change.outputs.SWC_CHANGE }}
@@ -1102,7 +1102,7 @@ jobs:
11021102
with:
11031103
fetch-depth: 25
11041104

1105-
- run: echo ::set-output name=DOCS_CHANGE::$(node scripts/run-for-change.js --not --type docs --exec echo 'nope')
1105+
- run: echo "::set-output name=DOCS_CHANGE::$(node scripts/run-for-change.js --not --type docs --exec echo 'nope')"
11061106
id: docs-change
11071107

11081108
- name: Setup node
@@ -1191,7 +1191,7 @@ jobs:
11911191
with:
11921192
fetch-depth: 25
11931193

1194-
- run: echo ::set-output name=SWC_CHANGE::$(node scripts/run-for-change.js --type next-swc --exec echo 'yup')
1194+
- run: echo "::set-output name=SWC_CHANGE::$(node scripts/run-for-change.js --type next-swc --exec echo 'yup')"
11951195
id: swc-change
11961196

11971197
- run: echo ${{ steps.swc-change.outputs.SWC_CHANGE }}

‎.github/workflows/pull_request_stats.yml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ jobs:
2424
fetch-depth: 25
2525

2626
- name: Check non-docs only change
27-
run: echo ::set-output name=DOCS_CHANGE::$(node scripts/run-for-change.js --not --type docs --exec echo 'nope')
27+
run: echo "::set-output name=DOCS_CHANGE::$(node scripts/run-for-change.js --not --type docs --exec echo 'nope')"
2828
id: docs-change
2929

3030
- name: Setup node
@@ -118,7 +118,7 @@ jobs:
118118
fetch-depth: 25
119119

120120
- name: Check non-docs only change
121-
run: echo ::set-output name=DOCS_CHANGE::$(node scripts/run-for-change.js --not --type docs --exec echo 'nope')
121+
run: echo "::set-output name=DOCS_CHANGE::$(node scripts/run-for-change.js --not --type docs --exec echo 'nope')"
122122
id: docs-change
123123

124124
- uses: actions/download-artifact@v3

‎packages/next/build/entries.ts‎

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import { normalizePathSep } from '../shared/lib/page-path/normalize-path-sep'
4646
import { normalizePagePath } from '../shared/lib/page-path/normalize-page-path'
4747
import { serverComponentRegex } from './webpack/loaders/utils'
4848
import { ServerRuntime } from '../types'
49+
import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'
4950
import { encodeMatchers } from './webpack/loaders/next-middleware-loader'
5051

5152
type ObjectValue<T> = T extends { [key: string]: infer V } ? V : never
@@ -223,6 +224,7 @@ export function getAppEntry(opts: {
223224
name: string
224225
pagePath: string
225226
appDir: string
227+
appPaths: string[] | null
226228
pageExtensions: string[]
227229
}) {
228230
return {
@@ -353,6 +355,22 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
353355
const nestedMiddleware: string[] = []
354356
let middlewareMatchers: MiddlewareMatcher[] | undefined = undefined
355357

358+
let appPathsPerRoute: Record<string, string[]> = {}
359+
if (appDir && appPaths) {
360+
for (const pathname in appPaths) {
361+
const normalizedPath = normalizeAppPath(pathname) || '/'
362+
if (!appPathsPerRoute[normalizedPath]) {
363+
appPathsPerRoute[normalizedPath] = []
364+
}
365+
appPathsPerRoute[normalizedPath].push(pathname)
366+
}
367+
368+
// Make sure to sort parallel routes to make the result deterministic.
369+
appPathsPerRoute = Object.fromEntries(
370+
Object.entries(appPathsPerRoute).map(([k, v]) => [k, v.sort()])
371+
)
372+
}
373+
356374
const getEntryHandler =
357375
(mappings: Record<string, string>, pagesType: 'app' | 'pages' | 'root') =>
358376
async (page: string) => {
@@ -431,10 +449,13 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
431449
},
432450
onServer: () => {
433451
if (pagesType === 'app' && appDir) {
452+
const matchedAppPaths =
453+
appPathsPerRoute[normalizeAppPath(page) || '/']
434454
server[serverBundlePath] = getAppEntry({
435455
name: serverBundlePath,
436456
pagePath: mappings[page],
437457
appDir,
458+
appPaths: matchedAppPaths,
438459
pageExtensions,
439460
})
440461
} else if (isTargetLikeServerless(target)) {
@@ -450,15 +471,18 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
450471
}
451472
},
452473
onEdgeServer: () => {
453-
const appDirLoader =
454-
pagesType === 'app'
455-
? getAppEntry({
456-
name: serverBundlePath,
457-
pagePath: mappings[page],
458-
appDir: appDir!,
459-
pageExtensions,
460-
}).import
461-
: ''
474+
let appDirLoader: string = ''
475+
if (pagesType === 'app') {
476+
const matchedAppPaths =
477+
appPathsPerRoute[normalizeAppPath(page) || '/']
478+
appDirLoader = getAppEntry({
479+
name: serverBundlePath,
480+
pagePath: mappings[page],
481+
appDir: appDir!,
482+
appPaths: matchedAppPaths,
483+
pageExtensions,
484+
}).import
485+
}
462486

463487
edgeServer[serverBundlePath] = getEdgeServerEntry({
464488
...params,

‎packages/next/build/index.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,9 @@ export default async function build(
550550
const pageKeys = {
551551
pages: Object.keys(mappedPages),
552552
app: mappedAppPages
553-
? Object.keys(mappedAppPages).map((key) => normalizeAppPath(key))
553+
? Object.keys(mappedAppPages).map(
554+
(key) => normalizeAppPath(key) || '/'
555+
)
554556
: undefined,
555557
}
556558

‎packages/next/build/webpack/loaders/next-app-loader.ts‎

Lines changed: 92 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -5,86 +5,104 @@ import { getModuleBuildInfo } from './get-module-build-info'
55
async function createTreeCodeFromPath({
66
pagePath,
77
resolve,
8-
removeExt,
8+
resolveParallelSegments,
99
}: {
1010
pagePath: string
1111
resolve: (pathname: string) => Promise<string | undefined>
12-
removeExt: (pathToRemoveExtensions: string) => string
12+
resolveParallelSegments: (
13+
pathname: string
14+
) => [key: string, segment: string][]
1315
}) {
14-
let tree: undefined | string
1516
const splittedPath = pagePath.split(/[\\/]/)
1617
const appDirPrefix = splittedPath[0]
1718

18-
const segments = ['', ...splittedPath.slice(1)]
19-
20-
// segment.length - 1 because arrays start at 0 and we're decrementing
21-
for (let i = segments.length - 1; i >= 0; i--) {
22-
const segment = removeExt(segments[i])
23-
const segmentPath = segments.slice(0, i + 1).join('/')
24-
25-
// First item in the list is the page which can't have layouts by itself
26-
if (i === segments.length - 1) {
27-
const resolvedPagePath = await resolve(pagePath)
28-
// Use '' for segment as it's the page. There can't be a segment called '' so this is the safest way to add it.
29-
tree = `['', {}, {filePath: ${JSON.stringify(
30-
resolvedPagePath
31-
)}, page: () => require(${JSON.stringify(resolvedPagePath)})}]`
32-
continue
33-
}
34-
35-
// For segmentPath === '' avoid double `/`
36-
const layoutPath = `${appDirPrefix}${segmentPath}/layout`
37-
// For segmentPath === '' avoid double `/`
38-
const loadingPath = `${appDirPrefix}${segmentPath}/loading`
39-
40-
const resolvedLayoutPath = await resolve(layoutPath)
41-
const resolvedLoadingPath = await resolve(loadingPath)
19+
async function createSubtreePropsFromSegmentPath(
20+
segments: string[]
21+
): Promise<string> {
22+
const segmentPath = segments.join('/')
4223

4324
// Existing tree are the children of the current segment
44-
const children = tree
25+
const props: Record<string, string> = {}
26+
27+
// We need to resolve all parallel routes in this level.
28+
const parallelSegments: [key: string, segment: string][] = []
29+
if (segments.length === 0) {
30+
parallelSegments.push(['children', ''])
31+
} else {
32+
parallelSegments.push(...resolveParallelSegments(segmentPath))
33+
}
4534

46-
tree = `['${segment}', {
47-
${
48-
// When there are no children the current index is the page component
49-
children ? `children: ${children},` : ''
35+
for (const [parallelKey, parallelSegment] of parallelSegments) {
36+
const parallelSegmentPath = segmentPath + '/' + parallelSegment
37+
38+
if (parallelSegment === 'page') {
39+
const matchedPagePath = `${appDirPrefix}${parallelSegmentPath}`
40+
const resolvedPagePath = await resolve(matchedPagePath)
41+
// Use '' for segment as it's the page. There can't be a segment called '' so this is the safest way to add it.
42+
props[parallelKey] = `['', {}, {filePath: ${JSON.stringify(
43+
resolvedPagePath
44+
)}, page: () => require(${JSON.stringify(resolvedPagePath)})}]`
45+
continue
5046
}
51-
}, {
52-
filePath: ${JSON.stringify(resolvedLayoutPath)},
53-
${
54-
resolvedLayoutPath
55-
? `layout: () => require(${JSON.stringify(resolvedLayoutPath)}),`
56-
: ''
57-
}
58-
${
59-
resolvedLoadingPath
60-
? `loading: () => require(${JSON.stringify(resolvedLoadingPath)}),`
61-
: ''
62-
}
63-
}]`
47+
48+
const subtree = await createSubtreePropsFromSegmentPath([
49+
...segments,
50+
parallelSegment,
51+
])
52+
53+
// For segmentPath === '' avoid double `/`
54+
const layoutPath = `${appDirPrefix}${parallelSegmentPath}/layout`
55+
// For segmentPath === '' avoid double `/`
56+
const loadingPath = `${appDirPrefix}${parallelSegmentPath}/loading`
57+
58+
const resolvedLayoutPath = await resolve(layoutPath)
59+
const resolvedLoadingPath = await resolve(loadingPath)
60+
61+
props[parallelKey] = `[
62+
'${parallelSegment}',
63+
${subtree},
64+
{
65+
filePath: ${JSON.stringify(resolvedLayoutPath)},
66+
${
67+
resolvedLayoutPath
68+
? `layout: () => require(${JSON.stringify(resolvedLayoutPath)}),`
69+
: ''
70+
}
71+
${
72+
resolvedLoadingPath
73+
? `loading: () => require(${JSON.stringify(
74+
resolvedLoadingPath
75+
)}),`
76+
: ''
77+
}
78+
}
79+
]`
80+
}
81+
82+
return `{
83+
${Object.entries(props)
84+
.map(([key, value]) => `${key}: ${value}`)
85+
.join(',\n')}
86+
}`
6487
}
6588

66-
return `const tree = ${tree};`
89+
const tree = await createSubtreePropsFromSegmentPath([])
90+
return `const tree = ${tree}.children;`
6791
}
6892

6993
function createAbsolutePath(appDir: string, pathToTurnAbsolute: string) {
7094
return pathToTurnAbsolute.replace(/^private-next-app-dir/, appDir)
7195
}
7296

73-
function removeExtensions(
74-
extensions: string[],
75-
pathToRemoveExtensions: string
76-
) {
77-
const regex = new RegExp(`(${extensions.join('|')})$`.replace(/\./g, '\\.'))
78-
return pathToRemoveExtensions.replace(regex, '')
79-
}
80-
8197
const nextAppLoader: webpack.LoaderDefinitionFunction<{
8298
name: string
8399
pagePath: string
84100
appDir: string
101+
appPaths: string[] | null
85102
pageExtensions: string[]
86103
}> = async function nextAppLoader() {
87-
const { name, appDir, pagePath, pageExtensions } = this.getOptions() || {}
104+
const { name, appDir, appPaths, pagePath, pageExtensions } =
105+
this.getOptions() || {}
88106

89107
const buildInfo = getModuleBuildInfo((this as any)._module)
90108
buildInfo.route = {
@@ -99,6 +117,24 @@ const nextAppLoader: webpack.LoaderDefinitionFunction<{
99117
}
100118
const resolve = this.getResolve(resolveOptions)
101119

120+
const normalizedAppPaths =
121+
typeof appPaths === 'string' ? [appPaths] : appPaths || []
122+
const resolveParallelSegments = (pathname: string) => {
123+
const matched: Record<string, string> = {}
124+
for (const path of normalizedAppPaths) {
125+
if (path.startsWith(pathname + '/')) {
126+
const restPath = path.slice(pathname.length + 1)
127+
128+
const matchedSegment = restPath.split('/')[0]
129+
const matchedKey = matchedSegment.startsWith('@')
130+
? matchedSegment.slice(1)
131+
: 'children'
132+
matched[matchedKey] = matchedSegment
133+
}
134+
}
135+
return Object.entries(matched)
136+
}
137+
102138
const resolver = async (pathname: string) => {
103139
try {
104140
const resolved = await resolve(this.rootContext, pathname)
@@ -120,7 +156,7 @@ const nextAppLoader: webpack.LoaderDefinitionFunction<{
120156
const treeCode = await createTreeCodeFromPath({
121157
pagePath,
122158
resolve: resolver,
123-
removeExt: (p) => removeExtensions(extensions, p),
159+
resolveParallelSegments,
124160
})
125161

126162
const result = `

0 commit comments

Comments
 (0)