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 all 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,18 @@
var l, 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?

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

exports.version = l.version
exports.renderToString = l.renderToString
exports.renderToStaticMarkup = l.renderToStaticMarkup
exports.renderToNodeStream = l.renderToNodeStream
exports.renderToStaticNodeStream = l.renderToStaticNodeStream
exports.renderToReadableStream = s.renderToReadableStream
if (s.resume) {
exports.resume = s.resume
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
var l, s
if (process.env.NODE_ENV === 'production') {
l = require('next/dist/compiled/react-dom/cjs/react-dom-server-legacy.browser.production.min.js')
s = require('next/dist/compiled/react-dom/cjs/react-dom-server.browser.production.min.js')
} else {
l = require('next/dist/compiled/react-dom/cjs/react-dom-server-legacy.browser.development.js')
s = require('next/dist/compiled/react-dom/cjs/react-dom-server.browser.development.js')
}

exports.version = l.version
exports.renderToString = l.renderToString
exports.renderToStaticMarkup = l.renderToStaticMarkup
exports.renderToNodeStream = l.renderToNodeStream
exports.renderToStaticNodeStream = l.renderToStaticNodeStream
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
}
21 changes: 15 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,14 @@ 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`,
// In Next.js runtime only use react-dom/server.edge
'react-dom/server.browser$': 'react-dom/server.edge',
// 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 +55,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 +261,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>
)
}
30 changes: 30 additions & 0 deletions test/e2e/app-dir/rsc-basic/rsc-basic.test.ts
Original file line number Diff line number Diff line change
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')
const markupContentInBrowser = await browser
.elementByCss('#markup')
.text()
expect(markupContentInBrowser).toBe(
'<div class="react-static-markup">React Static Markup</div>'
)

await next.patchFile(filePath, fileContent)
}
})

// 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