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

Server Router Improvements #45716

Merged
merged 27 commits into from
Feb 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
868f62b
feat: initial app routes support
wyattjoh Feb 8, 2023
bc8e4cd
tests: enable development tests
wyattjoh Feb 8, 2023
1470f87
refactor: adjusted to support appPaths for pages development
wyattjoh Feb 11, 2023
6295fca
fix: fixed bug with edge render
wyattjoh Feb 11, 2023
6dcc7f0
fix: fixed case where error pages were not ensured
wyattjoh Feb 11, 2023
b35f106
fix: fixed case for trailing slash handling in the router
wyattjoh Feb 11, 2023
43c867a
fix: handle edge case where api routes also include localization info…
wyattjoh Feb 11, 2023
3b3d76d
fix: catch loading error when manifest is not available
wyattjoh Feb 11, 2023
00c8936
feat: added provider caching for development
wyattjoh Feb 11, 2023
cebcc1e
feat: added cached file scanner
wyattjoh Feb 11, 2023
d9ab5d0
fix: fixed issue where cached matchers resulted in not-reloaded message
wyattjoh Feb 11, 2023
f87272b
fix: support better duplicate detection
wyattjoh Feb 11, 2023
5830d61
fix: improve handling for duplicate app/pages when detected
wyattjoh Feb 11, 2023
a2e917a
fix: fixed test failures around locale detection in development
wyattjoh Feb 13, 2023
d0fd708
fix: fixed error with error message detection
wyattjoh Feb 13, 2023
db58e08
fix: fixed issue with route sorting
wyattjoh Feb 13, 2023
0ea93cc
fix: ensure leading slash when matching, polyfilled request
wyattjoh Feb 13, 2023
bd453c9
fix: handle cases where the dev matchers are too slow during testing
wyattjoh Feb 13, 2023
3624594
chore: removed comments + old waits
wyattjoh Feb 13, 2023
c5c8fee
fix: improve parallel route support
wyattjoh Feb 13, 2023
47be100
fix: enhance duplicate matcher warning
wyattjoh Feb 13, 2023
40227c6
Merge branch 'canary' into wyattjoh/app-routes
timneutkens Feb 14, 2023
1813d52
Merge branch 'canary' into wyattjoh/app-routes
ijjk Feb 14, 2023
586f931
Merge branch 'canary' into wyattjoh/app-routes
ijjk Feb 14, 2023
87c1343
fix prefixing normalizer on windows
ijjk Feb 14, 2023
fe008d4
Merge branch 'canary' into wyattjoh/app-routes
ijjk Feb 14, 2023
302e392
Merge branch 'wyattjoh/app-routes' of github.com:wyattjoh/next.js int…
ijjk Feb 14, 2023
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
4 changes: 4 additions & 0 deletions errors/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,10 @@
"title": "context-in-server-component",
"path": "/errors/context-in-server-component.md"
},
{
"title": "next-response-next-in-app-route-handler",
"path": "/errors/next-response-next-in-app-route-handler.md"
},
{
"title": "react-client-hook-in-server-component",
"path": "/errors/react-client-hook-in-server-component.md"
Expand Down
14 changes: 14 additions & 0 deletions errors/next-response-next-in-app-route-handler.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# `NextResponse.next()` used in a App Route Handler

#### Why This Error Occurred

App Route Handler's do not currently support using the `NextResponse.next()` method to forward to the next middleware because the handler is considered the endpoint to the middleware chain. Handlers must always return a `Response` object instead.

#### Possible Ways to Fix It

Remove the `NextResponse.next()` and replace it with a correct response handler.

### Useful Links

- [`Response`](https://developer.mozilla.org/en-US/docs/Web/API/Response)
- [`NextResponse`](https://nextjs.org/docs/api-reference/next/server#nextresponse)
10 changes: 4 additions & 6 deletions packages/next/src/build/entries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ export function getAppEntry(opts: {
name: string
pagePath: string
appDir: string
appPaths: string[] | null
appPaths: ReadonlyArray<string> | null
pageExtensions: string[]
assetPrefix: string
isDev?: boolean
Expand Down Expand Up @@ -310,7 +310,7 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
let appPathsPerRoute: Record<string, string[]> = {}
if (appDir && appPaths) {
for (const pathname in appPaths) {
const normalizedPath = normalizeAppPath(pathname) || '/'
const normalizedPath = normalizeAppPath(pathname)
if (!appPathsPerRoute[normalizedPath]) {
appPathsPerRoute[normalizedPath] = []
}
Expand Down Expand Up @@ -403,8 +403,7 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
},
onServer: () => {
if (pagesType === 'app' && appDir) {
const matchedAppPaths =
appPathsPerRoute[normalizeAppPath(page) || '/']
const matchedAppPaths = appPathsPerRoute[normalizeAppPath(page)]
server[serverBundlePath] = getAppEntry({
name: serverBundlePath,
pagePath: mappings[page],
Expand All @@ -420,8 +419,7 @@ export async function createEntrypoints(params: CreateEntrypointsParams) {
onEdgeServer: () => {
let appDirLoader: string = ''
if (pagesType === 'app') {
const matchedAppPaths =
appPathsPerRoute[normalizeAppPath(page) || '/']
const matchedAppPaths = appPathsPerRoute[normalizeAppPath(page)]
appDirLoader = getAppEntry({
name: serverBundlePath,
pagePath: mappings[page],
Expand Down
12 changes: 8 additions & 4 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -498,7 +498,9 @@ export default async function build(
.traceAsyncFn(() =>
recursiveReadDir(
appDir,
new RegExp(`^page\\.(?:${config.pageExtensions.join('|')})$`)
new RegExp(
`^(page|route)\\.(?:${config.pageExtensions.join('|')})$`
)
)
)
}
Expand Down Expand Up @@ -575,7 +577,7 @@ export default async function build(
if (mappedAppPages) {
denormalizedAppPages = Object.keys(mappedAppPages)
for (const appKey of denormalizedAppPages) {
const normalizedAppPageKey = normalizeAppPath(appKey) || '/'
const normalizedAppPageKey = normalizeAppPath(appKey)
const pagePath = mappedPages[normalizedAppPageKey]
if (pagePath) {
const appPath = mappedAppPages[appKey]
Expand Down Expand Up @@ -1088,7 +1090,7 @@ export default async function build(
)

Object.keys(appPathsManifest).forEach((entry) => {
appPathRoutes[entry] = normalizeAppPath(entry) || '/'
appPathRoutes[entry] = normalizeAppPath(entry)
})
await promises.writeFile(
path.join(distDir, APP_PATH_ROUTES_MANIFEST),
Expand Down Expand Up @@ -1379,7 +1381,9 @@ export default async function build(
if (
(!isDynamicRoute(page) ||
!workerResult.prerenderRoutes?.length) &&
workerResult.appConfig?.revalidate !== 0
workerResult.appConfig?.revalidate !== 0 &&
// TODO-APP: (wyattjoh) this may be where we can enable prerendering for app handlers
originalAppPath.endsWith('/page')
) {
appStaticPaths.set(originalAppPath, [page])
appStaticPathsEncoded.set(originalAppPath, [page])
Expand Down
41 changes: 39 additions & 2 deletions packages/next/src/build/webpack/loaders/next-app-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { APP_DIR_ALIAS } from '../../../lib/constants'
import { buildMetadata, discoverStaticMetadataFiles } from './metadata/discover'

const isNotResolvedError = (err: any) => err.message.includes("Can't resolve")
import { isAppRouteRoute } from '../../../lib/is-app-route-route'

const FILE_TYPES = {
layout: 'layout',
Expand All @@ -25,6 +26,10 @@ const FILE_TYPES = {
const GLOBAL_ERROR_FILE_TYPE = 'global-error'
const PAGE_SEGMENT = 'page$'

type PathResolver = (
pathname: string,
resolveDir?: boolean
) => Promise<string | undefined>
export type ComponentsType = {
readonly [componentKey in ValueOf<typeof FILE_TYPES>]?: ModuleReference
} & {
Expand All @@ -33,6 +38,35 @@ export type ComponentsType = {
readonly metadata?: CollectedMetadata
}

async function createAppRouteCode({
pagePath,
resolver,
}: {
pagePath: string
resolver: PathResolver
}): Promise<string> {
// Split based on any specific path separators (both `/` and `\`)...
const splittedPath = pagePath.split(/[\\/]/)
// Then join all but the last part with the same separator, `/`...
const segmentPath = splittedPath.slice(0, -1).join('/')
// Then add the `/route` suffix...
const matchedPagePath = `${segmentPath}/route`
// This, when used with the resolver will give us the pathname to the built
// route handler file.
const resolvedPagePath = await resolver(matchedPagePath)

// TODO: verify if other methods need to be injected
// TODO: validate that the handler exports at least one of the supported methods

return `
import 'next/dist/server/node-polyfill-headers'

export * as handlers from ${JSON.stringify(resolvedPagePath)}

export { requestAsyncStorage } from 'next/dist/client/components/request-async-storage'
`
}

async function createTreeCodeFromPath(
pagePath: string,
{
Expand Down Expand Up @@ -279,8 +313,7 @@ const nextAppLoader: AppLoader = async function nextAppLoader() {

return Object.entries(matched)
}

const resolver = async (pathname: string, resolveDir?: boolean) => {
const resolver: PathResolver = async (pathname, resolveDir) => {
if (resolveDir) {
return createAbsolutePath(appDir, pathname)
}
Expand All @@ -302,6 +335,10 @@ const nextAppLoader: AppLoader = async function nextAppLoader() {
}
}

if (isAppRouteRoute(name)) {
return createAppRouteCode({ pagePath, resolver })
}

const {
treeCode,
pages: pageListCode,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { IncomingMessage, ServerResponse } from 'http'
import type { Rewrite } from '../../../../lib/load-custom-routes'
import type { BuildManifest } from '../../../../server/get-page-files'
import type { RouteMatch } from '../../../../shared/lib/router/utils/route-matcher'
import type { RouteMatchFn } from '../../../../shared/lib/router/utils/route-matcher'
import type { NextConfig } from '../../../../server/config'
import type {
GetServerSideProps,
Expand Down Expand Up @@ -144,7 +144,7 @@ export function getUtils({
trailingSlash?: boolean
}) {
let defaultRouteRegex: ReturnType<typeof getNamedRouteRegex> | undefined
let dynamicRouteMatcher: RouteMatch | undefined
let dynamicRouteMatcher: RouteMatchFn | undefined
let defaultRouteMatches: ParsedUrlQuery | undefined

if (pageIsDynamic) {
Expand Down
5 changes: 3 additions & 2 deletions packages/next/src/client/components/layout-router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { ErrorBoundary } from './error-boundary'
import { matchSegment } from './match-segments'
import { useRouter } from './navigation'
import { handleSmoothScroll } from '../../shared/lib/router/utils/handle-smooth-scroll'
import { getURLFromRedirectError, isRedirectError } from './redirect'

/**
* Add refetch marker to router state at the point of the current layout segment.
Expand Down Expand Up @@ -380,8 +381,8 @@ class RedirectErrorBoundary extends React.Component<
}

static getDerivedStateFromError(error: any) {
if (error?.digest?.startsWith('NEXT_REDIRECT')) {
const url = error.digest.split(';')[1]
if (isRedirectError(error)) {
const url = getURLFromRedirectError(error)
return { redirect: url }
}
// Re-throw if error is not for redirect
Expand Down
21 changes: 19 additions & 2 deletions packages/next/src/client/components/not-found.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,25 @@
export const NOT_FOUND_ERROR_CODE = 'NEXT_NOT_FOUND'
const NOT_FOUND_ERROR_CODE = 'NEXT_NOT_FOUND'

type NotFoundError = Error & { digest: typeof NOT_FOUND_ERROR_CODE }

/**
* When used in a React server component, this will set the status code to 404.
* When used in a custom app route it will just send a 404 status.
*/
export function notFound(): never {
// eslint-disable-next-line no-throw-literal
const error = new Error(NOT_FOUND_ERROR_CODE)
;(error as any).digest = NOT_FOUND_ERROR_CODE
;(error as NotFoundError).digest = NOT_FOUND_ERROR_CODE
throw error
}

/**
* Checks an error to determine if it's an error generated by the `notFound()`
* helper.
*
* @param error the error that may reference a not found error
* @returns true if the error is a not found error
*/
export function isNotFoundError(error: any): error is NotFoundError {
return error?.digest === NOT_FOUND_ERROR_CODE
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { useEffect } from 'react'
import { isNotFoundError } from '../../../not-found'
import { isRedirectError } from '../../../redirect'
import {
hydrationErrorWarning,
hydrationErrorComponentStack,
Expand All @@ -12,10 +14,7 @@ export const RuntimeErrorHandler = {

function isNextRouterError(error: any): boolean {
return (
error &&
error.digest &&
(error.digest.startsWith('NEXT_REDIRECT') ||
error.digest === 'NEXT_NOT_FOUND')
error && error.digest && (isRedirectError(error) || isNotFoundError(error))
)
}

Expand Down
6 changes: 3 additions & 3 deletions packages/next/src/client/components/redirect.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/* eslint-disable jest/no-try-expect */
import { redirect, REDIRECT_ERROR_CODE } from './redirect'
import { getURLFromRedirectError, isRedirectError, redirect } from './redirect'
describe('test', () => {
it('should throw a redirect error', () => {
try {
redirect('/dashboard')
throw new Error('did not throw')
} catch (err: any) {
expect(err.message).toBe(REDIRECT_ERROR_CODE)
expect(err.digest).toBe(`${REDIRECT_ERROR_CODE};/dashboard`)
expect(isRedirectError(err)).toBeTruthy()
expect(getURLFromRedirectError(err)).toEqual('/dashboard')
}
})
})
51 changes: 49 additions & 2 deletions packages/next/src/client/components/redirect.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,55 @@
export const REDIRECT_ERROR_CODE = 'NEXT_REDIRECT'
const REDIRECT_ERROR_CODE = 'NEXT_REDIRECT'

type RedirectError<U extends string> = Error & {
digest: `${typeof REDIRECT_ERROR_CODE};${U}`
}

/**
* When used in a React server component, this will insert a meta tag to
* redirect the user to the target page. When used in a custom app route, it
* will serve a 302 to the caller.
*
* @param url the url to redirect to
*/
export function redirect(url: string): never {
// eslint-disable-next-line no-throw-literal
const error = new Error(REDIRECT_ERROR_CODE)
;(error as any).digest = REDIRECT_ERROR_CODE + ';' + url
;(error as RedirectError<typeof url>).digest = `${REDIRECT_ERROR_CODE};${url}`
throw error
}

/**
* Checks an error to determine if it's an error generated by the
* `redirect(url)` helper.
*
* @param error the error that may reference a redirect error
* @returns true if the error is a redirect error
*/
export function isRedirectError<U extends string>(
error: any
): error is RedirectError<U> {
return (
typeof error?.digest === 'string' &&
error.digest.startsWith(REDIRECT_ERROR_CODE + ';') &&
error.digest.length > REDIRECT_ERROR_CODE.length + 1
)
}

/**
* Returns the encoded URL from the error if it's a RedirectError, null
* otherwise. Note that this does not validate the URL returned.
*
* @param error the error that may be a redirect error
* @return the url if the error was a redirect error
*/
export function getURLFromRedirectError<U extends string>(
error: RedirectError<U>
): U
export function getURLFromRedirectError(error: any): string | null
export function getURLFromRedirectError(error: any): string | null {
if (!isRedirectError(error)) return null

// Slices off the beginning of the digest that contains the code and the
// separating ';'.
return error.digest.slice(REDIRECT_ERROR_CODE.length + 1)
}
10 changes: 5 additions & 5 deletions packages/next/src/export/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ import RenderResult from '../server/render-result'
import isError from '../lib/is-error'
import { addRequestMeta } from '../server/request-meta'
import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'
import { REDIRECT_ERROR_CODE } from '../client/components/redirect'
import { DYNAMIC_ERROR_CODE } from '../client/components/hooks-server-context'
import { NOT_FOUND_ERROR_CODE } from '../client/components/not-found'
import { NEXT_DYNAMIC_NO_SSR_CODE } from '../shared/lib/lazy-dynamic/no-ssr-error'
import { IncrementalCache } from '../server/lib/incremental-cache'
import { isNotFoundError } from '../client/components/not-found'
import { isRedirectError } from '../client/components/redirect'
import { NEXT_DYNAMIC_NO_SSR_CODE } from '../shared/lib/lazy-dynamic/no-ssr-error'

loadRequireHook()

Expand Down Expand Up @@ -391,9 +391,9 @@ export default async function exportPage({
} catch (err: any) {
if (
err.digest !== DYNAMIC_ERROR_CODE &&
err.digest !== NOT_FOUND_ERROR_CODE &&
!isNotFoundError(err) &&
err.digest !== NEXT_DYNAMIC_NO_SSR_CODE &&
!err.digest?.startsWith(REDIRECT_ERROR_CODE)
!isRedirectError(err)
) {
throw err
}
Expand Down
3 changes: 3 additions & 0 deletions packages/next/src/lib/is-app-page-route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function isAppPageRoute(route: string): boolean {
return route.endsWith('/page')
}
3 changes: 3 additions & 0 deletions packages/next/src/lib/is-app-route-route.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function isAppRouteRoute(route: string): boolean {
return route.endsWith('/route')
}
Loading