Skip to content

Commit

Permalink
Always call createPagesMapping for root paths (vercel#60107)
Browse files Browse the repository at this point in the history
## What?

Always call `createPagesMapping` as it already handles the case when
there are no paths.

Also introduces `PAGE_TYPES` to have a single source of truth for these
types and where they're used. As you can see this replaces a ton of
hardcoded `'app'`, `'pages'`, and `'root'` references.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

### Adding or Updating Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md


## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->


Closes NEXT-1935
  • Loading branch information
timneutkens authored and agustints committed Jan 6, 2024
1 parent f7b3adb commit 9f13546
Show file tree
Hide file tree
Showing 20 changed files with 151 additions and 110 deletions.
17 changes: 9 additions & 8 deletions packages/next/src/build/analysis/get-page-static-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { isAPIRoute } from '../../lib/is-api-route'
import { isEdgeRuntime } from '../../lib/is-edge-runtime'
import { RSC_MODULE_TYPES } from '../../shared/lib/constants'
import type { RSCMeta } from '../webpack/loaders/get-module-build-info'
import { PAGE_TYPES } from '../../lib/page-types'

// TODO: migrate preferredRegion here
// Don't forget to update the next-types-plugin file as well
Expand Down Expand Up @@ -478,7 +479,7 @@ export async function getPageStaticInfo(params: {
nextConfig: Partial<NextConfig>
isDev?: boolean
page?: string
pageType: 'pages' | 'app' | 'root'
pageType: PAGE_TYPES
}): Promise<PageStaticInfo> {
const { isDev, pageFilePath, nextConfig, page, pageType } = params

Expand Down Expand Up @@ -514,7 +515,7 @@ export async function getPageStaticInfo(params: {

const extraConfig: Record<string, any> = {}

if (extraProperties && pageType === 'app') {
if (extraProperties && pageType === PAGE_TYPES.APP) {
for (const prop of extraProperties) {
if (!AUTHORIZED_EXTRA_ROUTER_PROPS.includes(prop)) continue
try {
Expand All @@ -525,14 +526,14 @@ export async function getPageStaticInfo(params: {
}
}
}
} else if (pageType === 'pages') {
} else if (pageType === PAGE_TYPES.PAGES) {
for (const key in config) {
if (!AUTHORIZED_EXTRA_ROUTER_PROPS.includes(key)) continue
extraConfig[key] = config[key]
}
}

if (pageType === 'app') {
if (pageType === PAGE_TYPES.APP) {
if (config) {
let message = `Page config in ${pageFilePath} is deprecated. Replace \`export const config=…\` with the following:`

Expand Down Expand Up @@ -565,7 +566,7 @@ export async function getPageStaticInfo(params: {
// and deprecate the old way. To prevent breaking changes for `pages`, we use the exported config
// as the fallback value.
let resolvedRuntime
if (pageType === 'app') {
if (pageType === PAGE_TYPES.APP) {
resolvedRuntime = runtime
} else {
resolvedRuntime = runtime || config.runtime
Expand All @@ -588,7 +589,7 @@ export async function getPageStaticInfo(params: {
}
}

const requiresServerRuntime = ssr || ssg || pageType === 'app'
const requiresServerRuntime = ssr || ssg || pageType === PAGE_TYPES.APP

const isAnAPIRoute = isAPIRoute(page?.replace(/^(?:\/src)?\/pages\//, '/'))

Expand All @@ -603,7 +604,7 @@ export async function getPageStaticInfo(params: {

if (
resolvedRuntime === SERVER_RUNTIME.edge &&
pageType === 'pages' &&
pageType === PAGE_TYPES.PAGES &&
page &&
!isAnAPIRoute
) {
Expand All @@ -622,7 +623,7 @@ export async function getPageStaticInfo(params: {
)

if (
pageType === 'app' &&
pageType === PAGE_TYPES.APP &&
directives?.has('client') &&
generateStaticParams
) {
Expand Down
107 changes: 60 additions & 47 deletions packages/next/src/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ import { encodeToBase64 } from './webpack/loaders/utils'
import { normalizeCatchAllRoutes } from './normalize-catchall-routes'
import type { PageExtensions } from './page-extensions-type'
import type { MappedPages } from './build-context'
import { PAGE_TYPES } from '../lib/page-types'

export function sortByPageExts(pageExtensions: PageExtensions) {
return (a: string, b: string) => {
Expand Down Expand Up @@ -106,7 +107,7 @@ export async function getStaticInfoIncludingLayouts({
pageFilePath,
isDev,
page,
pageType: isInsideAppDir ? 'app' : 'pages',
pageType: isInsideAppDir ? PAGE_TYPES.APP : PAGE_TYPES.PAGES,
})

const staticInfo: PageStaticInfo = isInsideAppDir
Expand Down Expand Up @@ -139,7 +140,7 @@ export async function getStaticInfoIncludingLayouts({
pageFilePath: layoutFile,
isDev,
page,
pageType: isInsideAppDir ? 'app' : 'pages',
pageType: isInsideAppDir ? PAGE_TYPES.APP : PAGE_TYPES.PAGES,
})

// Only runtime is relevant here.
Expand Down Expand Up @@ -226,13 +227,13 @@ export function createPagesMapping({
isDev: boolean
pageExtensions: PageExtensions
pagePaths: string[]
pagesType: 'pages' | 'root' | 'app'
pagesType: PAGE_TYPES
pagesDir: string | undefined
}): MappedPages {
const isAppRoute = pagesType === 'app'
const pages = pagePaths.reduce<{ [key: string]: string }>(
(result, pagePath) => {
// Do not process .d.ts files inside the `pages` folder
// Do not process .d.ts files as routes
if (pagePath.endsWith('.d.ts') && pageExtensions.includes('ts')) {
return result
}
Expand Down Expand Up @@ -262,38 +263,45 @@ export function createPagesMapping({
{}
)

if (pagesType === 'app') {
const hasAppPages = Object.keys(pages).some((page) =>
page.endsWith('/page')
)
return {
// If there's any app pages existed, add a default not-found page.
// If there's any custom not-found page existed, it will override the default one.
...(hasAppPages && {
'/_not-found': 'next/dist/client/components/not-found-error',
}),
...pages,
switch (pagesType) {
case PAGE_TYPES.ROOT: {
return pages
}
} else if (pagesType === 'root') {
return pages
}

if (isDev) {
delete pages['/_app']
delete pages['/_error']
delete pages['/_document']
}
case PAGE_TYPES.APP: {
const hasAppPages = Object.keys(pages).some((page) =>
page.endsWith('/page')
)
return {
// If there's any app pages existed, add a default not-found page.
// If there's any custom not-found page existed, it will override the default one.
...(hasAppPages && {
'/_not-found': 'next/dist/client/components/not-found-error',
}),
...pages,
}
}
case PAGE_TYPES.PAGES: {
if (isDev) {
delete pages['/_app']
delete pages['/_error']
delete pages['/_document']
}

// In development we always alias these to allow Webpack to fallback to
// the correct source file so that HMR can work properly when a file is
// added or removed.
const root = isDev && pagesDir ? PAGES_DIR_ALIAS : 'next/dist/pages'
// In development we always alias these to allow Webpack to fallback to
// the correct source file so that HMR can work properly when a file is
// added or removed.
const root = isDev && pagesDir ? PAGES_DIR_ALIAS : 'next/dist/pages'

return {
'/_app': `${root}/_app`,
'/_error': `${root}/_error`,
'/_document': `${root}/_document`,
...pages,
return {
'/_app': `${root}/_app`,
'/_error': `${root}/_error`,
'/_document': `${root}/_document`,
...pages,
}
}
default: {
return {}
}
}
}

Expand Down Expand Up @@ -324,7 +332,7 @@ export function getEdgeServerEntry(opts: {
page: string
pages: MappedPages
middleware?: Partial<MiddlewareConfig>
pagesType: 'app' | 'pages' | 'root'
pagesType: PAGE_TYPES
appDirLoader?: string
hasInstrumentationHook?: boolean
preferredRegion: string | string[] | undefined
Expand Down Expand Up @@ -456,9 +464,12 @@ export function runDependingOnPageType<T>(params: {
onServer: () => T
page: string
pageRuntime: ServerRuntime
pageType?: 'app' | 'pages' | 'root'
pageType?: PAGE_TYPES
}): void {
if (params.pageType === 'root' && isInstrumentationHookFile(params.page)) {
if (
params.pageType === PAGE_TYPES.ROOT &&
isInstrumentationHookFile(params.page)
) {
params.onServer()
params.onEdgeServer()
return
Expand Down Expand Up @@ -550,17 +561,14 @@ export async function createEntrypoints(
}

const getEntryHandler =
(
mappings: MappedPages,
pagesType: 'app' | 'pages' | 'root'
): ((page: string) => void) =>
(mappings: MappedPages, pagesType: PAGE_TYPES): ((page: string) => void) =>
async (page) => {
const bundleFile = normalizePagePath(page)
const clientBundlePath = posix.join(pagesType, bundleFile)
const serverBundlePath =
pagesType === 'pages'
pagesType === PAGE_TYPES.PAGES
? posix.join('pages', bundleFile)
: pagesType === 'app'
: pagesType === PAGE_TYPES.APP
? posix.join('app', bundleFile)
: bundleFile.slice(1)
const absolutePagePath = mappings[page]
Expand Down Expand Up @@ -629,7 +637,10 @@ export async function createEntrypoints(
preferredRegion: staticInfo.preferredRegion,
middlewareConfig: encodeToBase64(staticInfo.middleware || {}),
})
} else if (isInstrumentationHookFile(page) && pagesType === 'root') {
} else if (
isInstrumentationHookFile(page) &&
pagesType === PAGE_TYPES.ROOT
) {
server[serverBundlePath.replace('src/', '')] = {
import: absolutePagePath,
// the '../' is needed to make sure the file is not chunked
Expand Down Expand Up @@ -687,7 +698,7 @@ export async function createEntrypoints(
}).import
}
const normalizedServerBundlePath =
isInstrumentationHookFile(page) && pagesType === 'root'
isInstrumentationHookFile(page) && pagesType === PAGE_TYPES.ROOT
? serverBundlePath.replace('src/', '')
: serverBundlePath
edgeServer[normalizedServerBundlePath] = getEdgeServerEntry({
Expand All @@ -711,18 +722,20 @@ export async function createEntrypoints(
const promises: Promise<void[]>[] = []

if (appPaths) {
const entryHandler = getEntryHandler(appPaths, 'app')
const entryHandler = getEntryHandler(appPaths, PAGE_TYPES.APP)
promises.push(Promise.all(Object.keys(appPaths).map(entryHandler)))
}
if (rootPaths) {
promises.push(
Promise.all(
Object.keys(rootPaths).map(getEntryHandler(rootPaths, 'root'))
Object.keys(rootPaths).map(getEntryHandler(rootPaths, PAGE_TYPES.ROOT))
)
)
}
promises.push(
Promise.all(Object.keys(pages).map(getEntryHandler(pages, 'pages')))
Promise.all(
Object.keys(pages).map(getEntryHandler(pages, PAGE_TYPES.PAGES))
)
)

await Promise.all(promises)
Expand Down
28 changes: 13 additions & 15 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ import {
getPageStaticInfo,
} from './analysis/get-page-static-info'
import { createPagesMapping, getPageFilePath, sortByPageExts } from './entries'
import { PAGE_TYPES } from '../lib/page-types'
import { generateBuildId } from './generate-build-id'
import { isWriteable } from './is-writeable'
import * as Log from './output/log'
Expand Down Expand Up @@ -600,7 +601,7 @@ export default async function build(
createPagesMapping({
isDev: false,
pageExtensions: config.pageExtensions,
pagesType: 'pages',
pagesType: PAGE_TYPES.PAGES,
pagePaths: pagesPaths,
pagesDir,
})
Expand Down Expand Up @@ -630,7 +631,7 @@ export default async function build(
createPagesMapping({
pagePaths: appPaths,
isDev: false,
pagesType: 'app',
pagesType: PAGE_TYPES.APP,
pageExtensions: config.pageExtensions,
pagesDir: pagesDir,
})
Expand Down Expand Up @@ -672,18 +673,13 @@ export default async function build(
NextBuildContext.mappedAppPages = mappedAppPages
}

let mappedRootPaths: MappedPages = {}

if (rootPaths.length > 0) {
mappedRootPaths = createPagesMapping({
isDev: false,
pageExtensions: config.pageExtensions,
pagePaths: rootPaths,
pagesType: 'root',
pagesDir: pagesDir,
})
}

const mappedRootPaths = createPagesMapping({
isDev: false,
pageExtensions: config.pageExtensions,
pagePaths: rootPaths,
pagesType: PAGE_TYPES.ROOT,
pagesDir: pagesDir,
})
NextBuildContext.mappedRootPaths = mappedRootPaths

const pagesPageKeys = Object.keys(mappedPages)
Expand Down Expand Up @@ -1585,7 +1581,9 @@ export default async function build(
? await getPageStaticInfo({
pageFilePath,
nextConfig: config,
pageType,
// TODO: fix type mismatch
pageType:
pageType === 'app' ? PAGE_TYPES.APP : PAGE_TYPES.PAGES,
})
: undefined

Expand Down
3 changes: 2 additions & 1 deletion packages/next/src/build/templates/edge-ssr-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type { DocumentType } from '../../shared/lib/utils'
import type { BuildManifest } from '../../server/get-page-files'
import type { RequestData } from '../../server/web/types'
import type { NextConfigComplete } from '../../server/config-shared'
import { PAGE_TYPES } from '../../lib/page-types'

declare const incrementalCacheHandler: any
// OPTIONAL_IMPORT:incrementalCacheHandler
Expand Down Expand Up @@ -44,7 +45,7 @@ const subresourceIntegrityManifest = sriEnabled
const nextFontManifest = maybeJSONParse(self.__NEXT_FONT_MANIFEST)

const render = getRender({
pagesType: 'app',
pagesType: PAGE_TYPES.APP,
dev,
page: 'VAR_PAGE',
appMod,
Expand Down
3 changes: 2 additions & 1 deletion packages/next/src/build/templates/edge-ssr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ import RouteModule from '../../server/future/route-modules/pages/module'
import type { RequestData } from '../../server/web/types'
import type { BuildManifest } from '../../server/get-page-files'
import type { NextConfigComplete } from '../../server/config-shared'
import type { PAGE_TYPES } from '../../lib/page-types'

// injected by the loader afterwards.
declare const pagesType: 'app' | 'pages' | 'root'
declare const pagesType: PAGE_TYPES
declare const sriEnabled: boolean
declare const dev: boolean
declare const nextConfig: NextConfigComplete
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { WEBPACK_RESOURCE_QUERIES } from '../../../../lib/constants'
import { RouteKind } from '../../../../server/future/route-kind'
import { normalizePagePath } from '../../../../shared/lib/page-path/normalize-page-path'
import { loadEntrypoint } from '../../../load-entrypoint'
import type { PAGE_TYPES } from '../../../../lib/page-types'

export type EdgeSSRLoaderQuery = {
absolute500Path: string
Expand All @@ -21,7 +22,7 @@ export type EdgeSSRLoaderQuery = {
page: string
stringifiedConfig: string
appDirLoader?: string
pagesType: 'app' | 'pages' | 'root'
pagesType: PAGE_TYPES
sriEnabled: boolean
incrementalCacheHandlerPath?: string
preferredRegion: string | string[] | undefined
Expand Down
Loading

0 comments on commit 9f13546

Please sign in to comment.