From 0483d06563353abe6a51ff6b1530bf68ad0414f7 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Sun, 1 May 2022 00:18:53 +0200 Subject: [PATCH 1/2] Use flushed effects to generate styled-jsx styles insted of gIP by default --- packages/next/pages/_document.tsx | 9 ++++-- packages/next/server/base-server.ts | 3 +- packages/next/server/render.tsx | 29 ++++++++++++-------- packages/next/shared/lib/constants.ts | 1 + test/e2e/styled-jsx/app/pages/index.js | 8 ++++++ test/e2e/styled-jsx/index.test.ts | 19 +++++-------- test/integration/react-18/test/concurrent.js | 9 +++++- 7 files changed, 51 insertions(+), 27 deletions(-) create mode 100644 test/e2e/styled-jsx/app/pages/index.js diff --git a/packages/next/pages/_document.tsx b/packages/next/pages/_document.tsx index 11dcf48406db..badc0f7a6b85 100644 --- a/packages/next/pages/_document.tsx +++ b/packages/next/pages/_document.tsx @@ -1,9 +1,13 @@ import React, { Component, ReactElement, ReactNode, useContext } from 'react' -import { OPTIMIZED_FONT_PROVIDERS } from '../shared/lib/constants' +import { + OPTIMIZED_FONT_PROVIDERS, + NEXT_BUILTIN_DOCUMENT, +} from '../shared/lib/constants' import type { DocumentContext, DocumentInitialProps, DocumentProps, + DocumentType, } from '../shared/lib/utils' import { BuildManifest, getPageFiles } from '../server/get-page-files' import { cleanAmpPath } from '../server/utils' @@ -309,7 +313,7 @@ export default class Document

extends Component { // Add a special property to the built-in `Document` component so later we can // identify if a user customized `Document` is used or not. -;(Document as any).__next_internal_document = +const InternalFunctionDocument: DocumentType = function InternalFunctionDocument() { return ( @@ -321,6 +325,7 @@ export default class Document

extends Component { ) } +;(Document as any)[NEXT_BUILTIN_DOCUMENT] = InternalFunctionDocument export function Html( props: React.DetailedHTMLProps< diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index f47d30f94a0c..ab7215f4ab71 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -22,6 +22,7 @@ import { parse as parseQs } from 'querystring' import { format as formatUrl, parse as parseUrl } from 'url' import { getRedirectStatus } from '../lib/load-custom-routes' import { + NEXT_BUILTIN_DOCUMENT, SERVERLESS_DIRECTORY, SERVER_DIRECTORY, STATIC_STATUS_PAGES, @@ -1216,7 +1217,7 @@ export default abstract class Server { // When concurrent features is enabled, the built-in `Document` // component also supports dynamic HTML. (this.renderOpts.reactRoot && - !!(components.Document as any)?.__next_internal_document) + NEXT_BUILTIN_DOCUMENT in components.Document) // Disable dynamic HTML in cases that we know it won't be generated, // so that we can continue generating a cache key when possible. diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index cd3af5d6d4fd..e017e564ca28 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -6,6 +6,7 @@ import type { DomainLocale } from './config' import type { AppType, DocumentInitialProps, + DocumentType, DocumentProps, DocumentContext, NextComponentType, @@ -35,6 +36,7 @@ import { UNSTABLE_REVALIDATE_RENAME_ERROR, } from '../lib/constants' import { + NEXT_BUILTIN_DOCUMENT, SERVER_PROPS_ID, STATIC_PROPS_ID, STATIC_STATUS_PAGES, @@ -88,6 +90,7 @@ let warn: typeof import('../build/output/log').warn let postProcess: typeof import('../shared/lib/post-process').default const DOCTYPE = '' +const GIP_GEN_STYLES = '__GIP_GEN_STYLES__' const ReactDOMServer = process.env.__NEXT_REACT_ROOT ? require('react-dom/server.browser') : require('react-dom/server') @@ -769,6 +772,7 @@ export async function renderToHTML( const { html, head } = await docCtx.renderPage({ enhanceApp }) const styles = jsxStyleRegistry.styles({ nonce: options.nonce }) + ;(styles as any)[GIP_GEN_STYLES] = true return { html, head, styles } }, } @@ -1311,14 +1315,14 @@ export async function renderToHTML( // 1. Using `Document.getInitialProps` in the Edge runtime. // 2. Using the class component `Document` with concurrent features. - const builtinDocument = (Document as any).__next_internal_document as - | typeof Document - | undefined + const BuiltinFunctionalDocument: DocumentType | undefined = ( + Document as any + )[NEXT_BUILTIN_DOCUMENT] if (process.env.NEXT_RUNTIME === 'edge' && Document.getInitialProps) { // In the Edge runtime, `Document.getInitialProps` isn't supported. // We throw an error here if it's customized. - if (!builtinDocument) { + if (!BuiltinFunctionalDocument) { throw new Error( '`getInitialProps` in Document component is not supported with the Edge Runtime.' ) @@ -1329,8 +1333,8 @@ export async function renderToHTML( (isServerComponent || process.env.NEXT_RUNTIME === 'edge') && Document.getInitialProps ) { - if (builtinDocument) { - Document = builtinDocument + if (BuiltinFunctionalDocument) { + Document = BuiltinFunctionalDocument } else { throw new Error( '`getInitialProps` in Document component is not supported with React Server Components.' @@ -1433,6 +1437,7 @@ export async function renderToHTML( } if (!process.env.__NEXT_REACT_ROOT) { + // Enabling react legacy rendering mode: __NEXT_REACT_ROOT = false if (Document.getInitialProps) { const documentInitialPropsRes = await documentInitialProps() if (documentInitialPropsRes === null) return null @@ -1468,6 +1473,7 @@ export async function renderToHTML( } } } else { + // Enabling react concurrent rendering mode: __NEXT_REACT_ROOT = true let renderStream: ReadableStream & { allReady?: Promise | undefined } @@ -1534,13 +1540,11 @@ export async function renderToHTML( // be streamed. // Do not use `await` here. generateStaticFlightDataIfNeeded() - return await continueFromInitialStream({ renderStream, suffix, dataStream: serverComponentsInlinedTransformStream?.readable, - generateStaticHTML: - generateStaticHTML || !process.env.__NEXT_REACT_ROOT, + generateStaticHTML, flushEffectHandler, }) } @@ -1572,10 +1576,13 @@ export async function renderToHTML( return } - let styles + let styles: any[] if (hasDocumentGetInitialProps) { - styles = docProps.styles + // If styles are generated from default Document.getInitialProps in concurrent mode, + // discard it and use generated styles from flush-effects instead. + // To guarantee it's concurrent safe and no duplicates in both head and body. + styles = docProps.styles[GIP_GEN_STYLES] ? [] : docProps.styles } else { styles = jsxStyleRegistry.styles() jsxStyleRegistry.flush() diff --git a/packages/next/shared/lib/constants.ts b/packages/next/shared/lib/constants.ts index bedca6446bb5..7f3dc9acd928 100644 --- a/packages/next/shared/lib/constants.ts +++ b/packages/next/shared/lib/constants.ts @@ -25,6 +25,7 @@ export const CLIENT_PUBLIC_FILES_PATH = 'public' export const CLIENT_STATIC_FILES_PATH = 'static' export const CLIENT_STATIC_FILES_RUNTIME = 'runtime' export const STRING_LITERAL_DROP_BUNDLE = '__NEXT_DROP_CLIENT_FILE__' +export const NEXT_BUILTIN_DOCUMENT = '__NEXT_BUILTIN_DOCUMENT__' // server/middleware-flight-manifest.js export const MIDDLEWARE_FLIGHT_MANIFEST = 'middleware-flight-manifest' diff --git a/test/e2e/styled-jsx/app/pages/index.js b/test/e2e/styled-jsx/app/pages/index.js new file mode 100644 index 000000000000..31c938b25e4f --- /dev/null +++ b/test/e2e/styled-jsx/app/pages/index.js @@ -0,0 +1,8 @@ +export default function Page() { + return ( +

+ +

hello world

+
+ ) +} diff --git a/test/e2e/styled-jsx/index.test.ts b/test/e2e/styled-jsx/index.test.ts index 414570939fe6..189a37c79785 100644 --- a/test/e2e/styled-jsx/index.test.ts +++ b/test/e2e/styled-jsx/index.test.ts @@ -1,8 +1,11 @@ -import { createNext } from 'e2e-utils' +import path from 'path' +import { createNext, FileRef } from 'e2e-utils' import { NextInstance } from 'test/lib/next-modes/base' import { renderViaHTTP } from 'next-test-utils' import cheerio from 'cheerio' +const appDir = path.join(__dirname, 'app') + function runTest(packageManager?: string) { describe(`styled-jsx${packageManager ? ' ' + packageManager : ''}`, () => { let next: NextInstance @@ -10,16 +13,7 @@ function runTest(packageManager?: string) { beforeAll(async () => { next = await createNext({ files: { - 'pages/index.js': ` - export default function Page() { - return ( -
- -

hello world

-
- ) - } - `, + pages: new FileRef(path.join(appDir, 'pages')), }, ...(packageManager ? { @@ -33,7 +27,8 @@ function runTest(packageManager?: string) { it('should contain styled-jsx styles in html', async () => { const html = await renderViaHTTP(next.url, '/') const $ = cheerio.load(html) - expect($('head').text()).toContain('color:red') + const isReact17 = process.env.NEXT_TEST_REACT_VERSION === '^17' + expect($(isReact17 ? 'head' : 'body').text()).toMatch(/color:(\s)*red/) }) }) } diff --git a/test/integration/react-18/test/concurrent.js b/test/integration/react-18/test/concurrent.js index 92871dbab3b8..f849a0a80c94 100644 --- a/test/integration/react-18/test/concurrent.js +++ b/test/integration/react-18/test/concurrent.js @@ -32,7 +32,14 @@ export default (context, _render) => { }) }) - it('flushes styles as the page renders', async () => { + it('flushes styled-jsx styles as the page renders', async () => { + const html = await renderViaHTTP( + context.appPort, + '/use-flush-effect/styled-jsx' + ) + const stylesOccurrence = html.match(/color:(\s)*blue/g) || [] + expect(stylesOccurrence.length).toBe(1) + await withBrowser('/use-flush-effect/styled-jsx', async (browser) => { await check( () => browser.waitForElementByCss('#__jsx-900f996af369fc74').text(), From 4e20a12e890defce24d96df365e5619398c4685b Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Mon, 2 May 2022 22:19:33 +0200 Subject: [PATCH 2/2] ensure styles are flushed inside the default getInitialProps --- packages/next/server/render.tsx | 10 +++------- test/e2e/styled-jsx/index.test.ts | 3 +-- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/next/server/render.tsx b/packages/next/server/render.tsx index e017e564ca28..f38083b00d1b 100644 --- a/packages/next/server/render.tsx +++ b/packages/next/server/render.tsx @@ -90,7 +90,6 @@ let warn: typeof import('../build/output/log').warn let postProcess: typeof import('../shared/lib/post-process').default const DOCTYPE = '' -const GIP_GEN_STYLES = '__GIP_GEN_STYLES__' const ReactDOMServer = process.env.__NEXT_REACT_ROOT ? require('react-dom/server.browser') : require('react-dom/server') @@ -772,7 +771,7 @@ export async function renderToHTML( const { html, head } = await docCtx.renderPage({ enhanceApp }) const styles = jsxStyleRegistry.styles({ nonce: options.nonce }) - ;(styles as any)[GIP_GEN_STYLES] = true + jsxStyleRegistry.flush() return { html, head, styles } }, } @@ -1577,12 +1576,9 @@ export async function renderToHTML( return } - let styles: any[] + let styles if (hasDocumentGetInitialProps) { - // If styles are generated from default Document.getInitialProps in concurrent mode, - // discard it and use generated styles from flush-effects instead. - // To guarantee it's concurrent safe and no duplicates in both head and body. - styles = docProps.styles[GIP_GEN_STYLES] ? [] : docProps.styles + styles = docProps.styles } else { styles = jsxStyleRegistry.styles() jsxStyleRegistry.flush() diff --git a/test/e2e/styled-jsx/index.test.ts b/test/e2e/styled-jsx/index.test.ts index 189a37c79785..c0f52f658ff9 100644 --- a/test/e2e/styled-jsx/index.test.ts +++ b/test/e2e/styled-jsx/index.test.ts @@ -27,8 +27,7 @@ function runTest(packageManager?: string) { it('should contain styled-jsx styles in html', async () => { const html = await renderViaHTTP(next.url, '/') const $ = cheerio.load(html) - const isReact17 = process.env.NEXT_TEST_REACT_VERSION === '^17' - expect($(isReact17 ? 'head' : 'body').text()).toMatch(/color:(\s)*red/) + expect($('html').text()).toMatch(/color:(\s)*red/) }) }) }