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

Do not create client reference manifest for metadata route handlers #71225

Merged
merged 1 commit into from
Oct 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion packages/next/src/build/flying-shuttle/stitch-builds.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
} from '../../shared/lib/constants'
import { normalizeAppPath } from '../../shared/lib/router/utils/app-paths'
import type { NextConfigComplete } from '../../server/config-shared'
import { isMetadataRoute } from '../../lib/metadata/is-metadata-route'

export async function stitchBuilds(
{
Expand Down Expand Up @@ -106,7 +107,7 @@ export async function stitchBuilds(
path.join(distDir, entryFile + '.nft.json')
)

if (type === 'app') {
if (type === 'app' && !isMetadataRoute(entry)) {
const clientRefManifestFile = path.join(
'server',
type,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
} from '../utils'
import type { ChunkGroup } from 'webpack'
import { encodeURIPath } from '../../../shared/lib/encode-uri-path'
import { isMetadataRoute } from '../../../lib/metadata/is-metadata-route'

interface Options {
dev: boolean
Expand Down Expand Up @@ -519,9 +520,9 @@ export class ClientReferenceManifestPlugin {
manifestEntryFiles.push(entryName.replace(/\/page(\.[^/]+)?$/, '/page'))
}

// We also need to create manifests for route handler entrypoints to
// enable `'use cache'`.
if (/\/route$/.test(entryName)) {
// We also need to create manifests for route handler entrypoints
// (excluding metadata route handlers) to enable `'use cache'`.
if (/\/route$/.test(entryName) && !isMetadataRoute(entryName)) {
manifestEntryFiles.push(entryName)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { getModuleBuildInfo } from '../loaders/get-module-build-info'
import { getPageFilePath } from '../../entries'
import { resolveExternal } from '../../handle-externals'
import swcLoader from '../loaders/next-swc-loader'
import { isMetadataRoute } from '../../../lib/metadata/is-metadata-route'

const PLUGIN_NAME = 'TraceEntryPointsPlugin'
export const TRACE_IGNORES = [
Expand Down Expand Up @@ -277,15 +278,18 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {
)

if (entrypoint.name.startsWith('app/')) {
// include the client reference manifest
const clientManifestsForEntrypoint = nodePath.join(
outputPath,
outputPrefix,
entrypoint.name.replace(/%5F/g, '_') +
'_' +
CLIENT_REFERENCE_MANIFEST +
'.js'
)
// Include the client reference manifest for pages and route handlers,
// excluding metadata route handlers.
const clientManifestsForEntrypoint = isMetadataRoute(entrypoint.name)
? null
: nodePath.join(
outputPath,
outputPrefix,
entrypoint.name.replace(/%5F/g, '_') +
'_' +
CLIENT_REFERENCE_MANIFEST +
'.js'
)

if (clientManifestsForEntrypoint !== null) {
entryFiles.add(clientManifestsForEntrypoint)
Expand Down
7 changes: 6 additions & 1 deletion packages/next/src/build/webpack/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type {
ModuleGraph,
} from 'webpack'
import type { ModuleGraphConnection } from 'webpack'
import { isMetadataRoute } from '../../lib/metadata/is-metadata-route'

export function traverseModules(
compilation: Compilation,
Expand Down Expand Up @@ -47,7 +48,11 @@ export function forEachEntryModule(
) {
for (const [name, entry] of compilation.entries.entries()) {
// Skip for entries under pages/
if (name.startsWith('pages/')) {
if (
name.startsWith('pages/') ||
// Skip for metadata route handlers
(name.startsWith('app/') && isMetadataRoute(name))
) {
continue
}

Expand Down
6 changes: 5 additions & 1 deletion packages/next/src/server/load-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { wait } from '../lib/wait'
import { setReferenceManifestsSingleton } from './app-render/encryption-utils'
import { createServerModuleMap } from './app-render/action-utils'
import type { DeepReadonly } from '../shared/lib/deep-readonly'
import { isMetadataRoute } from '../lib/metadata/is-metadata-route'

export type ManifestItem = {
id: number | string
Expand Down Expand Up @@ -139,6 +140,9 @@ async function loadComponentsImpl<N = any>({
])
}

// Make sure to avoid loading the manifest for metadata route handlers.
const hasClientManifest = isAppPath && !isMetadataRoute(page)

// Load the manifest files first
const [
buildManifest,
Expand All @@ -150,7 +154,7 @@ async function loadComponentsImpl<N = any>({
loadManifestWithRetries<ReactLoadableManifest>(
join(distDir, REACT_LOADABLE_MANIFEST)
),
isAppPath
hasClientManifest
? loadClientReferenceManifest(
join(
distDir,
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/app-dir/hello-world/hello-world.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('hello-world', () => {
expect($('p').text()).toBe('hello world')
})

// Recommended for tests that need a full browser
// Recommended for tests that need a full browser.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was done to trigger a deploy test. The test/e2e/app-dir/hello-world test app does have a favicon.ico metadata route handler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it('should work using browser', async () => {
const browser = await next.browser('/')
expect(await browser.elementByCss('p').text()).toBe('hello world')
Expand All @@ -23,7 +23,7 @@ describe('hello-world', () => {
expect(html).toContain('hello world')
})

// In case you need to test the response object
// In case you need to test the response object.
it('should work with fetch', async () => {
const res = await next.fetch('/')
const html = await res.text()
Expand Down
Loading