From 1415609870fcdea2d0c0a1ab210a26f9dcb4859b Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 28 May 2024 13:55:12 +0200 Subject: [PATCH 1/4] Reland bunling webpack middleware changes (#66049) (#66052) Revert the revert in #66049 It was erroring in pages api with importing `react-dom/server` as this is disallowed in app but shouldn't be in pages. It's caused by we're validating middleware layer as server components but edge pages api is still bundled in the same layer, where we shouldn't apply the check. * Separate the api in api layers, and while handling middleware warnings, checking api layer as well * No need to check layers while handling externals in edge compiler * Found a bug that we shouldn't check if `config.transpilePackages` is defined then we enable `externalDir`, removed that condition. It fails the telemetry tests case build with code change from this PR. Add more tests for pages dir and middleware | | `react` condition | `react-dom/server` condition | | ---- | ---- | ---- | | middleware (edge) | react-server | not allowed, failed with dev/build checks | | pages/api edge | default condition | default condition | | pages/api node | default condition | default condition | --- packages/next/src/build/entries.ts | 11 ++-- packages/next/src/build/handle-externals.ts | 4 +- packages/next/src/build/utils.ts | 13 +++- packages/next/src/build/webpack-config.ts | 33 +++++----- .../webpack/plugins/middleware-plugin.ts | 11 +++- packages/next/src/lib/constants.ts | 7 ++- test/e2e/module-layer/middleware.js | 13 +++- test/e2e/module-layer/module-layer.test.ts | 60 ++++++++++++++----- .../module-layer/pages/api/default-edge.js | 11 ++++ test/e2e/module-layer/pages/api/default.js | 9 +++ test/e2e/module-layer/pages/api/hello-edge.js | 7 --- test/e2e/module-layer/pages/api/hello.js | 5 -- .../pages/api/server-only-edge.js | 12 ++++ .../e2e/module-layer/pages/api/server-only.js | 10 ++++ 14 files changed, 151 insertions(+), 55 deletions(-) create mode 100644 test/e2e/module-layer/pages/api/default-edge.js create mode 100644 test/e2e/module-layer/pages/api/default.js delete mode 100644 test/e2e/module-layer/pages/api/hello-edge.js delete mode 100644 test/e2e/module-layer/pages/api/hello.js create mode 100644 test/e2e/module-layer/pages/api/server-only-edge.js create mode 100644 test/e2e/module-layer/pages/api/server-only.js diff --git a/packages/next/src/build/entries.ts b/packages/next/src/build/entries.ts index 784d1aead8fb2..f7310dc3d24cd 100644 --- a/packages/next/src/build/entries.ts +++ b/packages/next/src/build/entries.ts @@ -812,11 +812,14 @@ export function finalizeEntrypoint({ } } case COMPILER_NAMES.edgeServer: { + const layer = isApi + ? WEBPACK_LAYERS.api + : isMiddlewareFilename(name) || isInstrumentation + ? WEBPACK_LAYERS.middleware + : undefined + return { - layer: - isMiddlewareFilename(name) || isApi || isInstrumentation - ? WEBPACK_LAYERS.middleware - : undefined, + layer, library: { name: ['_ENTRIES', `middleware_[name]`], type: 'assign' }, runtime: EDGE_RUNTIME_WEBPACK, asyncChunks: false, diff --git a/packages/next/src/build/handle-externals.ts b/packages/next/src/build/handle-externals.ts index 9ca94214035f3..5b38dd26e0994 100644 --- a/packages/next/src/build/handle-externals.ts +++ b/packages/next/src/build/handle-externals.ts @@ -10,7 +10,7 @@ import { NODE_ESM_RESOLVE_OPTIONS, NODE_RESOLVE_OPTIONS, } from './webpack-config' -import { isWebpackAppLayer, isWebpackServerOnlyLayer } from './utils' +import { isWebpackBundledLayer, isWebpackServerOnlyLayer } from './utils' import { normalizePathSep } from '../shared/lib/page-path/normalize-path-sep' const reactPackagesRegex = /^(react|react-dom|react-server-dom-webpack)($|\/)/ @@ -174,7 +174,7 @@ export function makeExternalHandler({ return `commonjs next/dist/lib/import-next-warning` } - const isAppLayer = isWebpackAppLayer(layer) + const isAppLayer = isWebpackBundledLayer(layer) // Relative requires don't need custom resolution, because they // are relative to requests we've already resolved here. diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index bcfd2c5643381..4c841865a9b8c 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -2256,6 +2256,15 @@ export function getSupportedBrowsers( return MODERN_BROWSERSLIST_TARGET } +// Use next/dist/compiled/react packages instead of installed react +export function isWebpackBuiltinReactLayer( + layer: WebpackLayerName | null | undefined +): boolean { + return Boolean( + layer && WEBPACK_LAYERS.GROUP.builtinReact.includes(layer as any) + ) +} + export function isWebpackServerOnlyLayer( layer: WebpackLayerName | null | undefined ): boolean { @@ -2278,8 +2287,8 @@ export function isWebpackDefaultLayer( return layer === null || layer === undefined } -export function isWebpackAppLayer( +export function isWebpackBundledLayer( layer: WebpackLayerName | null | undefined ): boolean { - return Boolean(layer && WEBPACK_LAYERS.GROUP.app.includes(layer as any)) + return Boolean(layer && WEBPACK_LAYERS.GROUP.bundled.includes(layer as any)) } diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 42ceb0368f877..4b1b92e4992ec 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -9,7 +9,8 @@ import { escapeStringRegexp } from '../shared/lib/escape-regexp' import { WEBPACK_LAYERS, WEBPACK_RESOURCE_QUERIES } from '../lib/constants' import type { WebpackLayerName } from '../lib/constants' import { - isWebpackAppLayer, + isWebpackBuiltinReactLayer, + isWebpackBundledLayer, isWebpackClientOnlyLayer, isWebpackDefaultLayer, isWebpackServerOnlyLayer, @@ -409,8 +410,7 @@ export default async function getBaseWebpackConfig( loggedIgnoredCompilerOptions = true } - const shouldIncludeExternalDirs = - config.experimental.externalDir || !!config.transpilePackages + const shouldIncludeExternalDirs = config.experimental.externalDir const codeCondition = { test: { or: [/\.(tsx|ts|js|cjs|mjs|jsx)$/, /__barrel_optimize__/] }, ...(shouldIncludeExternalDirs @@ -543,7 +543,7 @@ export default async function getBaseWebpackConfig( // This will cause some performance overhead but // acceptable as Babel will not be recommended. getSwcLoader({ - serverComponents: false, + serverComponents: true, bundleLayer: WEBPACK_LAYERS.middleware, }), babelLoader, @@ -592,13 +592,12 @@ export default async function getBaseWebpackConfig( // Loader for API routes needs to be differently configured as it shouldn't // have RSC transpiler enabled, so syntax checks such as invalid imports won't // be performed. - const apiRoutesLayerLoaders = - hasAppDir && useSWCLoader - ? getSwcLoader({ - serverComponents: false, - bundleLayer: WEBPACK_LAYERS.api, - }) - : defaultLoaders.babel + const apiRoutesLayerLoaders = useSWCLoader + ? getSwcLoader({ + serverComponents: false, + bundleLayer: WEBPACK_LAYERS.api, + }) + : defaultLoaders.babel const pageExtensions = config.pageExtensions @@ -1304,7 +1303,7 @@ export default async function getBaseWebpackConfig( test: /next[\\/]dist[\\/](esm[\\/])?server[\\/]future[\\/]route-modules[\\/]app-page[\\/]module/, }, { - issuerLayer: isWebpackAppLayer, + issuerLayer: isWebpackBundledLayer, resolve: { alias: createNextApiEsmAliases(), }, @@ -1326,7 +1325,7 @@ export default async function getBaseWebpackConfig( ...(hasAppDir && !isClient ? [ { - issuerLayer: isWebpackServerOnlyLayer, + issuerLayer: isWebpackBuiltinReactLayer, test: { // Resolve it if it is a source code file, and it has NOT been // opted out of bundling. @@ -1388,7 +1387,7 @@ export default async function getBaseWebpackConfig( // Alias react for switching between default set and share subset. oneOf: [ { - issuerLayer: isWebpackServerOnlyLayer, + issuerLayer: isWebpackBuiltinReactLayer, test: { // Resolve it if it is a source code file, and it has NOT been // opted out of bundling. @@ -1469,11 +1468,17 @@ export default async function getBaseWebpackConfig( test: codeCondition.test, issuerLayer: WEBPACK_LAYERS.middleware, use: middlewareLayerLoaders, + resolve: { + conditionNames: reactServerCondition, + }, }, { test: codeCondition.test, issuerLayer: WEBPACK_LAYERS.instrument, use: instrumentLayerLoaders, + resolve: { + conditionNames: reactServerCondition, + }, }, ...(hasAppDir ? [ diff --git a/packages/next/src/build/webpack/plugins/middleware-plugin.ts b/packages/next/src/build/webpack/plugins/middleware-plugin.ts index 866aff2ae1a32..e6bdccd6da88e 100644 --- a/packages/next/src/build/webpack/plugins/middleware-plugin.ts +++ b/packages/next/src/build/webpack/plugins/middleware-plugin.ts @@ -28,7 +28,10 @@ import type { Telemetry } from '../../../telemetry/storage' import { traceGlobals } from '../../../trace/shared' import { EVENT_BUILD_FEATURE_USAGE } from '../../../telemetry/events' import { normalizeAppPath } from '../../../shared/lib/router/utils/app-paths' -import { INSTRUMENTATION_HOOK_FILENAME } from '../../../lib/constants' +import { + INSTRUMENTATION_HOOK_FILENAME, + WEBPACK_LAYERS, +} from '../../../lib/constants' import type { CustomRoutes } from '../../../lib/load-custom-routes' import { isInterceptionRouteRewrite } from '../../../lib/generate-interception-routes-rewrites' import { getDynamicCodeEvaluationError } from './wellknown-errors-plugin/parse-dynamic-code-evaluation-error' @@ -272,7 +275,8 @@ function buildWebpackError({ } function isInMiddlewareLayer(parser: webpack.javascript.JavascriptParser) { - return parser.state.module?.layer === 'middleware' + const layer = parser.state.module?.layer + return layer === WEBPACK_LAYERS.middleware || layer === WEBPACK_LAYERS.api } function isNodeJsModule(moduleName: string) { @@ -849,7 +853,8 @@ export async function handleWebpackExternalForEdgeRuntime({ getResolve: () => any }) { if ( - contextInfo.issuerLayer === 'middleware' && + (contextInfo.issuerLayer === WEBPACK_LAYERS.middleware || + contextInfo.issuerLayer === WEBPACK_LAYERS.api) && isNodeJsModule(request) && !supportedEdgePolyfills.has(request) ) { diff --git a/packages/next/src/lib/constants.ts b/packages/next/src/lib/constants.ts index c250a65b4afbb..c098c8da62499 100644 --- a/packages/next/src/lib/constants.ts +++ b/packages/next/src/lib/constants.ts @@ -159,6 +159,11 @@ export type WebpackLayerName = const WEBPACK_LAYERS = { ...WEBPACK_LAYERS_NAMES, GROUP: { + builtinReact: [ + WEBPACK_LAYERS_NAMES.reactServerComponents, + WEBPACK_LAYERS_NAMES.actionBrowser, + WEBPACK_LAYERS_NAMES.appMetadataRoute, + ], serverOnly: [ WEBPACK_LAYERS_NAMES.reactServerComponents, WEBPACK_LAYERS_NAMES.actionBrowser, @@ -174,7 +179,7 @@ const WEBPACK_LAYERS = { WEBPACK_LAYERS_NAMES.serverSideRendering, WEBPACK_LAYERS_NAMES.appPagesBrowser, ], - app: [ + bundled: [ WEBPACK_LAYERS_NAMES.reactServerComponents, WEBPACK_LAYERS_NAMES.actionBrowser, WEBPACK_LAYERS_NAMES.appMetadataRoute, diff --git a/test/e2e/module-layer/middleware.js b/test/e2e/module-layer/middleware.js index 8a4d11761dd78..cd3fc531ccbe4 100644 --- a/test/e2e/module-layer/middleware.js +++ b/test/e2e/module-layer/middleware.js @@ -1,11 +1,20 @@ import 'server-only' -import React from 'react' +import * as React from 'react' import { NextResponse } from 'next/server' // import './lib/mixed-lib' export function middleware(request) { - if (React.useState) { + // To avoid webpack ESM exports checking warning + const ReactObject = Object(React) + if (ReactObject.useState) { throw new Error('React.useState should not be defined in server layer') } + + if (request.nextUrl.pathname === '/react-version') { + return Response.json({ + React: Object.keys(ReactObject), + }) + } + return NextResponse.next() } diff --git a/test/e2e/module-layer/module-layer.test.ts b/test/e2e/module-layer/module-layer.test.ts index bf665e1428df8..7d90ea9fef659 100644 --- a/test/e2e/module-layer/module-layer.test.ts +++ b/test/e2e/module-layer/module-layer.test.ts @@ -4,6 +4,10 @@ import { getRedboxSource, hasRedbox, retry } from 'next-test-utils' describe('module layer', () => { const { next, isNextStart, isNextDev, isTurbopack } = nextTestSetup({ files: __dirname, + dependencies: { + react: '19.0.0-rc-915b914b3a-20240515', + 'react-dom': '19.0.0-rc-915b914b3a-20240515', + }, }) function runTests() { @@ -18,8 +22,10 @@ describe('module layer', () => { '/app/route', '/app/route-edge', // pages/api - '/api/hello', - '/api/hello-edge', + '/api/default', + '/api/default-edge', + '/api/server-only', + '/api/server-only-edge', '/api/mixed', ] @@ -30,6 +36,35 @@ describe('module layer', () => { }) } + it('should render installed react-server condition for middleware', async () => { + const json = await next.fetch('/react-version').then((res) => res.json()) + expect(json.React).toContain('version') // basic react-server export + expect(json.React).not.toContain('useEffect') // no client api export + }) + + // This is for backward compatibility, don't change react usage in existing pages/api + it('should contain client react exports for pages api', async () => { + async function verifyReactExports(route, isEdge) { + const json = await next.fetch(route).then((res) => res.json()) + // contain all react-server and default condition exports + expect(json.React).toContain('version') + expect(json.React).toContain('useEffect') + + // contain react-dom-server default condition exports + expect(json.ReactDomServer).toContain('version') + expect(json.ReactDomServer).toContain('renderToString') + expect(json.ReactDomServer).toContain('renderToStaticMarkup') + expect(json.ReactDomServer).toContain( + isEdge ? 'renderToReadableStream' : 'renderToPipeableStream' + ) + } + + await verifyReactExports('/api/default', false) + await verifyReactExports('/api/default-edge', true) + await verifyReactExports('/api/server-only', false) + await verifyReactExports('/api/server-only-edge', true) + }) + if (isNextStart) { it('should log the build info properly', async () => { const cliOutput = next.cliOutput @@ -40,7 +75,8 @@ describe('module layer', () => { ) expect(functionsManifest.functions).toContainKeys([ '/app/route-edge', - '/api/hello-edge', + '/api/default-edge', + '/api/server-only-edge', '/app/client-edge', '/app/server-edge', ]) @@ -52,9 +88,10 @@ describe('module layer', () => { ) expect(middlewareManifest.middleware).toBeTruthy() expect(pagesManifest).toContainKeys([ - '/api/hello-edge', + '/api/default-edge', '/pages-ssr', - '/api/hello', + '/api/default', + '/api/server-only', ]) }) } @@ -81,22 +118,15 @@ describe('module layer', () => { .replace("// import './lib/mixed-lib'", "import './lib/mixed-lib'") ) - const existingCliOutputLength = next.cliOutput.length await retry(async () => { expect(await hasRedbox(browser)).toBe(true) const source = await getRedboxSource(browser) expect(source).toContain( - `'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.` + isTurbopack + ? `'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component.` + : `You're importing a component that imports client-only. It only works in a Client Component but none of its parents are marked with "use client"` ) }) - - if (!isTurbopack) { - const newCliOutput = next.cliOutput.slice(existingCliOutputLength) - expect(newCliOutput).toContain('./middleware.js') - expect(newCliOutput).toContain( - `'client-only' cannot be imported from a Server Component module. It should only be used from a Client Component` - ) - } }) }) } diff --git a/test/e2e/module-layer/pages/api/default-edge.js b/test/e2e/module-layer/pages/api/default-edge.js new file mode 100644 index 0000000000000..f4621f2500cc5 --- /dev/null +++ b/test/e2e/module-layer/pages/api/default-edge.js @@ -0,0 +1,11 @@ +import * as ReactDomServer from 'react-dom/server' +import * as React from 'react' + +export default async (_req) => { + return Response.json({ + React: Object.keys(Object(React)), + ReactDomServer: Object.keys(Object(ReactDomServer)), + }) +} + +export const runtime = 'edge' diff --git a/test/e2e/module-layer/pages/api/default.js b/test/e2e/module-layer/pages/api/default.js new file mode 100644 index 0000000000000..2b13630aa4ac6 --- /dev/null +++ b/test/e2e/module-layer/pages/api/default.js @@ -0,0 +1,9 @@ +import * as ReactDomServer from 'react-dom/server' +import * as React from 'react' + +export default async (_req, res) => { + return res.json({ + React: Object.keys(Object(React)), + ReactDomServer: Object.keys(Object(ReactDomServer)), + }) +} diff --git a/test/e2e/module-layer/pages/api/hello-edge.js b/test/e2e/module-layer/pages/api/hello-edge.js deleted file mode 100644 index dce0100296d19..0000000000000 --- a/test/e2e/module-layer/pages/api/hello-edge.js +++ /dev/null @@ -1,7 +0,0 @@ -import 'server-only' - -export default function handler() { - return new Response('pages/api/hello-edge.js:') -} - -export const runtime = 'edge' diff --git a/test/e2e/module-layer/pages/api/hello.js b/test/e2e/module-layer/pages/api/hello.js deleted file mode 100644 index d1fe5339d8e98..0000000000000 --- a/test/e2e/module-layer/pages/api/hello.js +++ /dev/null @@ -1,5 +0,0 @@ -import 'server-only' - -export default function handler(req, res) { - return res.send('pages/api/hello.js') -} diff --git a/test/e2e/module-layer/pages/api/server-only-edge.js b/test/e2e/module-layer/pages/api/server-only-edge.js new file mode 100644 index 0000000000000..17e682c015f4f --- /dev/null +++ b/test/e2e/module-layer/pages/api/server-only-edge.js @@ -0,0 +1,12 @@ +import 'server-only' +import * as ReactDomServer from 'react-dom/server' +import * as React from 'react' + +export default async (_req) => { + return Response.json({ + React: Object.keys(Object(React)), + ReactDomServer: Object.keys(Object(ReactDomServer)), + }) +} + +export const runtime = 'edge' diff --git a/test/e2e/module-layer/pages/api/server-only.js b/test/e2e/module-layer/pages/api/server-only.js new file mode 100644 index 0000000000000..7f276bdf9c846 --- /dev/null +++ b/test/e2e/module-layer/pages/api/server-only.js @@ -0,0 +1,10 @@ +import 'server-only' +import * as ReactDomServer from 'react-dom/server' +import * as React from 'react' + +export default async (_req, res) => { + return res.json({ + React: Object.keys(Object(React)), + ReactDomServer: Object.keys(Object(ReactDomServer)), + }) +} From 1b36d7615592187ed0017c19a7fcfab2d3431d9b Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 28 May 2024 16:26:40 +0200 Subject: [PATCH 2/4] fix dev overlay version indicator style (#66278) Fix the Build Error indicator location, it should be located on the top-right corner. Align the padding with ### Build Error ![image](https://github.com/vercel/next.js/assets/4800338/835506a5-bf29-4f3e-ae0e-091dc9a7b386) ### Runtime Error ![image](https://github.com/vercel/next.js/assets/4800338/2a61a933-58f2-471e-9d3f-84b3954b1460) --- .../VersionStalenessInfo.tsx | 17 ++++++++++------- .../components/VersionStalenessInfo/styles.ts | 1 - .../internal/container/BuildError.tsx | 2 +- .../internal/container/Errors.tsx | 11 ++++++++++- .../root-layout-missing-tags-error.tsx | 2 +- 5 files changed, 22 insertions(+), 11 deletions(-) diff --git a/packages/next/src/client/components/react-dev-overlay/internal/components/VersionStalenessInfo/VersionStalenessInfo.tsx b/packages/next/src/client/components/react-dev-overlay/internal/components/VersionStalenessInfo/VersionStalenessInfo.tsx index 9cdcf6838c86e..c6d90b16245fa 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/components/VersionStalenessInfo/VersionStalenessInfo.tsx +++ b/packages/next/src/client/components/react-dev-overlay/internal/components/VersionStalenessInfo/VersionStalenessInfo.tsx @@ -1,15 +1,18 @@ -import React from 'react' import type { VersionInfo } from '../../../../../../server/dev/parse-version-info' -export function VersionStalenessInfo(props: VersionInfo) { - if (!props) return null - const { staleness } = props - let { text, indicatorClass, title } = getStaleness(props) +export function VersionStalenessInfo({ + versionInfo, +}: { + versionInfo: VersionInfo | undefined +}) { + if (!versionInfo) return null + const { staleness } = versionInfo + let { text, indicatorClass, title } = getStaleness(versionInfo) if (!text) return null return ( - + {text} @@ -26,7 +29,7 @@ export function VersionStalenessInfo(props: VersionInfo) { )} {process.env.TURBOPACK ? ' (turbo)' : ''} - + ) } diff --git a/packages/next/src/client/components/react-dev-overlay/internal/components/VersionStalenessInfo/styles.ts b/packages/next/src/client/components/react-dev-overlay/internal/components/VersionStalenessInfo/styles.ts index 61f32b5c40b73..94d19dea738ee 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/components/VersionStalenessInfo/styles.ts +++ b/packages/next/src/client/components/react-dev-overlay/internal/components/VersionStalenessInfo/styles.ts @@ -6,7 +6,6 @@ const styles = css` text-align: right; } .nextjs-container-build-error-version-status small { - margin-left: var(--size-gap); font-size: var(--size-font-small); } .nextjs-container-build-error-version-status a { diff --git a/packages/next/src/client/components/react-dev-overlay/internal/container/BuildError.tsx b/packages/next/src/client/components/react-dev-overlay/internal/container/BuildError.tsx index 3e653b699410a..960a471434129 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/container/BuildError.tsx +++ b/packages/next/src/client/components/react-dev-overlay/internal/container/BuildError.tsx @@ -29,13 +29,13 @@ export const BuildError: React.FC = function BuildError({

{'Build Error'}

+

Failed to compile

- {versionInfo ? : null} diff --git a/packages/next/src/client/components/react-dev-overlay/internal/container/Errors.tsx b/packages/next/src/client/components/react-dev-overlay/internal/container/Errors.tsx index 9a9a9858300be..1c15d8d8cb76a 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/container/Errors.tsx +++ b/packages/next/src/client/components/react-dev-overlay/internal/container/Errors.tsx @@ -265,7 +265,7 @@ export function Errors({ {' error'} {readyErrors.length < 2 ? '' : 's'} - {versionInfo ? : null} +

{isServerError ? 'Server Error' : 'Unhandled Runtime Error'} @@ -323,6 +323,9 @@ export function Errors({ } export const styles = css` + .nextjs-container-errors-header { + position: relative; + } .nextjs-container-errors-header > h1 { font-size: var(--size-font-big); line-height: var(--size-font-bigger); @@ -406,4 +409,10 @@ export const styles = css` .nextjs-toast-errors-hide-button:hover { opacity: 1; } + .nextjs-container-errors-header + > .nextjs-container-build-error-version-status { + position: absolute; + top: 0; + right: 0; + } ` diff --git a/packages/next/src/client/components/react-dev-overlay/internal/container/root-layout-missing-tags-error.tsx b/packages/next/src/client/components/react-dev-overlay/internal/container/root-layout-missing-tags-error.tsx index 517c061a6c610..31134d529b58a 100644 --- a/packages/next/src/client/components/react-dev-overlay/internal/container/root-layout-missing-tags-error.tsx +++ b/packages/next/src/client/components/react-dev-overlay/internal/container/root-layout-missing-tags-error.tsx @@ -23,10 +23,10 @@ export const RootLayoutMissingTagsError: React.FC +

Missing required html tags

- {versionInfo ? : null}

Date: Tue, 28 May 2024 08:53:04 -0600 Subject: [PATCH 3/4] [ppr] Don't mark RSC requests as /_next/data requests (#66249) Old logic from the pages router was previously being hit during development. This was more apparent when PPR was enabled as it was mixing dynamic + static rendering in development which propagated to errors. This change ensures that requests that are made with `RSC: 1` are not marked as `/_next/data` URL's, and don't use the same logic paths. Previously it was a bit confusing because we used the variable `isDataReq` in a few places that made it hard to tell what it was referring to. In this case, the `isDataReq` is actually only used by the pages router. This renames the `isDataReq` to `isNextDataRequest` to make it clearer, as well as refactors to ensure that it's not used in the paths for app routes. Also to better represent the rendering modes the `supportsDynamicHTML` variable was renamed to `supportsDynamicResponse`. Fixes #66241 --- lint-staged.config.js | 7 +- packages/next/src/build/utils.ts | 2 +- .../loaders/next-edge-ssr-loader/render.ts | 2 +- packages/next/src/export/index.ts | 2 +- packages/next/src/export/routes/app-route.ts | 2 +- packages/next/src/export/worker.ts | 2 +- .../next/src/server/app-render/app-render.tsx | 4 +- packages/next/src/server/app-render/types.ts | 2 +- ...static-generation-async-storage-wrapper.ts | 4 +- packages/next/src/server/base-server.ts | 114 +++++++----------- packages/next/src/server/next-server.ts | 4 +- packages/next/src/server/render.tsx | 8 +- packages/next/src/server/request-meta.ts | 5 + packages/next/src/server/web/adapter.ts | 10 +- .../server/web/edge-route-module-wrapper.ts | 2 +- .../simple/app/[locale]/about/page.jsx | 5 + .../simple/app/[locale]/layout.jsx | 13 ++ .../simple/app/[locale]/page.jsx | 5 + .../ppr-navigations/simple/app/layout.jsx | 3 + .../ppr-navigations/simple/app/page.jsx | 7 ++ .../simple/components/page.jsx | 40 ++++++ .../ppr-navigations/simple/next.config.js | 5 + .../ppr-navigations/simple/simple.test.ts | 31 +++++ 23 files changed, 186 insertions(+), 93 deletions(-) create mode 100644 test/e2e/app-dir/ppr-navigations/simple/app/[locale]/about/page.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/app/[locale]/layout.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/app/[locale]/page.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/app/layout.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/app/page.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/components/page.jsx create mode 100644 test/e2e/app-dir/ppr-navigations/simple/next.config.js create mode 100644 test/e2e/app-dir/ppr-navigations/simple/simple.test.ts diff --git a/lint-staged.config.js b/lint-staged.config.js index c08b7f7edd03f..de8a5a2d9b0f3 100644 --- a/lint-staged.config.js +++ b/lint-staged.config.js @@ -30,7 +30,12 @@ module.exports = { return [ `prettier --with-node-modules --ignore-path .prettierignore --write ${escapedFileNames}`, - `eslint --no-ignore --max-warnings=0 --fix ${escape(eslintFileNames.filter((filename) => filename !== null)).join(' ')}`, + `eslint --no-ignore --max-warnings=0 --fix ${eslintFileNames + .filter((filename) => filename !== null) + .map((filename) => { + return `"${filename}"` + }) + .join(' ')}`, `git add ${escapedFileNames}`, ] }, diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index 4c841865a9b8c..7cf951f5db51b 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -1389,7 +1389,7 @@ export async function buildAppStaticPaths({ renderOpts: { originalPathname: page, incrementalCache, - supportsDynamicHTML: true, + supportsDynamicResponse: true, isRevalidate: false, experimental: { after: false, diff --git a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts index 83168210d7432..8e71beaf8aefd 100644 --- a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts +++ b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts @@ -93,7 +93,7 @@ export function getRender({ extendRenderOpts: { buildId, runtime: SERVER_RUNTIME.experimentalEdge, - supportsDynamicHTML: true, + supportsDynamicResponse: true, disableOptimizedLoading: true, serverActionsManifest, serverActions, diff --git a/packages/next/src/export/index.ts b/packages/next/src/export/index.ts index df484885448f2..ba1d9535d56e5 100644 --- a/packages/next/src/export/index.ts +++ b/packages/next/src/export/index.ts @@ -398,7 +398,7 @@ export async function exportAppImpl( domainLocales: i18n?.domains, disableOptimizedLoading: nextConfig.experimental.disableOptimizedLoading, // Exported pages do not currently support dynamic HTML. - supportsDynamicHTML: false, + supportsDynamicResponse: false, crossOrigin: nextConfig.crossOrigin, optimizeCss: nextConfig.experimental.optimizeCss, nextConfigOutput: nextConfig.output, diff --git a/packages/next/src/export/routes/app-route.ts b/packages/next/src/export/routes/app-route.ts index b860a86cdacc5..dc93d2c87124f 100644 --- a/packages/next/src/export/routes/app-route.ts +++ b/packages/next/src/export/routes/app-route.ts @@ -75,7 +75,7 @@ export async function exportAppRoute( experimental: experimental, originalPathname: page, nextExport: true, - supportsDynamicHTML: false, + supportsDynamicResponse: false, incrementalCache, waitUntil: undefined, onClose: undefined, diff --git a/packages/next/src/export/worker.ts b/packages/next/src/export/worker.ts index 2677a015259ff..9c0fc584050ff 100644 --- a/packages/next/src/export/worker.ts +++ b/packages/next/src/export/worker.ts @@ -269,7 +269,7 @@ async function exportPageImpl( disableOptimizedLoading, fontManifest: optimizeFonts ? requireFontManifest(distDir) : undefined, locale, - supportsDynamicHTML: false, + supportsDynamicResponse: false, originalPathname: page, experimental: { ...input.renderOpts.experimental, diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index dc13bb038fa43..b0bcd3b8bb7df 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -643,7 +643,7 @@ async function renderToHTMLOrFlightImpl( ComponentMod, dev, nextFontManifest, - supportsDynamicHTML, + supportsDynamicResponse, serverActions, appDirDevErrorLogger, assetPrefix = '', @@ -781,7 +781,7 @@ async function renderToHTMLOrFlightImpl( * These rules help ensure that other existing features like request caching, * coalescing, and ISR continue working as intended. */ - const generateStaticHTML = supportsDynamicHTML !== true + const generateStaticHTML = supportsDynamicResponse !== true // Pull out the hooks/references from the component. const { tree: loaderTree, taintObjectReference } = ComponentMod diff --git a/packages/next/src/server/app-render/types.ts b/packages/next/src/server/app-render/types.ts index da7a5760362a5..08c272854d129 100644 --- a/packages/next/src/server/app-render/types.ts +++ b/packages/next/src/server/app-render/types.ts @@ -129,7 +129,7 @@ export interface RenderOptsPartial { basePath: string trailingSlash: boolean clientReferenceManifest?: DeepReadonly - supportsDynamicHTML: boolean + supportsDynamicResponse: boolean runtime?: ServerRuntime serverComponents?: boolean enableTainting?: boolean diff --git a/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts b/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts index d49ce0a56aaf2..e7b71e377d54c 100644 --- a/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts +++ b/packages/next/src/server/async-storage/static-generation-async-storage-wrapper.ts @@ -41,7 +41,7 @@ export type StaticGenerationContext = { // mirrored. RenderOptsPartial, | 'originalPathname' - | 'supportsDynamicHTML' + | 'supportsDynamicResponse' | 'isRevalidate' | 'nextExport' | 'isDraftMode' @@ -77,7 +77,7 @@ export const StaticGenerationAsyncStorageWrapper: AsyncStorageWrapper< * coalescing, and ISR continue working as intended. */ const isStaticGeneration = - !renderOpts.supportsDynamicHTML && + !renderOpts.supportsDynamicResponse && !renderOpts.isDraftMode && !renderOpts.isServerAction diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts index 190cc1b485fcb..598ab6a60e386 100644 --- a/packages/next/src/server/base-server.ts +++ b/packages/next/src/server/base-server.ts @@ -527,7 +527,7 @@ export default abstract class Server< } this.renderOpts = { - supportsDynamicHTML: true, + supportsDynamicResponse: true, trailingSlash: this.nextConfig.trailingSlash, deploymentId: this.nextConfig.deploymentId, strictNextHead: this.nextConfig.experimental.strictNextHead ?? true, @@ -649,10 +649,6 @@ export default abstract class Server< return false } - // If we're here, this is a data request, as it didn't return and it matched - // either a RSC or a prefetch RSC request. - parsedUrl.query.__nextDataReq = '1' - if (req.url) { const parsed = parseUrl(req.url) parsed.pathname = parsedUrl.pathname @@ -1601,7 +1597,7 @@ export default abstract class Server< ...partialContext, renderOpts: { ...this.renderOpts, - supportsDynamicHTML: !isBotRequest, + supportsDynamicResponse: !isBotRequest, isBot: !!isBotRequest, }, } @@ -1647,7 +1643,7 @@ export default abstract class Server< ...partialContext, renderOpts: { ...this.renderOpts, - supportsDynamicHTML: false, + supportsDynamicResponse: false, }, } const payload = await fn(ctx) @@ -1946,7 +1942,7 @@ export default abstract class Server< } // Toggle whether or not this is a Data request - let isDataReq = + const isNextDataRequest = !!( query.__nextDataReq || (req.headers['x-nextjs-data'] && @@ -2056,7 +2052,7 @@ export default abstract class Server< isRoutePPREnabled && isRSCRequest && !isPrefetchRSCRequest // we need to ensure the status code if /404 is visited directly - if (is404Page && !isDataReq && !isRSCRequest) { + if (is404Page && !isNextDataRequest && !isRSCRequest) { res.statusCode = 404 } @@ -2097,7 +2093,7 @@ export default abstract class Server< // the query object. This ensures it won't be found by the `in` operator. if ('amp' in query && !query.amp) delete query.amp - if (opts.supportsDynamicHTML === true) { + if (opts.supportsDynamicResponse === true) { const isBotRequest = isBot(req.headers['user-agent'] || '') const isSupportedDocument = typeof components.Document?.getInitialProps !== 'function' || @@ -2109,19 +2105,14 @@ export default abstract class Server< // TODO-APP: should the first render for a dynamic app path // be static so we can collect revalidate and populate the // cache if there are no dynamic data requirements - opts.supportsDynamicHTML = + opts.supportsDynamicResponse = !isSSG && !isBotRequest && !query.amp && isSupportedDocument opts.isBot = isBotRequest } // In development, we always want to generate dynamic HTML. - if ( - !isDataReq && - isAppPath && - opts.dev && - opts.supportsDynamicHTML === false - ) { - opts.supportsDynamicHTML = true + if (!isNextDataRequest && isAppPath && opts.dev) { + opts.supportsDynamicResponse = true } const defaultLocale = isSSG @@ -2144,27 +2135,20 @@ export default abstract class Server< } } - if (isAppPath) { - if (!this.renderOpts.dev && !isPreviewMode && isSSG && isRSCRequest) { - // If this is an RSC request but we aren't in minimal mode, then we mark - // that this is a data request so that we can generate the flight data - // only. - if (!this.minimalMode) { - isDataReq = true - } - - // If this is a dynamic RSC request, ensure that we don't purge the - // flight headers to ensure that we will only produce the RSC response. - // We only need to do this in non-edge environments (as edge doesn't - // support static generation). - if ( - !isDynamicRSCRequest && - (!isEdgeRuntime(opts.runtime) || - (this.serverOptions as any).webServerConfig) - ) { - stripFlightHeaders(req.headers) - } - } + // If this is a request for an app path that should be statically generated + // and we aren't in the edge runtime, strip the flight headers so it will + // generate the static response. + if ( + isAppPath && + !opts.dev && + !isPreviewMode && + isSSG && + isRSCRequest && + !isDynamicRSCRequest && + (!isEdgeRuntime(opts.runtime) || + (this.serverOptions as any).webServerConfig) + ) { + stripFlightHeaders(req.headers) } let isOnDemandRevalidate = false @@ -2215,7 +2199,7 @@ export default abstract class Server< // remove /_next/data prefix from urlPathname so it matches // for direct page visit and /_next/data visit - if (isDataReq) { + if (isNextDataRequest) { resolvedUrlPathname = this.stripNextDataPath(resolvedUrlPathname) urlPathname = this.stripNextDataPath(urlPathname) } @@ -2224,7 +2208,7 @@ export default abstract class Server< if ( !isPreviewMode && isSSG && - !opts.supportsDynamicHTML && + !opts.supportsDynamicResponse && !isServerAction && !minimalPostponed && !isDynamicRSCRequest @@ -2300,10 +2284,10 @@ export default abstract class Server< const doRender: Renderer = async ({ postponed }) => { // In development, we always want to generate dynamic HTML. - let supportsDynamicHTML: boolean = - // If this isn't a data request and we're not in development, then we - // support dynamic HTML. - (!isDataReq && opts.dev === true) || + let supportsDynamicResponse: boolean = + // If we're in development, we always support dynamic HTML, unless it's + // a data request, in which case we only produce static HTML. + (!isNextDataRequest && opts.dev === true) || // If this is not SSG or does not have static paths, then it supports // dynamic HTML. (!isSSG && !hasStaticPaths) || @@ -2346,7 +2330,7 @@ export default abstract class Server< serverActions: this.nextConfig.experimental.serverActions, } : {}), - isDataReq, + isNextDataRequest, resolvedUrl, locale, locales, @@ -2367,7 +2351,7 @@ export default abstract class Server< ...opts.experimental, isRoutePPREnabled, }, - supportsDynamicHTML, + supportsDynamicResponse, isOnDemandRevalidate, isDraftMode: isPreviewMode, isServerAction, @@ -2377,9 +2361,9 @@ export default abstract class Server< } if (isDebugPPRSkeleton) { - supportsDynamicHTML = false + supportsDynamicResponse = false renderOpts.nextExport = true - renderOpts.supportsDynamicHTML = false + renderOpts.supportsDynamicResponse = false renderOpts.isStaticGeneration = true renderOpts.isRevalidate = true renderOpts.isDebugPPRSkeleton = true @@ -2411,7 +2395,7 @@ export default abstract class Server< after: renderOpts.experimental.after, }, originalPathname: components.ComponentMod.originalPathname, - supportsDynamicHTML, + supportsDynamicResponse, incrementalCache, isRevalidate: isSSG, waitUntil: this.getWaitUntil(), @@ -2725,7 +2709,7 @@ export default abstract class Server< throw new NoFallbackError() } - if (!isDataReq) { + if (!isNextDataRequest) { // Production already emitted the fallback as static HTML. if (isProduction) { const html = await this.getFallback( @@ -2859,11 +2843,11 @@ export default abstract class Server< revalidate = 0 } else if ( typeof cacheEntry.revalidate !== 'undefined' && - (!this.renderOpts.dev || (hasServerProps && !isDataReq)) + (!this.renderOpts.dev || (hasServerProps && !isNextDataRequest)) ) { // If this is a preview mode request, we shouldn't cache it. We also don't // cache 404 pages. - if (isPreviewMode || (is404Page && !isDataReq)) { + if (isPreviewMode || (is404Page && !isNextDataRequest)) { revalidate = 0 } @@ -2917,7 +2901,7 @@ export default abstract class Server< }) ) } - if (isDataReq) { + if (isNextDataRequest) { res.statusCode = 404 res.body('{"notFound":true}').send() return null @@ -2940,7 +2924,7 @@ export default abstract class Server< ) } - if (isDataReq) { + if (isNextDataRequest) { return { type: 'json', body: RenderResult.fromStatic( @@ -3015,7 +2999,7 @@ export default abstract class Server< // If the request is a data request, then we shouldn't set the status code // from the response because it should always be 200. This should be gated // behind the experimental PPR flag. - if (cachedData.status && (!isDataReq || !isRoutePPREnabled)) { + if (cachedData.status && (!isRSCRequest || !isRoutePPREnabled)) { res.statusCode = cachedData.status } @@ -3028,13 +3012,9 @@ export default abstract class Server< // as preview mode is a dynamic request (bypasses cache) and doesn't // generate both HTML and payloads in the same request so continue to just // return the generated payload - if (isDataReq && !isPreviewMode) { + if (isRSCRequest && !isPreviewMode) { // If this is a dynamic RSC request, then stream the response. - if (isDynamicRSCRequest) { - if (cachedData.pageData) { - throw new Error('Invariant: Expected pageData to be undefined') - } - + if (typeof cachedData.pageData !== 'string') { if (cachedData.postponed) { throw new Error('Invariant: Expected postponed to be undefined') } @@ -3047,16 +3027,10 @@ export default abstract class Server< // distinguishing between `force-static` and pages that have no // postponed state. // TODO: distinguish `force-static` from pages with no postponed state (static) - revalidate: 0, + revalidate: isDynamicRSCRequest ? 0 : cacheEntry.revalidate, } } - if (typeof cachedData.pageData !== 'string') { - throw new Error( - `Invariant: expected pageData to be a string, got ${typeof cachedData.pageData}` - ) - } - // As this isn't a prefetch request, we should serve the static flight // data. return { @@ -3126,7 +3100,7 @@ export default abstract class Server< // to the client on the same request. revalidate: 0, } - } else if (isDataReq) { + } else if (isNextDataRequest) { return { type: 'json', body: RenderResult.fromStatic(JSON.stringify(cachedData.pageData)), diff --git a/packages/next/src/server/next-server.ts b/packages/next/src/server/next-server.ts index e5721706faa02..bbfa7cb09ab3d 100644 --- a/packages/next/src/server/next-server.ts +++ b/packages/next/src/server/next-server.ts @@ -1856,7 +1856,7 @@ export default class NextNodeServer extends BaseServer< } // For edge to "fetch" we must always provide an absolute URL - const isDataReq = !!query.__nextDataReq + const isNextDataRequest = !!query.__nextDataReq const initialUrl = new URL( getRequestMeta(params.req, 'initURL') || '/', 'http://n' @@ -1867,7 +1867,7 @@ export default class NextNodeServer extends BaseServer< ...params.params, }).toString() - if (isDataReq) { + if (isNextDataRequest) { params.req.headers['x-nextjs-data'] = '1' } initialUrl.search = queryString diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index 3183a99260ca1..0f06bddf4fc3d 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -246,7 +246,7 @@ export type RenderOptsPartial = { ampValidator?: (html: string, pathname: string) => Promise ampSkipValidation?: boolean ampOptimizerConfig?: { [key: string]: any } - isDataReq?: boolean + isNextDataRequest?: boolean params?: ParsedUrlQuery previewProps: __ApiPreviewProps | undefined basePath: string @@ -268,7 +268,7 @@ export type RenderOptsPartial = { defaultLocale?: string domainLocales?: DomainLocale[] disableOptimizedLoading?: boolean - supportsDynamicHTML: boolean + supportsDynamicResponse: boolean isBot?: boolean runtime?: ServerRuntime serverComponents?: boolean @@ -449,7 +449,7 @@ export async function renderToHTMLImpl( getStaticProps, getStaticPaths, getServerSideProps, - isDataReq, + isNextDataRequest, params, previewProps, basePath, @@ -1170,7 +1170,7 @@ export async function renderToHTMLImpl( // Avoid rendering page un-necessarily for getServerSideProps data request // and getServerSideProps/getStaticProps redirects - if ((isDataReq && !isSSG) || metadata.isRedirect) { + if ((isNextDataRequest && !isSSG) || metadata.isRedirect) { return new RenderResult(JSON.stringify(props), { metadata, }) diff --git a/packages/next/src/server/request-meta.ts b/packages/next/src/server/request-meta.ts index 209f65c925afa..478bb24432354 100644 --- a/packages/next/src/server/request-meta.ts +++ b/packages/next/src/server/request-meta.ts @@ -189,6 +189,11 @@ type NextQueryMetadata = { __nextSsgPath?: string _nextBubbleNoFallback?: '1' + + /** + * When set to `1`, the request is for the `/_next/data` route using the pages + * router. + */ __nextDataReq?: '1' __nextCustomErrorRender?: '1' [NEXT_RSC_UNION_QUERY]?: string diff --git a/packages/next/src/server/web/adapter.ts b/packages/next/src/server/web/adapter.ts index 18030b38e6bf5..42100a08b78ff 100644 --- a/packages/next/src/server/web/adapter.ts +++ b/packages/next/src/server/web/adapter.ts @@ -125,9 +125,9 @@ export async function adapter( const buildId = requestUrl.buildId requestUrl.buildId = '' - const isDataReq = params.request.headers['x-nextjs-data'] + const isNextDataRequest = params.request.headers['x-nextjs-data'] - if (isDataReq && requestUrl.pathname === '/index') { + if (isNextDataRequest && requestUrl.pathname === '/index') { requestUrl.pathname = '/' } @@ -169,7 +169,7 @@ export async function adapter( * need to know about this property neither use it. We add it for testing * purposes. */ - if (isDataReq) { + if (isNextDataRequest) { Object.defineProperty(request, '__isData', { enumerable: false, value: true, @@ -323,7 +323,7 @@ export async function adapter( ) if ( - isDataReq && + isNextDataRequest && // if the rewrite is external and external rewrite // resolving config is enabled don't add this header // so the upstream app can set it instead @@ -367,7 +367,7 @@ export async function adapter( * it may end up with CORS error. Instead we map to an internal header so * the client knows the destination. */ - if (isDataReq) { + if (isNextDataRequest) { response.headers.delete('Location') response.headers.set( 'x-nextjs-redirect', diff --git a/packages/next/src/server/web/edge-route-module-wrapper.ts b/packages/next/src/server/web/edge-route-module-wrapper.ts index 7ba39c1931fa7..45e588f5fbd62 100644 --- a/packages/next/src/server/web/edge-route-module-wrapper.ts +++ b/packages/next/src/server/web/edge-route-module-wrapper.ts @@ -115,7 +115,7 @@ export class EdgeRouteModuleWrapper { notFoundRoutes: [], }, renderOpts: { - supportsDynamicHTML: true, + supportsDynamicResponse: true, waitUntil, onClose: closeController ? closeController.onClose.bind(closeController) diff --git a/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/about/page.jsx b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/about/page.jsx new file mode 100644 index 0000000000000..c9bf684ee8673 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/about/page.jsx @@ -0,0 +1,5 @@ +import { TestPage } from '../../../components/page' + +export default function Page({ params: { locale } }) { + return +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/layout.jsx b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/layout.jsx new file mode 100644 index 0000000000000..b55a7751211b5 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/layout.jsx @@ -0,0 +1,13 @@ +import { locales } from '../../components/page' + +export async function generateStaticParams() { + return locales.map((locale) => ({ locale })) +} + +export default function Layout({ children, params: { locale } }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/page.jsx b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/page.jsx new file mode 100644 index 0000000000000..abd02b646d87f --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/app/[locale]/page.jsx @@ -0,0 +1,5 @@ +import { TestPage } from '../../components/page' + +export default function Page({ params: { locale } }) { + return +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/app/layout.jsx b/test/e2e/app-dir/ppr-navigations/simple/app/layout.jsx new file mode 100644 index 0000000000000..caaaf5ccdf7b1 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/app/layout.jsx @@ -0,0 +1,3 @@ +export default ({ children }) => { + return children +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/app/page.jsx b/test/e2e/app-dir/ppr-navigations/simple/app/page.jsx new file mode 100644 index 0000000000000..a2fc5a8b040ea --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/app/page.jsx @@ -0,0 +1,7 @@ +import { redirect } from 'next/navigation' +import { locales } from '../components/page' + +export default () => { + // Redirect to the default locale + return redirect(`/${locales[0]}`) +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/components/page.jsx b/test/e2e/app-dir/ppr-navigations/simple/components/page.jsx new file mode 100644 index 0000000000000..92b7ea6a07d2f --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/components/page.jsx @@ -0,0 +1,40 @@ +import { unstable_noStore } from 'next/cache' +import Link from 'next/link' +import { Suspense } from 'react' + +export const locales = ['en', 'fr'] + +export const links = [ + { href: '/', text: 'Home' }, + ...locales + .map((locale) => { + return [ + { href: `/${locale}`, text: locale }, + { href: `/${locale}/about`, text: `${locale} - About` }, + ] + }) + .flat(), +] + +function Dynamic() { + unstable_noStore() + return

Dynamic
+} + +export function TestPage({ pathname }) { + return ( +
+
    + {links.map(({ href, text }) => ( +
  • + {text} +
    • + ))} +
+ {pathname} + Loading...
}> + + + + ) +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/next.config.js b/test/e2e/app-dir/ppr-navigations/simple/next.config.js new file mode 100644 index 0000000000000..6013aed786290 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/next.config.js @@ -0,0 +1,5 @@ +module.exports = { + experimental: { + ppr: true, + }, +} diff --git a/test/e2e/app-dir/ppr-navigations/simple/simple.test.ts b/test/e2e/app-dir/ppr-navigations/simple/simple.test.ts new file mode 100644 index 0000000000000..930f953bf70e1 --- /dev/null +++ b/test/e2e/app-dir/ppr-navigations/simple/simple.test.ts @@ -0,0 +1,31 @@ +import { nextTestSetup } from 'e2e-utils' +import { links, locales } from './components/page' + +describe('ppr-navigations simple', () => { + const { next } = nextTestSetup({ + files: __dirname, + }) + + it('can navigate between all the links and back', async () => { + const browser = await next.browser('/') + + try { + for (const { href } of links) { + // Find the link element for the href and click it. + await browser.elementByCss(`a[href="${href}"]`).click() + + // Wait for that page to load. + if (href === '/') { + // The root page redirects to the first locale. + await browser.waitForElementByCss(`[data-value="/${locales[0]}"]`) + } else { + await browser.waitForElementByCss(`[data-value="${href}"]`) + } + + await browser.elementByCss('#dynamic') + } + } finally { + await browser.close() + } + }) +}) From 9d1ae19af360367e53c0f5a570e261e94cc8e59b Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 28 May 2024 08:53:26 -0600 Subject: [PATCH 4/4] [lint] Disable linting using project config for tests (#66145) During integration testing, previously, calls to `next build` could rely on the project (the Next.js project) level ESLint configuration. In order to correct this, a new `lint` option was added to `nextBuild` that can be passed to enabled linting. If this is `false` or `undefined`, a `--no-lint` argument will be passed to `next build` to prevent it from running. --- .../clean-distdir/test/index.test.js | 15 ++++--- test/integration/dist-dir/test/index.test.js | 12 ++++-- .../eslint/test/lint-cache.test.js | 36 +---------------- .../eslint/test/next-build.test.js | 39 ++++++++----------- .../integration/telemetry/test/config.test.js | 3 ++ test/lib/next-modes/next-start.ts | 1 + test/lib/next-test-utils.ts | 12 ++++++ 7 files changed, 53 insertions(+), 65 deletions(-) diff --git a/test/integration/clean-distdir/test/index.test.js b/test/integration/clean-distdir/test/index.test.js index 7859587df932c..4ad38631e225a 100644 --- a/test/integration/clean-distdir/test/index.test.js +++ b/test/integration/clean-distdir/test/index.test.js @@ -5,17 +5,18 @@ import { join } from 'path' import { nextBuild } from 'next-test-utils' const appDir = join(__dirname, '../') -const customFile = join(appDir, '.next/extra-file.txt') -const cacheDir = join(appDir, '.next/cache') -// const swcCacheDir = join(appDir, '.next/cache/swc') +const nextDir = join(appDir, '.next') +const customFile = join(nextDir, '/extra-file.txt') +const cacheDir = join(nextDir, '/cache') +// const swcCacheDir = join(nextDir, '/cache/swc') const nextConfig = join(appDir, 'next.config.js') let nextConfigContent async function checkFileWrite(existsAfterBuild) { - await nextBuild(appDir) + await nextBuild(appDir, [], { lint: true }) fs.writeFileSync(customFile, 'this is a testing file') - await nextBuild(appDir) + await nextBuild(appDir, [], { lint: true }) expect(fs.existsSync(customFile)).toBe(existsAfterBuild) // `.next/cache` should be preserved in all cases expect(fs.existsSync(cacheDir)).toBe(true) @@ -32,6 +33,10 @@ describe('Cleaning distDir', () => { ;(process.env.TURBOPACK_DEV ? describe.skip : describe)( 'production mode', () => { + beforeAll(() => { + fs.removeSync(nextDir) + }) + runTests() describe('disabled write', () => { diff --git a/test/integration/dist-dir/test/index.test.js b/test/integration/dist-dir/test/index.test.js index 93e06e5f16d7a..30b6578f25c94 100644 --- a/test/integration/dist-dir/test/index.test.js +++ b/test/integration/dist-dir/test/index.test.js @@ -25,7 +25,7 @@ describe('distDir', () => { beforeAll(async () => { await fs.remove(join(appDir, '.next')) await fs.remove(join(appDir, 'dist')) - await nextBuild(appDir) + await nextBuild(appDir, [], { lint: true }) appPort = await findPort() app = await nextStart(appDir, appPort) }) @@ -79,7 +79,10 @@ describe('distDir', () => { it('should throw error with invalid distDir', async () => { const origNextConfig = await fs.readFile(nextConfig, 'utf8') await fs.writeFile(nextConfig, `module.exports = { distDir: '' }`) - const { stderr } = await nextBuild(appDir, [], { stderr: true }) + const { stderr } = await nextBuild(appDir, [], { + stderr: true, + lint: true, + }) await fs.writeFile(nextConfig, origNextConfig) expect(stderr).toContain( @@ -93,7 +96,10 @@ describe('distDir', () => { nextConfig, `module.exports = { distDir: undefined, eslint: { ignoreDuringBuilds: true } }` ) - const { stderr } = await nextBuild(appDir, [], { stderr: true }) + const { stderr } = await nextBuild(appDir, [], { + stderr: true, + lint: true, + }) await fs.writeFile(nextConfig, origNextConfig) expect(stderr.length).toBe(0) diff --git a/test/integration/eslint/test/lint-cache.test.js b/test/integration/eslint/test/lint-cache.test.js index 3ae7f7b346be8..2004b59de4991 100644 --- a/test/integration/eslint/test/lint-cache.test.js +++ b/test/integration/eslint/test/lint-cache.test.js @@ -1,43 +1,11 @@ import fs from 'fs-extra' -import os from 'os' import { join } from 'path' -import findUp from 'next/dist/compiled/find-up' -import { File, nextBuild, nextLint } from 'next-test-utils' - -const dirFirstTimeSetup = join(__dirname, '../first-time-setup') -const dirCustomConfig = join(__dirname, '../custom-config') -const dirWebVitalsConfig = join(__dirname, '../config-core-web-vitals') -const dirPluginRecommendedConfig = join( - __dirname, - '../plugin-recommended-config' -) -const dirPluginCoreWebVitalsConfig = join( - __dirname, - '../plugin-core-web-vitals-config' -) -const dirIgnoreDuringBuilds = join(__dirname, '../ignore-during-builds') -const dirBaseDirectories = join(__dirname, '../base-directories') -const dirBaseDirectoriesConfigFile = new File( - join(dirBaseDirectories, '/next.config.js') -) -const dirCustomDirectories = join(__dirname, '../custom-directories') -const dirConfigInPackageJson = join(__dirname, '../config-in-package-json') -const dirInvalidOlderEslintVersion = join( - __dirname, - '../invalid-eslint-version' -) -const dirMaxWarnings = join(__dirname, '../max-warnings') -const dirEmptyDirectory = join(__dirname, '../empty-directory') -const dirEslintIgnore = join(__dirname, '../eslint-ignore') -const dirNoEslintPlugin = join(__dirname, '../no-eslint-plugin') -const dirNoConfig = join(__dirname, '../no-config') +import { nextLint } from 'next-test-utils' + const dirEslintCache = join(__dirname, '../eslint-cache') const dirEslintCacheCustomDir = join(__dirname, '../eslint-cache-custom-dir') -const dirFileLinting = join(__dirname, '../file-linting') -const mjsCjsLinting = join(__dirname, '../mjs-cjs-linting') -const dirTypescript = join(__dirname, '../with-typescript') test('eslint caching is enabled by default', async () => { const cacheDir = join(dirEslintCache, '.next', 'cache') diff --git a/test/integration/eslint/test/next-build.test.js b/test/integration/eslint/test/next-build.test.js index 7612943cfc05e..be61a27cd16a1 100644 --- a/test/integration/eslint/test/next-build.test.js +++ b/test/integration/eslint/test/next-build.test.js @@ -1,43 +1,23 @@ import fs from 'fs-extra' -import os from 'os' import { join } from 'path' -import findUp from 'next/dist/compiled/find-up' -import { File, nextBuild, nextLint } from 'next-test-utils' +import { nextBuild } from 'next-test-utils' const dirFirstTimeSetup = join(__dirname, '../first-time-setup') const dirCustomConfig = join(__dirname, '../custom-config') -const dirWebVitalsConfig = join(__dirname, '../config-core-web-vitals') -const dirPluginRecommendedConfig = join( - __dirname, - '../plugin-recommended-config' -) -const dirPluginCoreWebVitalsConfig = join( - __dirname, - '../plugin-core-web-vitals-config' -) const dirIgnoreDuringBuilds = join(__dirname, '../ignore-during-builds') const dirBaseDirectories = join(__dirname, '../base-directories') -const dirBaseDirectoriesConfigFile = new File( - join(dirBaseDirectories, '/next.config.js') -) const dirCustomDirectories = join(__dirname, '../custom-directories') -const dirConfigInPackageJson = join(__dirname, '../config-in-package-json') const dirInvalidOlderEslintVersion = join( __dirname, '../invalid-eslint-version' ) -const dirMaxWarnings = join(__dirname, '../max-warnings') const dirEmptyDirectory = join(__dirname, '../empty-directory') const dirEslintIgnore = join(__dirname, '../eslint-ignore') const dirNoEslintPlugin = join(__dirname, '../no-eslint-plugin') -const dirNoConfig = join(__dirname, '../no-config') const dirEslintCache = join(__dirname, '../eslint-cache') const dirEslintCacheCustomDir = join(__dirname, '../eslint-cache-custom-dir') -const dirFileLinting = join(__dirname, '../file-linting') -const mjsCjsLinting = join(__dirname, '../mjs-cjs-linting') -const dirTypescript = join(__dirname, '../with-typescript') describe('Next Build', () => { ;(process.env.TURBOPACK_DEV ? describe.skip : describe)( @@ -50,6 +30,7 @@ describe('Next Build', () => { const { stdout, stderr } = await nextBuild(dirFirstTimeSetup, [], { stdout: true, stderr: true, + lint: true, }) const output = stdout + stderr @@ -62,6 +43,7 @@ describe('Next Build', () => { const { stdout, stderr } = await nextBuild(dirCustomConfig, [], { stdout: true, stderr: true, + lint: true, }) const output = stdout + stderr @@ -77,6 +59,7 @@ describe('Next Build', () => { const { stdout, stderr } = await nextBuild(dirIgnoreDuringBuilds, [], { stdout: true, stderr: true, + lint: true, }) const output = stdout + stderr @@ -90,6 +73,7 @@ describe('Next Build', () => { const { stdout, stderr } = await nextBuild(dirBaseDirectories, [], { stdout: true, stderr: true, + lint: true, }) const output = stdout + stderr @@ -121,6 +105,7 @@ describe('Next Build', () => { const { stdout, stderr } = await nextBuild(dirCustomDirectories, [], { stdout: true, stderr: true, + lint: true, }) const output = stdout + stderr @@ -140,6 +125,7 @@ describe('Next Build', () => { { stdout: true, stderr: true, + lint: true, } ) @@ -153,6 +139,7 @@ describe('Next Build', () => { const { stdout, stderr } = await nextBuild(dirEmptyDirectory, [], { stdout: true, stderr: true, + lint: true, }) const output = stdout + stderr @@ -168,6 +155,7 @@ describe('Next Build', () => { const { stdout, stderr } = await nextBuild(dirEslintIgnore, [], { stdout: true, stderr: true, + lint: true, }) const output = stdout + stderr @@ -183,6 +171,7 @@ describe('Next Build', () => { const { stdout, stderr } = await nextBuild(dirNoEslintPlugin, [], { stdout: true, stderr: true, + lint: true, }) const output = stdout + stderr @@ -195,7 +184,9 @@ describe('Next Build', () => { const cacheDir = join(dirEslintCache, '.next', 'cache') await fs.remove(cacheDir) - await nextBuild(dirEslintCache, []) + await nextBuild(dirEslintCache, [], { + lint: true, + }) const files = await fs.readdir(join(cacheDir, 'eslint/')) const cacheExists = files.some((f) => /\.cache/.test(f)) @@ -210,7 +201,9 @@ describe('Next Build', () => { await fs.remove(oldCacheDir) await fs.remove(newCacheDir) - await nextBuild(dirEslintCacheCustomDir, []) + await nextBuild(dirEslintCacheCustomDir, [], { + lint: true, + }) expect(fs.existsSync(oldCacheDir)).toBe(false) diff --git a/test/integration/telemetry/test/config.test.js b/test/integration/telemetry/test/config.test.js index 94a926e758a13..80aec6f56bf08 100644 --- a/test/integration/telemetry/test/config.test.js +++ b/test/integration/telemetry/test/config.test.js @@ -165,6 +165,7 @@ describe('config telemetry', () => { const { stderr } = await nextBuild(appDir, [], { stderr: true, env: { NEXT_TELEMETRY_DEBUG: 1 }, + lint: true, }) await fs.remove(path.join(appDir, '.eslintrc')) @@ -222,6 +223,7 @@ describe('config telemetry', () => { const { stderr } = await nextBuild(appDir, [], { stderr: true, env: { NEXT_TELEMETRY_DEBUG: 1 }, + lint: true, }) await fs.remove(nextConfig) @@ -266,6 +268,7 @@ describe('config telemetry', () => { const { stderr } = await nextBuild(appDir, [], { stderr: true, env: { NEXT_TELEMETRY_DEBUG: 1 }, + lint: true, }) const featureUsageEvents = findAllTelemetryEvents( stderr, diff --git a/test/lib/next-modes/next-start.ts b/test/lib/next-modes/next-start.ts index 6366c7a114438..571879924d424 100644 --- a/test/lib/next-modes/next-start.ts +++ b/test/lib/next-modes/next-start.ts @@ -68,6 +68,7 @@ export class NextStartInstance extends NextInstance { if (this.buildCommand) { buildArgs = this.buildCommand.split(' ') } + if (this.startCommand) { startArgs = this.startCommand.split(' ') } diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 0179ecd530ecf..02730fc7b5a4e 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -192,6 +192,12 @@ export interface NextOptions { stdout?: true | 'log' ignoreFail?: boolean + /** + * If true, this enables the linting step in the build process. If false or + * undefined, it adds a `--no-lint` flag to the build command. + */ + lint?: boolean + onStdout?: (data: any) => void onStderr?: (data: any) => void } @@ -442,6 +448,12 @@ export function nextBuild( args: string[] = [], opts: NextOptions = {} ) { + // If the build hasn't requested it to be linted explicitly, disable linting + // if it's not already disabled. + if (!opts.lint && !args.includes('--no-lint')) { + args.push('--no-lint') + } + return runNextCommand(['build', dir, ...args], opts) }