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

Optionally bundle legacy react-dom/server APIs based on usage #59737

Merged
merged 11 commits into from
Dec 20, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 4 additions & 5 deletions packages/next/src/build/create-compiler-aliases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,15 +255,14 @@ export function createRSCAliases(
'react-dom/static$': `next/dist/compiled/react-dom-experimental/static`,
'react-dom/static.edge$': `next/dist/compiled/react-dom-experimental/static.edge`,
'react-dom/static.browser$': `next/dist/compiled/react-dom-experimental/static.browser`,
'react-dom/server.edge$': `next/dist/compiled/react-dom${bundledReactChannel}/server.edge`,
'react-dom/server.browser$': `next/dist/compiled/react-dom${bundledReactChannel}/server.browser`,
// optimizations to ignore the legacy build of react-dom/server in `server.browser` build
'react-dom/server.edge$': `next/dist/build/webpack/alias/react-dom-server-edge${bundledReactChannel}.js`,
'react-dom/server.browser$': `next/dist/build/webpack/alias/react-dom-server-browser${bundledReactChannel}.js`,
// react-server-dom-webpack alias
'react-server-dom-webpack/client$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/client`,
'react-server-dom-webpack/client.edge$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/client.edge`,
'react-server-dom-webpack/server.edge$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/server.edge`,
'react-server-dom-webpack/server.node$': `next/dist/compiled/react-server-dom-webpack${bundledReactChannel}/server.node`,
// optimisations to ignore the legacy build of react-dom/server
'./cjs/react-dom-server-legacy.browser.production.min.js': `next/dist/build/noop-react-dom-server-legacy`,
'./cjs/react-dom-server-legacy.browser.development.js': `next/dist/build/noop-react-dom-server-legacy`,
}

if (!isEdgeServer) {
Expand Down
10 changes: 0 additions & 10 deletions packages/next/src/build/noop-react-dom-server-legacy.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
const ERROR_MESSAGE =
'Internal Error: do not use legacy react-dom/server APIs. If you encountered this error, please open an issue on the Next.js repo.'

function error() {
throw new Error(ERROR_MESSAGE)
}

var s
if (process.env.NODE_ENV === 'production') {
Copy link
Contributor

Choose a reason for hiding this comment

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

did you copy this by hand? can't we automate something with taskr?

Copy link
Member

Choose a reason for hiding this comment

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

I think this file can be used in both dev and prod so this is expected?

s = require('next/dist/compiled/react-dom-experimental/cjs/react-dom-server.browser.production.min.js')
} else {
s = require('next/dist/compiled/react-dom-experimental/cjs/react-dom-server.browser.development.js')
}

exports.renderToString = error
exports.renderToStaticMarkup = error
exports.renderToNodeStream = error
exports.renderToStaticNodeStream = error

exports.version = s.version
exports.renderToReadableStream = s.renderToReadableStream
if (s.resume) {
exports.resume = s.resume
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
const ERROR_MESSAGE =
'Internal Error: do not use legacy react-dom/server APIs. If you encountered this error, please open an issue on the Next.js repo.'

function error() {
throw new Error(ERROR_MESSAGE)
}

var s
if (process.env.NODE_ENV === 'production') {
s = require('next/dist/compiled/react-dom/cjs/react-dom-server.browser.production.min.js')
} else {
s = require('next/dist/compiled/react-dom/cjs/react-dom-server.browser.development.js')
}

exports.renderToString = error
exports.renderToStaticMarkup = error
exports.renderToNodeStream = error
exports.renderToStaticNodeStream = error

exports.version = s.version
exports.renderToReadableStream = s.renderToReadableStream
if (s.resume) {
exports.resume = s.resume
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const ERROR_MESSAGE =
'Internal Error: do not use legacy react-dom/server APIs. If you encountered this error, please open an issue on the Next.js repo.'

function error() {
throw new Error(ERROR_MESSAGE)
}

var b
if (process.env.NODE_ENV === 'production') {
b = require('next/dist/compiled/react-dom-experimental/cjs/react-dom-server.edge.production.min.js')
} else {
b = require('next/dist/compiled/react-dom-experimental/cjs/react-dom-server.edge.development.js')
}

exports.version = b.version
exports.renderToReadableStream = b.renderToReadableStream
exports.renderToNodeStream = b.renderToNodeStream
exports.renderToStaticNodeStream = b.renderToStaticNodeStream
exports.renderToString = error
exports.renderToStaticMarkup = error
if (b.resume) {
exports.resume = b.resume
}
23 changes: 23 additions & 0 deletions packages/next/src/build/webpack/alias/react-dom-server-edge.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const ERROR_MESSAGE =
'Internal Error: do not use legacy react-dom/server APIs. If you encountered this error, please open an issue on the Next.js repo.'

function error() {
throw new Error(ERROR_MESSAGE)
}

var b
if (process.env.NODE_ENV === 'production') {
b = require('next/dist/compiled/react-dom/cjs/react-dom-server.edge.production.min.js')
} else {
b = require('next/dist/compiled/react-dom/cjs/react-dom-server.edge.development.js')
}

exports.version = b.version
exports.renderToReadableStream = b.renderToReadableStream
exports.renderToNodeStream = b.renderToNodeStream
exports.renderToStaticNodeStream = b.renderToStaticNodeStream
exports.renderToString = error
exports.renderToStaticMarkup = error
if (b.resume) {
exports.resume = b.resume
}
20 changes: 14 additions & 6 deletions packages/next/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@ const pagesExternals = [
'react-server-dom-webpack/server.node',
]

const appExternals = [
// Externalize the react-dom/server legacy implementation outside of the runtime.
// If users are using them and imported from 'react-dom/server' they will get the external asset bundled.
'next/dist/compiled/react-dom/cjs/react-dom-server-legacy.browser.development.js',
'next/dist/compiled/react-dom/cjs/react-dom-server-legacy.browser.production.min.js',
'next/dist/compiled/react-dom-experimental/cjs/react-dom-server-legacy.browser.development.js',
'next/dist/compiled/react-dom-experimental/cjs/react-dom-server-legacy.browser.production.min.js',
]

function makeAppAliases(reactChannel = '') {
return {
react$: `next/dist/compiled/react${reactChannel}`,
Expand All @@ -30,11 +39,13 @@ function makeAppAliases(reactChannel = '') {
'react/jsx-dev-runtime$': `next/dist/compiled/react${reactChannel}/jsx-dev-runtime`,
'react-dom/client$': `next/dist/compiled/react-dom${reactChannel}/client`,
'react-dom/server$': `next/dist/compiled/react-dom${reactChannel}/server`,
'react-dom/server.edge$': `next/dist/compiled/react-dom${reactChannel}/server.edge`,
'react-dom/server.browser$': `next/dist/compiled/react-dom${reactChannel}/server.browser`,
'react-dom/static$': `next/dist/compiled/react-dom-experimental/static`,
'react-dom/static.edge$': `next/dist/compiled/react-dom-experimental/static.edge`,
'react-dom/static.browser$': `next/dist/compiled/react-dom-experimental/static.browser`,
// optimizations to ignore the legacy build of react-dom/server in `server.browser` build
'react-dom/server.edge$': `next/dist/build/webpack/alias/react-dom-server-edge${reactChannel}.js`,
'react-dom/server.browser$': `next/dist/build/webpack/alias/react-dom-server-browser${reactChannel}.js`,
// react-server-dom-webpack alias
'react-server-dom-turbopack/client$': `next/dist/compiled/react-server-dom-turbopack${reactChannel}/client`,
'react-server-dom-turbopack/client.edge$': `next/dist/compiled/react-server-dom-turbopack${reactChannel}/client.edge`,
'react-server-dom-turbopack/server.edge$': `next/dist/compiled/react-server-dom-turbopack${reactChannel}/server.edge`,
Expand All @@ -43,9 +54,6 @@ function makeAppAliases(reactChannel = '') {
'react-server-dom-webpack/client.edge$': `next/dist/compiled/react-server-dom-webpack${reactChannel}/client.edge`,
'react-server-dom-webpack/server.edge$': `next/dist/compiled/react-server-dom-webpack${reactChannel}/server.edge`,
'react-server-dom-webpack/server.node$': `next/dist/compiled/react-server-dom-webpack${reactChannel}/server.node`,
// optimisations to ignore the legacy build of react-dom/server
'./cjs/react-dom-server-legacy.browser.production.min.js': `next/dist/build/noop-react-dom-server-legacy`,
'./cjs/react-dom-server-legacy.browser.development.js': `next/dist/build/noop-react-dom-server-legacy`,
}
}

Expand Down Expand Up @@ -252,7 +260,7 @@ module.exports = ({ dev, turbo, bundleType, experimental }) => {
},
externals: [
...sharedExternals,
...(bundleType === 'pages' ? pagesExternals : []),
...(bundleType === 'pages' ? pagesExternals : appExternals),
externalsMap,
externalHandler,
],
Expand Down
6 changes: 6 additions & 0 deletions test/e2e/app-dir/rsc-basic/app/app-react/client-react.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,21 @@
import React from 'react'
import ReactDOM from 'react-dom'
import ReactDOMServer from 'react-dom/server.edge'
import { renderToStaticMarkup } from 'react-dom/server'

export default function ClientReact() {
const markup = renderToStaticMarkup(
<div className="react-static-markup">{'React Static Markup'}</div>
)

return (
<div>
<p id="client-react">{'React.version=' + React.version}</p>
<p id="client-react-dom">{'ReactDOM.version=' + ReactDOM.version}</p>
<p id="client-react-dom-server">
{'ReactDOMServer.version=' + ReactDOMServer.version}
</p>
<p id="markup">{markup}</p>
</div>
)
}
32 changes: 31 additions & 1 deletion test/e2e/app-dir/rsc-basic/rsc-basic.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import path from 'path'
import { check } from 'next-test-utils'
import { check, hasRedbox } from 'next-test-utils'
import { createNextDescribe } from 'e2e-utils'
import cheerio from 'cheerio'

Expand Down Expand Up @@ -544,6 +544,36 @@ createNextDescribe(
)
})

it('should be able to call legacy react-dom/server APIs in client components', async () => {
const $ = await next.render$('/app-react')
const content = $('#markup').text()
expect(content).toBe(
'<div class="react-static-markup">React Static Markup</div>'
)

if (isNextDev) {
const filePath = 'app/app-react/client-react.js'
const fileContent = await next.readFile(filePath)
await next.patchFile(
filePath,
fileContent.replace(
`import { renderToStaticMarkup } from 'react-dom/server'`,
`import { renderToStaticMarkup } from 'react-dom/server.browser'`
)
)

const browser = await next.browser('/app-react')
expect(await hasRedbox(browser)).toBe(true)

await next.patchFile(filePath, fileContent)

await check(async () => {
expect(await hasRedbox(browser)).toBe(false)
return 'success'
}, 'success')
}
})

// disable this flaky test
it.skip('should support partial hydration with inlined server data in browser', async () => {
// Should end up with "next_streaming_data".
Expand Down
4 changes: 3 additions & 1 deletion test/turbopack-tests-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -4044,7 +4044,9 @@
"app dir - rsc basics should track client components in dynamic imports",
"app dir - rsc basics should use canary react for app"
],
"failed": [],
"failed": [
"app dir - rsc basics should be able to call legacy react-dom/server APIs in client components"
huozhi marked this conversation as resolved.
Show resolved Hide resolved
],
"pending": [
"app dir - rsc basics should support partial hydration with inlined server data in browser",
"app dir - rsc basics should support webpack loader rules"
Expand Down