Skip to content

Commit

Permalink
Add support for optional catchall with new router (#38444)
Browse files Browse the repository at this point in the history
Follow-up of #38439.

Found a small issue with booting `next start` that is now resolved.
Also added optional catchall routes support.



## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] 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)
  • Loading branch information
timneutkens committed Jul 8, 2022
1 parent 3da3df2 commit b421fa2
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 44 deletions.
16 changes: 8 additions & 8 deletions packages/next/build/webpack/plugins/pages-manifest-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ export type PagesManifest = { [page: string]: string }

let edgeServerPages = {}
let nodeServerPages = {}
let edgeServerRootPaths = {}
let nodeServerRootPaths = {}
let edgeServerAppPaths = {}
let nodeServerAppPaths = {}

// This plugin creates a pages-manifest.json from page entrypoints.
// This is used for mapping paths like `/` to `.next/server/static/<buildid>/pages/index.js` when doing SSR
Expand Down Expand Up @@ -42,7 +42,7 @@ export default class PagesManifestPlugin implements webpack.Plugin {
createAssets(compilation: any, assets: any) {
const entrypoints = compilation.entrypoints
const pages: PagesManifest = {}
const rootPaths: PagesManifest = {}
const appPaths: PagesManifest = {}

for (const entrypoint of entrypoints.values()) {
const pagePath = getRouteFromEntrypoint(
Expand Down Expand Up @@ -78,7 +78,7 @@ export default class PagesManifestPlugin implements webpack.Plugin {
file = normalizePathSep(file)

if (entrypoint.name.startsWith('app/')) {
rootPaths[pagePath] = file
appPaths[pagePath] = file
} else {
pages[pagePath] = file
}
Expand All @@ -88,10 +88,10 @@ export default class PagesManifestPlugin implements webpack.Plugin {
// we need to merge both pages to generate the full manifest.
if (this.isEdgeRuntime) {
edgeServerPages = pages
edgeServerRootPaths = rootPaths
edgeServerAppPaths = appPaths
} else {
nodeServerPages = pages
nodeServerRootPaths = rootPaths
nodeServerAppPaths = appPaths
}

assets[
Expand All @@ -113,8 +113,8 @@ export default class PagesManifestPlugin implements webpack.Plugin {
] = new sources.RawSource(
JSON.stringify(
{
...edgeServerRootPaths,
...nodeServerRootPaths,
...edgeServerAppPaths,
...nodeServerAppPaths,
},
null,
2
Expand Down
60 changes: 49 additions & 11 deletions packages/next/server/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,34 @@ export type ChildProp = {
segment: Segment
}

function getSegmentParam(segment: string): {
param: string
type: 'catchall' | 'optional-catchall' | 'dynamic'
} | null {
if (segment.startsWith('[[...') && segment.endsWith(']]')) {
return {
type: 'optional-catchall',
param: segment.slice(5, -2),
}
}

if (segment.startsWith('[...') && segment.endsWith(']')) {
return {
type: 'catchall',
param: segment.slice(4, -1),
}
}

if (segment.startsWith('[') && segment.endsWith(']')) {
return {
type: 'dynamic',
param: segment.slice(1, -1),
}
}

return null
}

export async function renderToHTML(
req: IncomingMessage,
res: ServerResponse,
Expand Down Expand Up @@ -383,25 +411,35 @@ export async function renderToHTML(
segment: string
): { param: string; value: string } | null => {
// TODO: use correct matching for dynamic routes to get segment param
const segmentParam =
segment.startsWith('[') && segment.endsWith(']')
? segment.slice(segment.startsWith('[...') ? 4 : 1, -1)
: null
const segmentParam = getSegmentParam(segment)
if (!segmentParam) {
return null
}

const key = segmentParam.param

if (!segmentParam || (!pathParams[segmentParam] && !query[segmentParam])) {
if (!pathParams[key] && !query[key]) {
if (segmentParam.type === 'optional-catchall') {
return {
param: key,
value: '',
}
}
return null
}

return {
param: segmentParam,
// @ts-expect-error TODO: handle case where value is an array
param: key,
value:
// TODO: this should only read from `pathParams`. There's an inconsistency where `query` holds params currently which has to be fixed.
pathParams[segmentParam] ??
(Array.isArray(query[segmentParam])
(Array.isArray(pathParams[key])
? // @ts-expect-error TODO: handle case where value is an array
pathParams[key].join('/')
: pathParams[key]) ??
(Array.isArray(query[key])
? // @ts-expect-error TODO: handle case where value is an array
query[segmentParam].join('/')
: query[segmentParam]),
query[key].join('/')
: query[key]),
}
}

Expand Down
26 changes: 4 additions & 22 deletions packages/next/server/base-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { format as formatUrl, parse as parseUrl } from 'url'
import { getRedirectStatus } from '../lib/load-custom-routes'
import {
NEXT_BUILTIN_DOCUMENT,
NEXT_CLIENT_SSR_ENTRY_SUFFIX,
SERVERLESS_DIRECTORY,
SERVER_DIRECTORY,
STATIC_STATUS_PAGES,
Expand Down Expand Up @@ -1026,33 +1027,14 @@ export default abstract class Server<ServerOptions extends Options = Options> {
const appPathRoutes: Record<string, string> = {}

Object.keys(this.appPathsManifest || {}).forEach((entry) => {
if (entry.endsWith(NEXT_CLIENT_SSR_ENTRY_SUFFIX)) {
return
}
appPathRoutes[normalizeAppPath(entry) || '/'] = entry
})
return appPathRoutes
}

protected getAppPathLayouts(pathname: string): string[] {
const layoutPaths: string[] = []

if (this.appPathRoutes) {
const paths = Object.values(this.appPathRoutes)
const parts = pathname.split('/').filter(Boolean)

for (let i = 1; i < parts.length; i++) {
const layoutPath = `/${parts.slice(0, i).join('/')}/layout`

if (paths.includes(layoutPath)) {
layoutPaths.push(layoutPath)
}
}

if (this.appPathRoutes['/layout']) {
layoutPaths.unshift('/layout')
}
}
return layoutPaths
}

protected async run(
req: BaseNextRequest,
res: BaseNextResponse,
Expand Down
3 changes: 0 additions & 3 deletions packages/next/server/require.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,6 @@ export function getPagePath(
let rootPathsManifest: undefined | PagesManifest

if (appDirEnabled) {
if (page === '/_root') {
return join(serverBuildPath, 'root.js')
}
rootPathsManifest = require(join(serverBuildPath, APP_PATHS_MANIFEST))
}
const pagesManifest = require(join(
Expand Down

0 comments on commit b421fa2

Please sign in to comment.