Skip to content

Commit

Permalink
Avoid bundling react-dom/server.browser in Pages router
Browse files Browse the repository at this point in the history
This was a regression introduced when extending support for Pages router
to React 18.
We have to keep the `try-catch` when the pages runtime is used as an external
(e.g. when the Node.js runtime is used).
However, when bundling, we can alias the module relevant to choosing the
correct `react-dom/server` entrypoint to the one we actually want.
  • Loading branch information
eps1lon committed Sep 25, 2024
1 parent 565abf2 commit e490dbe
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 35 deletions.
9 changes: 9 additions & 0 deletions crates/next-core/src/next_import_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,15 @@ async fn insert_next_server_special_aliases(
external_esm_if_node(project_path, "next/dist/compiled/@vercel/og/index.node.js"),
);

import_map.insert_exact_alias(
"next/dist/server/ReactDOMServerPages",
ImportMapping::Alternatives(vec![
request_to_import_mapping(project_path, "react-dom/server.edge"),
request_to_import_mapping(project_path, "react-dom/server.browser"),
])
.cell(),
);

import_map.insert_exact_alias(
"@opentelemetry/api",
// It needs to prefer the local version of @opentelemetry/api
Expand Down
9 changes: 9 additions & 0 deletions packages/next/src/build/create-compiler-aliases.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import path from 'path'
import * as React from 'react'
import {
DOT_NEXT_ALIAS,
PAGES_DIR_ALIAS,
Expand All @@ -21,6 +22,8 @@ interface CompilerAliases {
[alias: string]: string | string[]
}

const isReact19 = typeof React.use === 'function'

export function createWebpackAliases({
distDir,
isClient,
Expand Down Expand Up @@ -90,6 +93,12 @@ export function createWebpackAliases({
return {
'@vercel/og$': 'next/dist/server/og/image-response',

// Avoid bundling both entrypoints in React 19 when we just need one.
// Also avoids bundler warnings in React 18 where react-dom/server.edge doesn't exist.
'next/dist/server/ReactDOMServerPages': isReact19
? 'react-dom/server.edge'
: 'react-dom/server.browser',

// Alias next/dist imports to next/dist/esm assets,
// let this alias hit before `next` alias.
...(isEdgeServer
Expand Down
8 changes: 0 additions & 8 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1974,14 +1974,6 @@ export default async function getBaseWebpackConfig(
)
),
].filter(Boolean as any as ExcludesFalse),
ignoreWarnings: [
(warning) => {
// require('react-dom/server.edge') is wrapped in try-catch so save to ignore.
return warning.message.startsWith(
'Module not found: Error: Package path ./server.edge is not exported from package'
)
},
],
}

// Support tsconfig and jsconfig baseUrl
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import type { Revalidate, SwrDelta } from './lib/revalidate'
import type { COMPILER_NAMES } from '../shared/lib/constants'

import React, { type JSX } from 'react'
import ReactDOMServerPages from './ReactDOMServerPages'
import ReactDOMServerPages from 'next/dist/server/ReactDOMServerPages'
import { StyleRegistry, createStyleRegistry } from 'styled-jsx'
import {
GSP_NO_RETURNED_VALUE,
Expand Down
4 changes: 4 additions & 0 deletions packages/next/types/$$compiled.internal.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ declare module 'VAR_USERLAND'
declare module 'VAR_MODULE_DOCUMENT'
declare module 'VAR_MODULE_APP'

declare module 'next/dist/server/ReactDOMServerPages' {
export * from 'react-dom/server.edge'
}

declare module 'next/dist/compiled/@napi-rs/triples' {
export * from '@napi-rs/triples'
}
Expand Down
4 changes: 0 additions & 4 deletions packages/next/types/react-dom.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,10 +70,6 @@ declare module 'react-dom/server.edge' {
>
}

declare module 'react-dom/server.browser' {
export * from 'react-dom/server.edge'
}

declare module 'react-dom/static.edge' {
import type { JSX } from 'react'
/**
Expand Down
22 changes: 0 additions & 22 deletions test/development/basic/next-rs-api.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import type {
import loadConfig from 'next/dist/server/config'
import path from 'path'

const isReact18 = parseInt(process.env.NEXT_TEST_REACT_VERSION) === 18

function normalizePath(path: string) {
return path
.replace(/\[project\].+\/node_modules\//g, '[project]/.../node_modules/')
Expand All @@ -42,21 +40,6 @@ function styledStringToMarkdown(styled: StyledString): string {
}
}

function isReactDOMServerEdgeConditionalBundlingIssue(issue: {
description?: Issue['description']
filePath: string
}) {
return (
isReact18 &&
issue.filePath ===
'[project]/.../node_modules/next/dist/esm/server/ReactDOMServerPages.js' &&
issue.description?.type === 'text' &&
issue.description?.value.includes(
'Import map: aliased to module "react-dom" with subpath "/server.edge" inside of [project]/'
)
)
}

function normalizeIssues(issues: Issue[]) {
return issues
.map((issue) => ({
Expand All @@ -69,11 +52,6 @@ function normalizeIssues(issues: Issue[]) {
source: normalizePath(issue.source.source.ident),
},
}))
.filter((issue) => {
// The conditional bundling is wrapped in a try-catch.
// It doesn't surface to the user, so it's safe to ignore here.
return !isReactDOMServerEdgeConditionalBundlingIssue(issue)
})
.sort((a, b) => {
const a_ = JSON.stringify(a)
const b_ = JSON.stringify(b)
Expand Down

0 comments on commit e490dbe

Please sign in to comment.