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

Adopt react 18 rc1 #34972

Merged
merged 7 commits into from Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from 6 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
4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -144,9 +144,9 @@
"pretty-ms": "7.0.0",
"random-seed": "0.3.0",
"react": "17.0.2",
"react-18": "npm:react@18.0.0-rc.0",
"react-18": "npm:react@18.0.0-rc.1",
"react-dom": "17.0.2",
"react-dom-18": "npm:react-dom@18.0.0-rc.0",
"react-dom-18": "npm:react-dom@18.0.0-rc.1",
"react-ssr-prepass": "1.0.8",
"relay-compiler": "13.0.2",
"relay-runtime": "13.0.2",
Expand Down
Expand Up @@ -69,11 +69,16 @@ async function parseImportsInfo({
transformedSource += JSON.stringify(`${importSource}?__sc_client__`)
imports += `require(${JSON.stringify(importSource)})\n`
} else {
// FIXME
// case: 'react'
// Avoid module resolution error like Cannot find `./?__rsc_server__` in react/package.json

// cases: 'react/jsx-runtime', 'react/jsx-dev-runtime'
// This is a special case to avoid the Duplicate React error.
// Since we already include React in the SSR runtime,
// here we can't create a new module with the ?__rsc_server__ query.
if (
['react/jsx-runtime', 'react/jsx-dev-runtime'].includes(
['react', 'react/jsx-runtime', 'react/jsx-dev-runtime'].includes(
importSource
)
) {
Expand Down
15 changes: 7 additions & 8 deletions packages/next/client/index.tsx
Expand Up @@ -547,9 +547,10 @@ function renderReactElement(

const reactEl = fn(shouldHydrate ? markHydrateComplete : markRenderComplete)
if (process.env.__NEXT_REACT_ROOT) {
const ReactDOMClient = require('react-dom/client')
if (!reactRoot) {
// Unlike with createRoot, you don't need a separate root.render() call here
reactRoot = (ReactDOM as any).hydrateRoot(domEl, reactEl)
reactRoot = (ReactDOMClient as any).hydrateRoot(domEl, reactEl)
// TODO: Remove shouldHydrate variable when React 18 is stable as it can depend on `reactRoot` existing
shouldHydrate = false
} else {
Expand Down Expand Up @@ -812,13 +813,11 @@ if (process.env.__NEXT_RSC) {

return (
<RefreshContext.Provider value={refreshCache}>
<React.Suspense fallback={null}>
<ServerRoot
cacheKey={cacheKey}
serialized={__flight_serialized__}
_fresh={__flight_fresh__}
/>
</React.Suspense>
<ServerRoot
cacheKey={cacheKey}
serialized={__flight_serialized__}
_fresh={__flight_fresh__}
/>
</RefreshContext.Provider>
)
}
Expand Down
27 changes: 6 additions & 21 deletions packages/next/server/render.tsx
Expand Up @@ -395,11 +395,7 @@ function createServerComponentRenderer(
}

const Component = (props: any) => {
return (
<React.Suspense fallback={null}>
<ServerComponentWrapper {...props} />
</React.Suspense>
)
return <ServerComponentWrapper {...props} />
}

// Although it's not allowed to attach some static methods to Component,
Expand Down Expand Up @@ -1353,7 +1349,6 @@ export async function renderToHTML(
))}
</>
),
generateStaticHTML: true,
})

const flushed = await streamToString(flushEffectStream)
Expand All @@ -1365,7 +1360,6 @@ export async function renderToHTML(
element: content,
suffix,
dataStream: serverComponentsInlinedTransformStream?.readable,
generateStaticHTML: generateStaticHTML || !hasConcurrentFeatures,
flushEffectHandler,
})
}
Expand Down Expand Up @@ -1498,7 +1492,6 @@ export async function renderToHTML(
const documentStream = await renderToStream({
ReactDOMServer,
element: document,
generateStaticHTML: true,
})
documentHTML = await streamToString(documentStream)
} else {
Expand Down Expand Up @@ -1753,23 +1746,21 @@ function renderToStream({
element,
suffix,
dataStream,
generateStaticHTML,
flushEffectHandler,
}: {
ReactDOMServer: typeof import('react-dom/server')
element: React.ReactElement
suffix?: string
dataStream?: ReadableStream<Uint8Array>
generateStaticHTML: boolean
flushEffectHandler?: () => Promise<string>
}): Promise<ReadableStream<Uint8Array>> {
return new Promise((resolve, reject) => {
return new Promise(async (resolve, reject) => {
let resolved = false

const closeTag = '</body></html>'
const suffixUnclosed = suffix ? suffix.split(closeTag)[0] : null

const doResolve = () => {
const doResolve = (renderStream: ReadableStream<Uint8Array>) => {
if (!resolved) {
resolved = true

Expand Down Expand Up @@ -1798,7 +1789,7 @@ function renderToStream({
}
}

const renderStream: ReadableStream<Uint8Array> = (
const renderStream: ReadableStream<Uint8Array> = await (
ReactDOMServer as any
).renderToReadableStream(element, {
onError(err: Error) {
Expand All @@ -1807,15 +1798,9 @@ function renderToStream({
reject(err)
}
},
onCompleteShell() {
if (!generateStaticHTML) {
doResolve()
}
},
onCompleteAll() {
doResolve()
},
Comment on lines -1815 to -1817
Copy link
Contributor

@devknoll devknoll Mar 3, 2022

Choose a reason for hiding this comment

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

We don't want to remove this, because otherwise we will serve partial streams to crawlers. Instead, you can make the function async now (instead of directly constructing a promise) and do something like this instead:

let resolveAllCompleted
const allCompleted = new Promise(resolve => { resolveAllCompleted = resolve })
const renderStream = await ReactDOMServer.renderToReadableStream(element, {
  onError(err: Error) {
    // TODO: Log non-fatal error
  },
  onCompleteAll() {
    resolveAllCompleted()
  }
}

if (generateStaticHTML) {
  // Wait for the entire stream to complete before resolving
  await allCompleted
}

// doResolve(renderStream) stuff here

Copy link
Contributor

Choose a reason for hiding this comment

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

There should be a test case against this, we should also find out why it didn't fail here...

})

doResolve(renderStream)
})
}

Expand Down
19 changes: 0 additions & 19 deletions test/integration/react-18/app/pages/suspense/unwrapped.js

This file was deleted.

15 changes: 0 additions & 15 deletions test/integration/react-18/test/index.test.js
Expand Up @@ -25,7 +25,6 @@ const nodeArgs = ['-r', join(__dirname, 'require-hook.js')]
const appDir = join(__dirname, '../app')
const nextConfig = new File(join(appDir, 'next.config.js'))
const dynamicHello = new File(join(appDir, 'components/dynamic-hello.js'))
const unwrappedPage = new File(join(appDir, 'pages/suspense/unwrapped.js'))
const invalidPage = new File(join(appDir, 'pages/invalid.js'))

const USING_CREATE_ROOT = 'Using the createRoot API for React'
Expand Down Expand Up @@ -95,20 +94,6 @@ describe('React 18 Support', () => {

describe('Basics', () => {
runTests('default setting with react 18', (context) => basics(context))

it('suspense is not allowed in blocking rendering mode (dev)', async () => {
// set dynamic.suspense = true but not wrapping with <Suspense>
unwrappedPage.replace('wrapped = true', 'wrapped = false')
const appPort = await findPort()
const app = await launchApp(appDir, appPort, { nodeArgs })
const html = await renderViaHTTP(appPort, '/suspense/unwrapped')
unwrappedPage.restore()
await killApp(app)

expect(html).toContain(
'A React component suspended while rendering, but no fallback UI was specified'
)
})
})

// React 18 with Strict Mode enabled might cause double invocation of lifecycle methods.
Expand Down
5 changes: 1 addition & 4 deletions test/integration/react-18/test/with-react-18.js
@@ -1,13 +1,10 @@
module.exports = function withReact18(config) {
config.webpack = (webpackConfig) => {
const { alias } = webpackConfig.resolve
// FIXME: resolving react/jsx-runtime https://github.com/facebook/react/issues/20235
alias['react/jsx-dev-runtime'] = 'react/jsx-dev-runtime.js'
alias['react/jsx-runtime'] = 'react/jsx-runtime.js'

// Use react 18
alias['react'] = 'react-18'
alias['react-dom'] = 'react-dom-18'
alias['react-dom/client'] = 'react-dom-18/client'
alias['react-dom/server'] = 'react-dom-18/server'
alias['react-dom/server.browser'] = 'react-dom-18/server.browser'

Expand Down
@@ -1,7 +1,6 @@
const withReact18 = require('../../react-18/test/with-react-18')

module.exports = withReact18({
trailingSlash: true,
huozhi marked this conversation as resolved.
Show resolved Hide resolved
reactStrictMode: true,
onDemandEntries: {
maxInactiveAge: 1000 * 60 * 60,
Expand Down
Expand Up @@ -77,10 +77,7 @@ export default function (context, { runtime, env }) {

it('should support next/link in server components', async () => {
const linkHTML = await renderViaHTTP(context.appPort, '/next-api/link')
const linkText = getNodeBySelector(
linkHTML,
'div[hidden] > a[href="/"]'
).text()
const linkText = getNodeBySelector(linkHTML, '#__next > a[href="/"]').text()

expect(linkText).toContain('go home')

Expand Down Expand Up @@ -114,7 +111,7 @@ export default function (context, { runtime, env }) {
const imageHTML = await renderViaHTTP(context.appPort, '/next-api/image')
const imageTag = getNodeBySelector(
imageHTML,
'div[hidden] > span > span > img'
'#__next > span > span > img'
)

expect(imageTag.attr('src')).toContain('data:image')
Expand Down Expand Up @@ -176,13 +173,13 @@ export default function (context, { runtime, env }) {
expect(
getNodeBySelector(
clientExportsHTML,
'div[hidden] > div > #named-exports'
'#__next > div > #named-exports'
).text()
).toBe('abcde')
expect(
getNodeBySelector(
clientExportsHTML,
'div[hidden] > div > #default-exports-arrow'
'#__next > div > #default-exports-arrow'
).text()
).toBe('client-default-export-arrow')

Expand Down
27 changes: 17 additions & 10 deletions test/production/react-18-streaming-ssr/index.test.ts
Expand Up @@ -24,8 +24,8 @@ describe('react 18 streaming SSR in minimal mode', () => {
},
},
dependencies: {
react: '18.0.0-rc.0',
'react-dom': '18.0.0-rc.0',
react: '18.0.0-rc.1',
'react-dom': '18.0.0-rc.1',
},
})
})
Expand All @@ -47,8 +47,8 @@ describe('react 18 streaming SSR with custom next configs', () => {
next = await createNext({
files: {
'pages/hello.js': `
export default function Page() {
return <p>hello</p>
export default function Page() {
return <p>hello nextjs</p>
}
`,
},
Expand All @@ -60,19 +60,26 @@ describe('react 18 streaming SSR with custom next configs', () => {
},
},
dependencies: {
react: '18.0.0-rc.0',
'react-dom': '18.0.0-rc.0',
react: '18.0.0-rc.1',
'react-dom': '18.0.0-rc.1',
},
})
})
afterAll(() => next.destroy())

it('should redirect paths without trailing-slash and render when slash is appended', async () => {
const page = '/hello'
const html = await renderViaHTTP(next.url, page + '/')
const res = await fetchViaHTTP(next.url, page, {}, { redirect: 'manual' })
const redirectRes = await fetchViaHTTP(
next.url,
page,
{},
{ redirect: 'manual' }
)
const res = await fetchViaHTTP(next.url, page + '/')
const html = await res.text()

expect(html).toContain('hello')
expect(res.status).toBe(308)
expect(redirectRes.status).toBe(308)
expect(res.status).toBe(200)
expect(html).toContain('hello nextjs')
})
})
29 changes: 13 additions & 16 deletions yarn.lock
Expand Up @@ -17527,22 +17527,20 @@ rc@^1.0.1, rc@^1.1.6, rc@^1.2.7, rc@^1.2.8:
minimist "^1.2.0"
strip-json-comments "~2.0.1"

"react-18@npm:react@18.0.0-rc.0":
version "18.0.0-rc.0"
resolved "https://registry.yarnpkg.com/react/-/react-18.0.0-rc.0.tgz#60bfcf1edd0b35fbeeeca852515c6cc2ce06a6eb"
integrity sha512-PawosMBgF8k5Nlc3++ibzjFqPvo1XKv80MNtVYqz3abHHB2w3IpU65sSdSmBd2ooCwVhcp9b1vkx/twqhakNtA==
"react-18@npm:react@18.0.0-rc.1":
version "18.0.0-rc.1"
resolved "https://registry.yarnpkg.com/react/-/react-18.0.0-rc.1.tgz#fbaef1cbf8b1eddb36a869ee1ea723044aa670c2"
integrity sha512-IdvjOtyeFhITZwDrj+GMvY7kbWnGklRzNByO7/pOVIt0o3VZi++BUybpK3Fen1gDPkOZDP/rag7lSh6RZuWOeQ==
dependencies:
loose-envify "^1.1.0"
object-assign "^4.1.1"

"react-dom-18@npm:react-dom@18.0.0-rc.0":
version "18.0.0-rc.0"
resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.0.0-rc.0.tgz#aa07044bdd6399ff94c664b2985e2e25948fbf3e"
integrity sha512-tdD1n0svTndHBQvVAq/f2Kx7FgQ30CpSLp87/neQKAHPW5WtdgW1sBSwmFAcMQOrmstTuP0M+zRlH86f9kMX/A==
"react-dom-18@npm:react-dom@18.0.0-rc.1":
version "18.0.0-rc.1"
resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.0.0-rc.1.tgz#97296d5a5e7582e65ea0d82a9ccd5953aa05e7f6"
integrity sha512-BZ9NEwUp56MEguEwAzuh3u4bYE9Jv3QrzjaTmu11PV4C/lJCARTELEI16vjnHLq184GoJcCHMBrDILqlCrkZFQ==
dependencies:
loose-envify "^1.1.0"
object-assign "^4.1.1"
scheduler "^0.21.0-rc.0"
scheduler "^0.21.0-rc.1"

react-dom@17.0.2:
version "17.0.2"
Expand Down Expand Up @@ -18543,13 +18541,12 @@ scheduler@^0.20.2:
loose-envify "^1.1.0"
object-assign "^4.1.1"

scheduler@^0.21.0-rc.0:
version "0.21.0-rc.0-next-f2a59df48-20211208"
resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.21.0-rc.0-next-f2a59df48-20211208.tgz#54e18e1d360194fd54b47a00616e46403fcabdf1"
integrity sha512-x0oLd3YIih9GHqWTaFYejVe6Au+4TadOWZciAq8m4+Fuo5qCi4/3M35a9irVSIP3+qcg/fCqHKJETT9G0ejD1A==
scheduler@^0.21.0-rc.1:
version "0.21.0-rc.1-next-f468816ef-20220225"
resolved "https://registry.yarnpkg.com/scheduler/-/scheduler-0.21.0-rc.1-next-f468816ef-20220225.tgz#1909b9ce9e67de48f5ca06b52d98828401771e66"
integrity sha512-/maKldJ78Ba2o1e9kh2mUpqNZH7XrmSQ1o1aa3HUPN/OMotUB9+pBKX/y3Zihkjco21H4cujG9hK2sJOZUpzJw==
dependencies:
loose-envify "^1.1.0"
object-assign "^4.1.1"

"schema-utils2@npm:schema-utils@2.7.1":
version "2.7.1"
Expand Down