From 063154918c28b4f2a45ffd8d506fc44924483d6e Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 19 Sep 2023 15:45:59 +0200 Subject: [PATCH] Fix react packages are not bundled for metadata routes (#55579) `isAppLayer` condition was missing `app-metadata-route` layer, made it as a util now like other webpack layer utils, add metadata route layer to the group. Then `React.cache` can be available there. Also update regex to be compatible across platform Fixes #55561 Closes NEXT-1635 --- packages/next/src/build/utils.ts | 6 +++ packages/next/src/build/webpack-config.ts | 39 ++++++++++--------- packages/next/src/lib/constants.ts | 10 ++++- .../client-component.tsx | 10 +++++ .../app/client-ref-dependency/sitemap.tsx | 16 ++++++++ .../metadata-dynamic-routes/index.test.ts | 7 ++++ 6 files changed, 68 insertions(+), 20 deletions(-) create mode 100644 test/e2e/app-dir/metadata-dynamic-routes/app/client-ref-dependency/client-component.tsx create mode 100644 test/e2e/app-dir/metadata-dynamic-routes/app/client-ref-dependency/sitemap.tsx diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index b3e248c745422..c012b196bb4e4 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -2131,3 +2131,9 @@ export function isWebpackDefaultLayer( ): boolean { return layer === null || layer === undefined } + +export function isWebpackAppLayer( + layer: WebpackLayerName | null | undefined +): boolean { + return Boolean(layer && WEBPACK_LAYERS.GROUP.app.includes(layer as any)) +} diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 750baea6b508a..4bce85fc2ef64 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -19,7 +19,11 @@ import { WEBPACK_RESOURCE_QUERIES, WebpackLayerName, } from '../lib/constants' -import { isWebpackDefaultLayer, isWebpackServerLayer } from './utils' +import { + isWebpackAppLayer, + isWebpackDefaultLayer, + isWebpackServerLayer, +} from './utils' import { CustomRoutes } from '../lib/load-custom-routes.js' import { isEdgeRuntime } from '../lib/is-edge-runtime' import { @@ -412,7 +416,7 @@ function createRSCAliases( } if (!opts.isEdgeServer) { - if (opts.layer === 'ssr') { + if (opts.layer === WEBPACK_LAYERS.serverSideRendering) { alias = Object.assign(alias, { 'react/jsx-runtime$': `next/dist/server/future/route-modules/app-page/vendored/shared/react-jsx-runtime`, 'react/jsx-dev-runtime$': `next/dist/server/future/route-modules/app-page/vendored/shared/react-jsx-dev-runtime`, @@ -421,7 +425,7 @@ function createRSCAliases( 'react-dom/server.edge$': `next/dist/server/future/route-modules/app-page/vendored/${opts.layer}/react-dom-server-edge`, 'react-server-dom-webpack/client.edge$': `next/dist/server/future/route-modules/app-page/vendored/${opts.layer}/react-server-dom-webpack-client-edge`, }) - } else if (opts.layer === 'rsc') { + } else if (opts.layer === WEBPACK_LAYERS.reactServerComponents) { alias = Object.assign(alias, { 'react/jsx-runtime$': `next/dist/server/future/route-modules/app-page/vendored/shared/react-jsx-runtime`, 'react/jsx-dev-runtime$': `next/dist/server/future/route-modules/app-page/vendored/shared/react-jsx-dev-runtime`, @@ -434,7 +438,7 @@ function createRSCAliases( } if (opts.isEdgeServer) { - if (opts.layer === 'rsc') { + if (opts.layer === WEBPACK_LAYERS.reactServerComponents) { alias[ 'react$' ] = `next/dist/compiled/react${bundledReactChannel}/react.shared-subset` @@ -1383,15 +1387,7 @@ export default async function getBaseWebpackConfig( return `commonjs next/dist/lib/import-next-warning` } - const isAppLayer = ( - [ - WEBPACK_LAYERS.reactServerComponents, - WEBPACK_LAYERS.serverSideRendering, - WEBPACK_LAYERS.appPagesBrowser, - WEBPACK_LAYERS.actionBrowser, - WEBPACK_LAYERS.appRouteHandler, - ] as WebpackLayerName[] - ).includes(layer!) + const isAppLayer = isWebpackAppLayer(layer) // Relative requires don't need custom resolution, because they // are relative to requests we've already resolved here. @@ -1461,25 +1457,30 @@ export default async function getBaseWebpackConfig( // Specific Next.js imports that should remain external // TODO-APP: Investigate if we can remove this. if (request.startsWith('next/dist/')) { + // Non external that needs to be transpiled // Image loader needs to be transpiled - if (/^next\/dist\/shared\/lib\/image-loader/.test(request)) { + if (/^next[\\/]dist[\\/]shared[\\/]lib[\\/]image-loader/.test(request)) { return } - if (/^next\/dist\/compiled\/next-server/.test(request)) { + if (/^next[\\/]dist[\\/]compiled[\\/]next-server/.test(request)) { return `commonjs ${request}` } if ( - /^next\/dist\/shared\/(?!lib\/router\/router)/.test(request) || - /^next\/dist\/compiled\/.*\.c?js$/.test(request) + /^next[\\/]dist[\\/]shared[\\/](?!lib[\\/]router[\\/]router)/.test( + request + ) || + /^next[\\/]dist[\\/]compiled[\\/].*\.c?js$/.test(request) ) { return `commonjs ${request}` } if ( - /^next\/dist\/esm\/shared\/(?!lib\/router\/router)/.test(request) || - /^next\/dist\/compiled\/.*\.mjs$/.test(request) + /^next[\\/]dist[\\/]esm[\\/]shared[\\/](?!lib[\\/]router[\\/]router)/.test( + request + ) || + /^next[\\/]dist[\\/]compiled[\\/].*\.mjs$/.test(request) ) { return `module ${request}` } diff --git a/packages/next/src/lib/constants.ts b/packages/next/src/lib/constants.ts index 0ec3d8c07f894..2a605586b817a 100644 --- a/packages/next/src/lib/constants.ts +++ b/packages/next/src/lib/constants.ts @@ -107,7 +107,7 @@ const WEBPACK_LAYERS_NAMES = { */ reactServerComponents: 'rsc', /** - * Server Side Rendering layer (ssr). + * Server Side Rendering layer for app (ssr). */ serverSideRendering: 'ssr', /** @@ -157,6 +157,14 @@ const WEBPACK_LAYERS = { WEBPACK_LAYERS_NAMES.middleware, WEBPACK_LAYERS_NAMES.api, ], + app: [ + WEBPACK_LAYERS_NAMES.reactServerComponents, + WEBPACK_LAYERS_NAMES.actionBrowser, + WEBPACK_LAYERS_NAMES.appMetadataRoute, + WEBPACK_LAYERS_NAMES.appRouteHandler, + WEBPACK_LAYERS_NAMES.serverSideRendering, + WEBPACK_LAYERS_NAMES.appPagesBrowser, + ], }, } diff --git a/test/e2e/app-dir/metadata-dynamic-routes/app/client-ref-dependency/client-component.tsx b/test/e2e/app-dir/metadata-dynamic-routes/app/client-ref-dependency/client-component.tsx new file mode 100644 index 0000000000000..29a8386108f33 --- /dev/null +++ b/test/e2e/app-dir/metadata-dynamic-routes/app/client-ref-dependency/client-component.tsx @@ -0,0 +1,10 @@ +'use client' + +import React from 'react' + +function ClientComponent() { + const [state] = React.useState('component') + return
{`client-` + state}
+} + +export const clientRef = diff --git a/test/e2e/app-dir/metadata-dynamic-routes/app/client-ref-dependency/sitemap.tsx b/test/e2e/app-dir/metadata-dynamic-routes/app/client-ref-dependency/sitemap.tsx new file mode 100644 index 0000000000000..76d65e1bde6ca --- /dev/null +++ b/test/e2e/app-dir/metadata-dynamic-routes/app/client-ref-dependency/sitemap.tsx @@ -0,0 +1,16 @@ +import React from 'react' +import { clientRef } from './client-component' + +export const contentType = 'image/png' +const cachedNoop = React.cache(() => null) + +function noopCall(value) { + return value +} + +export default function sitemap() { + // keep the variable from being tree-shaken + noopCall(clientRef) + cachedNoop() + return [] +} diff --git a/test/e2e/app-dir/metadata-dynamic-routes/index.test.ts b/test/e2e/app-dir/metadata-dynamic-routes/index.test.ts index c4e6112fff9f1..1affe0adff37d 100644 --- a/test/e2e/app-dir/metadata-dynamic-routes/index.test.ts +++ b/test/e2e/app-dir/metadata-dynamic-routes/index.test.ts @@ -66,6 +66,13 @@ createNextDescribe( " `) }) + + it('should not throw if client components are imported but not used', async () => { + const { status } = await next.fetch( + '/client-ref-dependency/sitemap.xml' + ) + expect(status).toBe(200) + }) }) describe('social image routes', () => {