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.

Sign up for GitHub

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

Jump to bottom

Refactor server routing #37725

Merged
merged 4 commits into from
Jun 16, 2022
Merged

Refactor server routing #37725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3e5568f
Bugfix: use `trailingSlash` correctly when formatting a pathname
javivelasco Jun 16, 2022
f5f68bd
Allow to ignore prefix on `addLocale`
javivelasco Jun 16, 2022
60497f6
Polymorphic type for `getCustomRoute`
javivelasco Jun 16, 2022
d1d4e48
Refactor Server Router
javivelasco Jun 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions packages/next/server/base-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -705,7 +705,6 @@ export default abstract class Server<ServerOptions extends Options = Options> {
}

protected generateRoutes(): {
basePath: string
headers: Route[]
rewrites: {
beforeFiles: Route[]
Expand All @@ -719,7 +718,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
pageChecker: PageChecker
useFileSystemPublicRoutes: boolean
dynamicRoutes: DynamicRoutes | undefined
locales: string[]
nextConfig: NextConfig
} {
const publicRoutes = this.generatePublicRoutes()
const imageRoutes = this.generateImageRoutes()
Expand Down Expand Up @@ -834,6 +833,7 @@ export default abstract class Server<ServerOptions extends Options = Options> {
const catchAllRoute: Route = {
match: getPathMatch('/:path*'),
type: 'route',
matchesLocale: true,
name: 'Catchall render',
fn: async (req, res, _params, parsedUrl) => {
let { pathname, query } = parsedUrl
Expand Down Expand Up @@ -899,9 +899,8 @@ export default abstract class Server<ServerOptions extends Options = Options> {
catchAllMiddleware,
useFileSystemPublicRoutes,
dynamicRoutes: this.dynamicRoutes,
basePath: this.nextConfig.basePath,
pageChecker: this.hasPage.bind(this),
locales: this.nextConfig.i18n?.locales || [],
nextConfig: this.nextConfig,
}
}

Expand Down
1 change: 0 additions & 1 deletion packages/next/server/dev/next-dev-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,6 @@ export default class DevServer extends Server {
fsRoutes.push({
match: getPathMatch('/:path*'),
type: 'route',
requireBasePath: false,
name: 'catchall public directory route',
fn: async (req, res, params, parsedUrl) => {
const { pathname } = parsedUrl
Expand Down
11 changes: 9 additions & 2 deletions packages/next/server/next-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ export default class NextNodeServer extends BaseServer {
return [
{
match: getPathMatch('/:path*'),
matchesBasePath: true,
name: 'public folder catchall',
fn: async (req, res, params, parsedUrl) => {
const pathParts: string[] = params.path || []
Expand Down Expand Up @@ -973,7 +974,7 @@ export default class NextNodeServer extends BaseServer {
let fallback: Route[] = []

if (!this.minimalMode) {
const buildRewrite = (rewrite: Rewrite, check = true) => {
const buildRewrite = (rewrite: Rewrite, check = true): Route => {
const rewriteRoute = getCustomRoute({
type: 'rewrite',
rule: rewrite,
Expand All @@ -985,6 +986,10 @@ export default class NextNodeServer extends BaseServer {
type: rewriteRoute.type,
name: `Rewrite route ${rewriteRoute.source}`,
match: rewriteRoute.match,
matchesBasePath: true,
matchesLocale: true,
matchesLocaleAPIRoutes: true,
matchesTrailingSlash: true,
fn: async (req, res, params, parsedUrl) => {
const { newUrl, parsedDestination } = prepareDestination({
appendParamsToQuery: true,
Expand All @@ -1011,7 +1016,7 @@ export default class NextNodeServer extends BaseServer {
query: parsedDestination.query,
}
},
} as Route
}
}

if (Array.isArray(this.customRoutes.rewrites)) {
Expand Down Expand Up @@ -1264,6 +1269,8 @@ export default class NextNodeServer extends BaseServer {

return {
match: getPathMatch('/:path*'),
matchesBasePath: true,
matchesLocale: true,
type: 'route',
name: 'middleware catchall',
fn: async (req, res, _params, parsed) => {
Expand Down
177 changes: 76 additions & 101 deletions packages/next/server/router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { NextConfig } from './config'
import type { ParsedUrlQuery } from 'querystring'
import type { BaseNextRequest, BaseNextResponse } from './base-http'
import type {
Expand All @@ -13,6 +14,8 @@ import { RouteHas } from '../lib/load-custom-routes'
import { matchHas } from '../shared/lib/router/utils/prepare-destination'
import { removePathPrefix } from '../shared/lib/router/utils/remove-path-prefix'
import { getRequestMeta } from './request-meta'
import { formatNextPathnameInfo } from '../shared/lib/router/utils/format-next-pathname-info'
import { getNextPathnameInfo } from '../shared/lib/router/utils/get-next-pathname-info'

type RouteResult = {
finished: boolean
Expand All @@ -27,7 +30,10 @@ export type Route = {
check?: boolean
statusCode?: number
name: string
requireBasePath?: false
matchesBasePath?: true
matchesLocale?: true
matchesLocaleAPIRoutes?: true
matchesTrailingSlash?: true
internal?: true
fn: (
req: BaseNextRequest,
Expand All @@ -41,10 +47,7 @@ export type DynamicRoutes = Array<{ page: string; match: RouteMatch }>

export type PageChecker = (pathname: string) => Promise<boolean>

const customRouteTypes = new Set(['rewrite', 'redirect', 'header'])

export default class Router {
basePath: string
headers: Route[]
fsRoutes: Route[]
redirects: Route[]
Expand All @@ -58,11 +61,10 @@ export default class Router {
pageChecker: PageChecker
dynamicRoutes: DynamicRoutes
useFileSystemPublicRoutes: boolean
locales: string[]
seenRequests: Set<any>
nextConfig: NextConfig

constructor({
basePath = '',
headers = [],
fsRoutes = [],
rewrites = {
Expand All @@ -76,9 +78,8 @@ export default class Router {
dynamicRoutes = [],
pageChecker,
useFileSystemPublicRoutes,
locales = [],
nextConfig,
}: {
basePath: string
headers: Route[]
fsRoutes: Route[]
rewrites: {
Expand All @@ -92,9 +93,9 @@ export default class Router {
dynamicRoutes: DynamicRoutes | undefined
pageChecker: PageChecker
useFileSystemPublicRoutes: boolean
locales: string[]
nextConfig: NextConfig
}) {
this.basePath = basePath
this.nextConfig = nextConfig
this.headers = headers
this.fsRoutes = fsRoutes
this.rewrites = rewrites
Expand All @@ -104,10 +105,17 @@ export default class Router {
this.catchAllMiddleware = catchAllMiddleware
this.dynamicRoutes = dynamicRoutes
this.useFileSystemPublicRoutes = useFileSystemPublicRoutes
this.locales = locales
this.seenRequests = new Set()
}

get locales() {
return this.nextConfig.i18n?.locales || []
}

get basePath() {
return this.nextConfig.basePath || ''
}

setDynamicRoutes(routes: DynamicRoutes = []) {
this.dynamicRoutes = routes
}
Expand Down Expand Up @@ -228,7 +236,6 @@ export default class Router {
{
type: 'route',
name: 'page checker',
requireBasePath: false,
match: getPathMatch('/:path*'),
fn: async (
checkerReq,
Expand Down Expand Up @@ -262,7 +269,6 @@ export default class Router {
{
type: 'route',
name: 'dynamic route/page check',
requireBasePath: false,
match: getPathMatch('/:path*'),
fn: async (
_checkerReq,
Expand All @@ -283,131 +289,100 @@ export default class Router {
// disabled
...(this.useFileSystemPublicRoutes ? [this.catchAllRoute] : []),
]
const originallyHadBasePath =
!this.basePath || getRequestMeta(req, '_nextHadBasePath')

for (const testRoute of allRoutes) {
// if basePath is being used, the basePath will still be included
// in the pathname here to allow custom-routes to require containing
// it or not, filesystem routes and pages must always include the basePath
// if it is set
let currentPathname = parsedUrlUpdated.pathname as string
const originalPathname = currentPathname
const requireBasePath = testRoute.requireBasePath !== false
const isCustomRoute = customRouteTypes.has(testRoute.type)
const isPublicFolderCatchall =
testRoute.name === 'public folder catchall'
const isMiddlewareCatchall = testRoute.name === 'middleware catchall'
const keepBasePath =
isCustomRoute || isPublicFolderCatchall || isMiddlewareCatchall
const keepLocale = isCustomRoute

const currentPathnameNoBasePath = removePathPrefix(
currentPathname,
this.basePath
)

if (!keepBasePath) {
currentPathname = currentPathnameNoBasePath
const originalPathname = parsedUrlUpdated.pathname as string
const pathnameInfo = getNextPathnameInfo(originalPathname, {
nextConfig: this.nextConfig,
parseData: false,
})

if (
pathnameInfo.locale &&
!testRoute.matchesLocaleAPIRoutes &&
pathnameInfo.pathname.match(/^\/api(?:\/|$)/)
) {
continue
}

const localePathResult = normalizeLocalePath(
currentPathnameNoBasePath,
this.locales
)
if (getRequestMeta(req, '_nextHadBasePath')) {
pathnameInfo.basePath = this.basePath
}

const activeBasePath = keepBasePath ? this.basePath : ''
const basePath = pathnameInfo.basePath
if (!testRoute.matchesBasePath) {
pathnameInfo.basePath = ''
}

// don't match API routes when they are locale prefixed
// e.g. /api/hello shouldn't match /en/api/hello as a page
// rewrites/redirects can match though
if (
!isCustomRoute &&
localePathResult.detectedLocale &&
localePathResult.pathname.match(/^\/api(?:\/|$)/)
testRoute.matchesLocale &&
parsedUrl.query.__nextLocale &&
!pathnameInfo.locale
) {
continue
pathnameInfo.locale = parsedUrl.query.__nextLocale
}

if (keepLocale) {
if (
!testRoute.internal &&
parsedUrl.query.__nextLocale &&
!localePathResult.detectedLocale
) {
currentPathname = `${activeBasePath}/${
parsedUrl.query.__nextLocale
}${
currentPathnameNoBasePath === '/' ? '' : currentPathnameNoBasePath
}`
}
if (
!testRoute.matchesLocale &&
pathnameInfo.locale === this.nextConfig.i18n?.defaultLocale &&
pathnameInfo.locale
) {
pathnameInfo.locale = undefined
}

if (
getRequestMeta(req, '__nextHadTrailingSlash') &&
!currentPathname.endsWith('/')
) {
currentPathname += '/'
}
} else {
currentPathname = `${
getRequestMeta(req, '_nextHadBasePath') ? activeBasePath : ''
}${
activeBasePath && currentPathnameNoBasePath === '/'
? ''
: currentPathnameNoBasePath
}`
if (
testRoute.matchesTrailingSlash &&
getRequestMeta(req, '__nextHadTrailingSlash')
) {
pathnameInfo.trailingSlash = true
}

let newParams = testRoute.match(currentPathname)
const matchPathname = formatNextPathnameInfo({
ignorePrefix: true,
...pathnameInfo,
})

let newParams = testRoute.match(matchPathname)
if (testRoute.has && newParams) {
const hasParams = matchHas(req, testRoute.has, parsedUrlUpdated.query)

if (hasParams) {
Object.assign(newParams, hasParams)
} else {
newParams = false
}
}

// Check if the match function matched
if (newParams) {
// since we require basePath be present for non-custom-routes we
// 404 here when we matched an fs route
if (!keepBasePath) {
if (
!originallyHadBasePath &&
!getRequestMeta(req, '_nextDidRewrite')
) {
if (requireBasePath) {
// consider this a non-match so the 404 renders
return false
}
// page checker occurs before rewrites so we need to continue
// to check those since they don't always require basePath
continue
}

parsedUrlUpdated.pathname = currentPathname
}
/**
* If it is a matcher that doesn't match the basePath (like the public
* directory) but Next.js is configured to use a basePath that was
* never there, we consider this an invalid match and keep routing.
*/
if (
newParams &&
this.basePath &&
!testRoute.matchesBasePath &&
!getRequestMeta(req, '_nextDidRewrite') &&
!basePath
) {
continue
}

if (newParams) {
parsedUrlUpdated.pathname = matchPathname
const result = await testRoute.fn(
req,
res,
newParams,
parsedUrlUpdated
)

// The response was handled
if (result.finished) {
return true
}

// since the fs route didn't finish routing we need to re-add the
// basePath to continue checking with the basePath present
if (!keepBasePath) {
parsedUrlUpdated.pathname = originalPathname
}
parsedUrlUpdated.pathname = originalPathname

if (result.pathname) {
parsedUrlUpdated.pathname = result.pathname
Expand Down
Loading