Skip to content

Commit

Permalink
Fix react packages are not bundled for metadata routes (#55579)
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
huozhi authored Sep 19, 2023
1 parent bad5365 commit 0631549
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 20 deletions.
6 changes: 6 additions & 0 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
39 changes: 20 additions & 19 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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`,
Expand All @@ -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`,
Expand All @@ -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`
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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}`
}
Expand Down
10 changes: 9 additions & 1 deletion packages/next/src/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ const WEBPACK_LAYERS_NAMES = {
*/
reactServerComponents: 'rsc',
/**
* Server Side Rendering layer (ssr).
* Server Side Rendering layer for app (ssr).
*/
serverSideRendering: 'ssr',
/**
Expand Down Expand Up @@ -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,
],
},
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
'use client'

import React from 'react'

function ClientComponent() {
const [state] = React.useState('component')
return <div>{`client-` + state}</div>
}

export const clientRef = <ClientComponent />
Original file line number Diff line number Diff line change
@@ -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 []
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/metadata-dynamic-routes/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down

0 comments on commit 0631549

Please sign in to comment.