Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved route resolution in next-app-loader #40109

Merged
merged 19 commits into from Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/build_test_deploy.yml
Expand Up @@ -51,7 +51,7 @@ jobs:
run: sudo ethtool -K eth0 tx off rx off

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

- run: echo ${{steps.docs-change.outputs.DOCS_CHANGE}}
Expand Down Expand Up @@ -124,7 +124,7 @@ jobs:
path: ./*
key: ${{ github.sha }}-${{ github.run_number }}

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

- run: echo ${{ steps.swc-change.outputs.SWC_CHANGE }}
Expand Down Expand Up @@ -1102,7 +1102,7 @@ jobs:
with:
fetch-depth: 25

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

- name: Setup node
Expand Down Expand Up @@ -1191,7 +1191,7 @@ jobs:
with:
fetch-depth: 25

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

- run: echo ${{ steps.swc-change.outputs.SWC_CHANGE }}
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/pull_request_stats.yml
Expand Up @@ -24,7 +24,7 @@ jobs:
fetch-depth: 25

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

- name: Setup node
Expand Down Expand Up @@ -118,7 +118,7 @@ jobs:
fetch-depth: 25

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

- uses: actions/download-artifact@v3
Expand Down
42 changes: 33 additions & 9 deletions packages/next/build/entries.ts
Expand Up @@ -46,6 +46,7 @@ import { normalizePathSep } from '../shared/lib/page-path/normalize-path-sep'
import { normalizePagePath } from '../shared/lib/page-path/normalize-page-path'
import { serverComponentRegex } from './webpack/loaders/utils'
import { ServerRuntime } from '../types'
import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'
import { encodeMatchers } from './webpack/loaders/next-middleware-loader'

type ObjectValue<T> = T extends { [key: string]: infer V } ? V : never
Expand Down Expand Up @@ -223,6 +224,7 @@ export function getAppEntry(opts: {
name: string
pagePath: string
appDir: string
appPaths: string[] | null
pageExtensions: string[]
}) {
return {
Expand Down Expand Up @@ -353,6 +355,22 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
const nestedMiddleware: string[] = []
let middlewareMatchers: MiddlewareMatcher[] | undefined = undefined

let appPathsPerRoute: Record<string, string[]> = {}
if (appDir && appPaths) {
for (const pathname in appPaths) {
const normalizedPath = normalizeAppPath(pathname) || '/'
if (!appPathsPerRoute[normalizedPath]) {
appPathsPerRoute[normalizedPath] = []
}
appPathsPerRoute[normalizedPath].push(pathname)
}

// Make sure to sort parallel routes to make the result deterministic.
appPathsPerRoute = Object.fromEntries(
Object.entries(appPathsPerRoute).map(([k, v]) => [k, v.sort()])
)
}

const getEntryHandler =
(mappings: Record<string, string>, pagesType: 'app' | 'pages' | 'root') =>
async (page: string) => {
Expand Down Expand Up @@ -431,10 +449,13 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
},
onServer: () => {
if (pagesType === 'app' && appDir) {
const matchedAppPaths =
appPathsPerRoute[normalizeAppPath(page) || '/']
server[serverBundlePath] = getAppEntry({
name: serverBundlePath,
pagePath: mappings[page],
appDir,
appPaths: matchedAppPaths,
pageExtensions,
})
} else if (isTargetLikeServerless(target)) {
Expand All @@ -450,15 +471,18 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
}
},
onEdgeServer: () => {
const appDirLoader =
pagesType === 'app'
? getAppEntry({
name: serverBundlePath,
pagePath: mappings[page],
appDir: appDir!,
pageExtensions,
}).import
: ''
let appDirLoader: string = ''
if (pagesType === 'app') {
const matchedAppPaths =
appPathsPerRoute[normalizeAppPath(page) || '/']
appDirLoader = getAppEntry({
name: serverBundlePath,
pagePath: mappings[page],
appDir: appDir!,
appPaths: matchedAppPaths,
pageExtensions,
}).import
}

edgeServer[serverBundlePath] = getEdgeServerEntry({
...params,
Expand Down
4 changes: 3 additions & 1 deletion packages/next/build/index.ts
Expand Up @@ -550,7 +550,9 @@ export default async function build(
const pageKeys = {
pages: Object.keys(mappedPages),
app: mappedAppPages
? Object.keys(mappedAppPages).map((key) => normalizeAppPath(key))
? Object.keys(mappedAppPages).map(
(key) => normalizeAppPath(key) || '/'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be explicitly checking for '' instead of using ||

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have a couple of other places doing the same thing, it should probably be built into normalizeAppPath.

)
: undefined,
}

Expand Down
148 changes: 92 additions & 56 deletions packages/next/build/webpack/loaders/next-app-loader.ts
Expand Up @@ -5,86 +5,104 @@ import { getModuleBuildInfo } from './get-module-build-info'
async function createTreeCodeFromPath({
pagePath,
resolve,
removeExt,
resolveParallelSegments,
}: {
pagePath: string
resolve: (pathname: string) => Promise<string | undefined>
removeExt: (pathToRemoveExtensions: string) => string
resolveParallelSegments: (
pathname: string
) => [key: string, segment: string][]
}) {
let tree: undefined | string
const splittedPath = pagePath.split(/[\\/]/)
const appDirPrefix = splittedPath[0]

const segments = ['', ...splittedPath.slice(1)]

// segment.length - 1 because arrays start at 0 and we're decrementing
for (let i = segments.length - 1; i >= 0; i--) {
const segment = removeExt(segments[i])
const segmentPath = segments.slice(0, i + 1).join('/')

// First item in the list is the page which can't have layouts by itself
if (i === segments.length - 1) {
const resolvedPagePath = await resolve(pagePath)
// Use '' for segment as it's the page. There can't be a segment called '' so this is the safest way to add it.
tree = `['', {}, {filePath: ${JSON.stringify(
resolvedPagePath
)}, page: () => require(${JSON.stringify(resolvedPagePath)})}]`
continue
}

// For segmentPath === '' avoid double `/`
const layoutPath = `${appDirPrefix}${segmentPath}/layout`
// For segmentPath === '' avoid double `/`
const loadingPath = `${appDirPrefix}${segmentPath}/loading`

const resolvedLayoutPath = await resolve(layoutPath)
const resolvedLoadingPath = await resolve(loadingPath)
async function createSubtreePropsFromSegmentPath(
segments: string[]
): Promise<string> {
const segmentPath = segments.join('/')

// Existing tree are the children of the current segment
const children = tree
const props: Record<string, string> = {}

// We need to resolve all parallel routes in this level.
const parallelSegments: [key: string, segment: string][] = []
if (segments.length === 0) {
parallelSegments.push(['children', ''])
} else {
parallelSegments.push(...resolveParallelSegments(segmentPath))
}

tree = `['${segment}', {
${
// When there are no children the current index is the page component
children ? `children: ${children},` : ''
for (const [parallelKey, parallelSegment] of parallelSegments) {
const parallelSegmentPath = segmentPath + '/' + parallelSegment

if (parallelSegment === 'page') {
const matchedPagePath = `${appDirPrefix}${parallelSegmentPath}`
const resolvedPagePath = await resolve(matchedPagePath)
// Use '' for segment as it's the page. There can't be a segment called '' so this is the safest way to add it.
props[parallelKey] = `['', {}, {filePath: ${JSON.stringify(
resolvedPagePath
)}, page: () => require(${JSON.stringify(resolvedPagePath)})}]`
continue
}
}, {
filePath: ${JSON.stringify(resolvedLayoutPath)},
${
resolvedLayoutPath
? `layout: () => require(${JSON.stringify(resolvedLayoutPath)}),`
: ''
}
${
resolvedLoadingPath
? `loading: () => require(${JSON.stringify(resolvedLoadingPath)}),`
: ''
}
}]`

const subtree = await createSubtreePropsFromSegmentPath([
...segments,
parallelSegment,
])

// For segmentPath === '' avoid double `/`
const layoutPath = `${appDirPrefix}${parallelSegmentPath}/layout`
// For segmentPath === '' avoid double `/`
const loadingPath = `${appDirPrefix}${parallelSegmentPath}/loading`

const resolvedLayoutPath = await resolve(layoutPath)
const resolvedLoadingPath = await resolve(loadingPath)

props[parallelKey] = `[
'${parallelSegment}',
${subtree},
{
filePath: ${JSON.stringify(resolvedLayoutPath)},
${
resolvedLayoutPath
? `layout: () => require(${JSON.stringify(resolvedLayoutPath)}),`
: ''
}
${
resolvedLoadingPath
? `loading: () => require(${JSON.stringify(
resolvedLoadingPath
)}),`
: ''
}
}
]`
}

return `{
${Object.entries(props)
.map(([key, value]) => `${key}: ${value}`)
.join(',\n')}
}`
}

return `const tree = ${tree};`
const tree = await createSubtreePropsFromSegmentPath([])
return `const tree = ${tree}.children;`
}

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

function removeExtensions(
extensions: string[],
pathToRemoveExtensions: string
) {
const regex = new RegExp(`(${extensions.join('|')})$`.replace(/\./g, '\\.'))
return pathToRemoveExtensions.replace(regex, '')
}

const nextAppLoader: webpack.LoaderDefinitionFunction<{
name: string
pagePath: string
appDir: string
appPaths: string[] | null
pageExtensions: string[]
}> = async function nextAppLoader() {
const { name, appDir, pagePath, pageExtensions } = this.getOptions() || {}
const { name, appDir, appPaths, pagePath, pageExtensions } =
this.getOptions() || {}

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

const normalizedAppPaths =
typeof appPaths === 'string' ? [appPaths] : appPaths || []
const resolveParallelSegments = (pathname: string) => {
const matched: Record<string, string> = {}
for (const path of normalizedAppPaths) {
if (path.startsWith(pathname + '/')) {
const restPath = path.slice(pathname.length + 1)

const matchedSegment = restPath.split('/')[0]
const matchedKey = matchedSegment.startsWith('@')
? matchedSegment.slice(1)
: 'children'
matched[matchedKey] = matchedSegment
}
}
return Object.entries(matched)
}

const resolver = async (pathname: string) => {
try {
const resolved = await resolve(this.rootContext, pathname)
Expand All @@ -120,7 +156,7 @@ const nextAppLoader: webpack.LoaderDefinitionFunction<{
const treeCode = await createTreeCodeFromPath({
pagePath,
resolve: resolver,
removeExt: (p) => removeExtensions(extensions, p),
resolveParallelSegments,
})

const result = `
Expand Down