From 0e5b9f1dfa49113a697f307fc430522f576acf1f Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Tue, 19 Nov 2024 23:05:58 +0900 Subject: [PATCH 1/6] test: current behavior --- .../dynamic-import/dynamic-import.test.ts | 35 +++++++++++ .../src/components/red-button-lazy.tsx | 13 ++++ .../src/components/red-button.tsx | 10 +++ .../src/components/red.module.css | 3 + .../src/pages/dynamic-import.tsx | 6 ++ .../dynamic-import/src/pages/index.tsx | 15 +++++ .../next-dynamic.test.ts | 61 +++++++++++++++++++ .../react-lazy.test.ts | 35 +++++++++++ .../src/components/red-button.tsx | 10 +++ .../src/components/red.module.css | 3 + .../src/pages/index.tsx | 15 +++++ .../src/pages/next-dynamic-ssr-false.tsx | 11 ++++ .../src/pages/next-dynamic.tsx | 10 +++ .../src/pages/react-lazy.tsx | 15 +++++ 14 files changed, 242 insertions(+) create mode 100644 test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts create mode 100644 test/production/dynamic-css-client-navigation/dynamic-import/src/components/red-button-lazy.tsx create mode 100644 test/production/dynamic-css-client-navigation/dynamic-import/src/components/red-button.tsx create mode 100644 test/production/dynamic-css-client-navigation/dynamic-import/src/components/red.module.css create mode 100644 test/production/dynamic-css-client-navigation/dynamic-import/src/pages/dynamic-import.tsx create mode 100644 test/production/dynamic-css-client-navigation/dynamic-import/src/pages/index.tsx create mode 100644 test/production/dynamic-css-client-navigation/next-dynamic.test.ts create mode 100644 test/production/dynamic-css-client-navigation/react-lazy.test.ts create mode 100644 test/production/dynamic-css-client-navigation/src/components/red-button.tsx create mode 100644 test/production/dynamic-css-client-navigation/src/components/red.module.css create mode 100644 test/production/dynamic-css-client-navigation/src/pages/index.tsx create mode 100644 test/production/dynamic-css-client-navigation/src/pages/next-dynamic-ssr-false.tsx create mode 100644 test/production/dynamic-css-client-navigation/src/pages/next-dynamic.tsx create mode 100644 test/production/dynamic-css-client-navigation/src/pages/react-lazy.tsx diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts b/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts new file mode 100644 index 0000000000000..644a5b7d5411c --- /dev/null +++ b/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts @@ -0,0 +1,35 @@ +import { nextTestSetup } from 'e2e-utils' + +describe('dynamic-css-client-navigation dynamic import', () => { + const { next, isTurbopack } = nextTestSetup({ + files: __dirname, + }) + + const isTurbopackBuild = isTurbopack && process.env.TURBOPACK_BUILD + + it('should not remove style when navigating from static imported component to dynamic import', async () => { + const browser = await next.browser('/') + expect( + await browser + .elementByCss('a[href="/dynamic-import"]') + .click() + .waitForElementByCss('#red-button') + .text() + ).toBe('Red Button') + + const buttonBgColor = await browser.eval( + `window.getComputedStyle(document.querySelector('button')).backgroundColor` + ) + + // TODO: remove this condition after fix + if (isTurbopackBuild) { + expect(buttonBgColor).not.toBe('rgb(239, 239, 239)') + expect(buttonBgColor).toBe('rgb(255, 0, 0)') + } else { + // TODO: replace this after fix + // should be red, but is gray now. + expect(buttonBgColor).toBe('rgb(239, 239, 239)') + expect(buttonBgColor).not.toBe('rgb(255, 0, 0)') + } + }) +}) diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/src/components/red-button-lazy.tsx b/test/production/dynamic-css-client-navigation/dynamic-import/src/components/red-button-lazy.tsx new file mode 100644 index 0000000000000..9892eb36f527d --- /dev/null +++ b/test/production/dynamic-css-client-navigation/dynamic-import/src/components/red-button-lazy.tsx @@ -0,0 +1,13 @@ +import React, { useEffect, useState } from 'react' + +export function RedButtonLazy() { + const [Component, setComponent] = useState(null) + + useEffect(() => { + import('./red-button').then((module) => + setComponent(() => module.RedButton) + ) + }, []) + + return Component && +} diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/src/components/red-button.tsx b/test/production/dynamic-css-client-navigation/dynamic-import/src/components/red-button.tsx new file mode 100644 index 0000000000000..46a3f6e01d2d3 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/dynamic-import/src/components/red-button.tsx @@ -0,0 +1,10 @@ +import React from 'react' +import classes from './red.module.css' + +export function RedButton() { + return ( + + ) +} diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/src/components/red.module.css b/test/production/dynamic-css-client-navigation/dynamic-import/src/components/red.module.css new file mode 100644 index 0000000000000..905be4a9e18b4 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/dynamic-import/src/components/red.module.css @@ -0,0 +1,3 @@ +.button { + background-color: rgb(255, 0, 0); +} diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/dynamic-import.tsx b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/dynamic-import.tsx new file mode 100644 index 0000000000000..12cf5867e938a --- /dev/null +++ b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/dynamic-import.tsx @@ -0,0 +1,6 @@ +import React from 'react' +import { RedButtonLazy } from '../components/red-button-lazy' + +export default function DynamicImport() { + return +} diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/index.tsx b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/index.tsx new file mode 100644 index 0000000000000..5cf38549d8299 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/index.tsx @@ -0,0 +1,15 @@ +import React from 'react' +import Link from 'next/link' +import { RedButton } from '../components/red-button' +import { RedButtonLazy } from '../components/red-button-lazy' + +export default function Home() { + return ( + <> + {/* To reproduce, RedButton and RedButtonLazy should be imported. */} + + + /dynamic-import + + ) +} diff --git a/test/production/dynamic-css-client-navigation/next-dynamic.test.ts b/test/production/dynamic-css-client-navigation/next-dynamic.test.ts new file mode 100644 index 0000000000000..727e87e191e36 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/next-dynamic.test.ts @@ -0,0 +1,61 @@ +import { nextTestSetup } from 'e2e-utils' + +describe('dynamic-css-client-navigation next/dynamic', () => { + const { next, isTurbopack } = nextTestSetup({ + files: __dirname, + }) + + const isTurbopackBuild = isTurbopack && process.env.TURBOPACK_BUILD + + it('should not remove style when navigating from static imported component to next/dynamic', async () => { + const browser = await next.browser('/') + expect( + await browser + .elementByCss('a[href="/next-dynamic"]') + .click() + .waitForElementByCss('#red-button') + .text() + ).toBe('Red Button') + + const buttonBgColor = await browser.eval( + `window.getComputedStyle(document.querySelector('button')).backgroundColor` + ) + + // TODO: remove this condition after fix + if (isTurbopackBuild) { + expect(buttonBgColor).not.toBe('rgb(239, 239, 239)') + expect(buttonBgColor).toBe('rgb(255, 0, 0)') + } else { + // TODO: replace this after fix + // should be red, but is gray now. + expect(buttonBgColor).toBe('rgb(239, 239, 239)') + expect(buttonBgColor).not.toBe('rgb(255, 0, 0)') + } + }) + + it('should not remove style when navigating from static imported component to next/dynamic with ssr: false', async () => { + const browser = await next.browser('/') + expect( + await browser + .elementByCss('a[href="/next-dynamic-ssr-false"]') + .click() + .waitForElementByCss('#red-button') + .text() + ).toBe('Red Button') + + const buttonBgColor = await browser.eval( + `window.getComputedStyle(document.querySelector('button')).backgroundColor` + ) + + // TODO: remove this condition after fix + if (isTurbopackBuild) { + expect(buttonBgColor).not.toBe('rgb(239, 239, 239)') + expect(buttonBgColor).toBe('rgb(255, 0, 0)') + } else { + // TODO: replace this after fix + // should be red, but is gray now. + expect(buttonBgColor).toBe('rgb(239, 239, 239)') + expect(buttonBgColor).not.toBe('rgb(255, 0, 0)') + } + }) +}) diff --git a/test/production/dynamic-css-client-navigation/react-lazy.test.ts b/test/production/dynamic-css-client-navigation/react-lazy.test.ts new file mode 100644 index 0000000000000..dfb6bfb6cf7a3 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/react-lazy.test.ts @@ -0,0 +1,35 @@ +import { nextTestSetup } from 'e2e-utils' + +describe('dynamic-css-client-navigation react lazy', () => { + const { next, isTurbopack } = nextTestSetup({ + files: __dirname, + }) + + const isTurbopackBuild = isTurbopack && process.env.TURBOPACK_BUILD + + it('should not remove style when navigating from static imported component to react lazy', async () => { + const browser = await next.browser('/') + expect( + await browser + .elementByCss('a[href="/react-lazy"]') + .click() + .waitForElementByCss('#red-button') + .text() + ).toBe('Red Button') + + const buttonBgColor = await browser.eval( + `window.getComputedStyle(document.querySelector('button')).backgroundColor` + ) + + // TODO: remove this condition after fix + if (isTurbopackBuild) { + expect(buttonBgColor).not.toBe('rgb(239, 239, 239)') + expect(buttonBgColor).toBe('rgb(255, 0, 0)') + } else { + // TODO: replace this after fix + // should be red, but is gray now. + expect(buttonBgColor).toBe('rgb(239, 239, 239)') + expect(buttonBgColor).not.toBe('rgb(255, 0, 0)') + } + }) +}) diff --git a/test/production/dynamic-css-client-navigation/src/components/red-button.tsx b/test/production/dynamic-css-client-navigation/src/components/red-button.tsx new file mode 100644 index 0000000000000..46a3f6e01d2d3 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/components/red-button.tsx @@ -0,0 +1,10 @@ +import React from 'react' +import classes from './red.module.css' + +export function RedButton() { + return ( + + ) +} diff --git a/test/production/dynamic-css-client-navigation/src/components/red.module.css b/test/production/dynamic-css-client-navigation/src/components/red.module.css new file mode 100644 index 0000000000000..905be4a9e18b4 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/components/red.module.css @@ -0,0 +1,3 @@ +.button { + background-color: rgb(255, 0, 0); +} diff --git a/test/production/dynamic-css-client-navigation/src/pages/index.tsx b/test/production/dynamic-css-client-navigation/src/pages/index.tsx new file mode 100644 index 0000000000000..854254ad65722 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/pages/index.tsx @@ -0,0 +1,15 @@ +import React from 'react' +import Link from 'next/link' +import { RedButton } from '../components/red-button' + +export default function Home() { + return ( + <> + {/* To reproduce, RedButton should be imported. */} + + /next-dynamic + /next-dynamic-ssr-false + /react-lazy + + ) +} diff --git a/test/production/dynamic-css-client-navigation/src/pages/next-dynamic-ssr-false.tsx b/test/production/dynamic-css-client-navigation/src/pages/next-dynamic-ssr-false.tsx new file mode 100644 index 0000000000000..b4ec3bfac8291 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/pages/next-dynamic-ssr-false.tsx @@ -0,0 +1,11 @@ +import React from 'react' +import dynamic from 'next/dynamic' + +const NextDynamicRedButton = dynamic( + () => import('../components/red-button').then((module) => module.RedButton), + { ssr: false } +) + +export default function NextDynamic() { + return +} diff --git a/test/production/dynamic-css-client-navigation/src/pages/next-dynamic.tsx b/test/production/dynamic-css-client-navigation/src/pages/next-dynamic.tsx new file mode 100644 index 0000000000000..4ae4907490502 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/pages/next-dynamic.tsx @@ -0,0 +1,10 @@ +import React from 'react' +import dynamic from 'next/dynamic' + +const NextDynamicRedButton = dynamic(() => + import('../components/red-button').then((module) => module.RedButton) +) + +export default function NextDynamic() { + return +} diff --git a/test/production/dynamic-css-client-navigation/src/pages/react-lazy.tsx b/test/production/dynamic-css-client-navigation/src/pages/react-lazy.tsx new file mode 100644 index 0000000000000..b920554e40801 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/pages/react-lazy.tsx @@ -0,0 +1,15 @@ +import React, { lazy, Suspense } from 'react' + +const ReactLazyRedButton = lazy(() => + import('../components/red-button').then((module) => ({ + default: module.RedButton, + })) +) + +export default function ReactLazy() { + return ( + + + + ) +} From e70a299d4fbb78e90aa0784a67a3e2aae8bf460c Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Wed, 20 Nov 2024 04:36:25 +0900 Subject: [PATCH 2/6] fix and update test --- packages/next/src/client/index.tsx | 26 ++++++++++++++++++- packages/next/src/server/render.tsx | 13 ++++++++++ packages/next/src/shared/lib/utils.ts | 1 + .../dynamic-import/dynamic-import.test.ts | 15 ++--------- .../next-dynamic.test.ts | 26 +++---------------- .../react-lazy.test.ts | 15 ++--------- 6 files changed, 46 insertions(+), 50 deletions(-) diff --git a/packages/next/src/client/index.tsx b/packages/next/src/client/index.tsx index 42ebd0da9bb09..48c44969c8c42 100644 --- a/packages/next/src/client/index.tsx +++ b/packages/next/src/client/index.tsx @@ -89,6 +89,9 @@ let headManager: { } let initialMatchesMiddleware = false let lastAppProps: AppProps +let dynamicCSSFiles: string[] +let dynamicCSSFilePaths: Set +let publicAssetPath: string let lastRenderReject: (() => void) | null let devClient: any @@ -202,10 +205,16 @@ export async function initialize(opts: { devClient?: any } = {}): Promise<{ window.__NEXT_DATA__ = initialData defaultLocale = initialData.defaultLocale + dynamicCSSFiles = initialData.dynamicCSSFiles || [] const prefix: string = initialData.assetPrefix || '' // With dynamic assetPrefix it's no longer possible to set assetPrefix at the build time // So, this is how we do it in the client side at runtime - ;(self as any).__next_set_public_path__(`${prefix}/_next/`) //eslint-disable-line + publicAssetPath = prefix + '/_next/' + ;(self as any).__next_set_public_path__(publicAssetPath) //eslint-disable-line + + dynamicCSSFilePaths = new Set( + dynamicCSSFiles.map((cssFile) => `${publicAssetPath}${cssFile}`) + ) // Initialize next/config with the environment configuration setConfig({ @@ -757,6 +766,21 @@ function doRender(input: RenderRouteInfo): Promise { looseToArray( document.querySelectorAll('link[data-n-p]') ).forEach((el) => { + const href = el.getAttribute('href') + if (!href) { + return + } + // When client-navigating to a path which requires the same CSS file, + // `mini-css-extract-plugin` will skip injecting the stylesheet as the + // link tag with expected href is still in the DOM. Therefore we remove + // the `data-n-p` attribute and let the stylesheet remain. This is possible + // because we do not "unload" the dynamic stylesheets during the client + // navigation already. Therefore the result will be the same with removing + // the current stylesheet and then dynamically loading the same stylesheet. + if (dynamicCSSFilePaths.has(href)) { + el.removeAttribute('data-n-p') + return + } el.parentNode!.removeChild(el) }) } diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index 148430f6dbeb2..0b50230666069 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -1416,6 +1416,17 @@ export async function renderToHTMLImpl( } } + const dynamicCSSFiles = new Set() + Object.values(reactLoadableManifest).forEach(({ files }) => { + if (files) { + files.forEach((file) => { + if (file.endsWith('.css')) { + dynamicCSSFiles.add(file) + } + }) + } + }) + const hybridAmp = ampState.hybrid const docComponentsRendered: DocumentProps['docComponentsRendered'] = {} @@ -1446,6 +1457,8 @@ export async function renderToHTMLImpl( dynamicImportsIds.size === 0 ? undefined : Array.from(dynamicImportsIds), + dynamicCSSFiles: + dynamicCSSFiles.size > 0 ? Array.from(dynamicCSSFiles) : undefined, err: renderOpts.err ? serializeError(dev, renderOpts.err) : undefined, // Error if one happened, otherwise don't sent in the resulting HTML gsp: !!getStaticProps ? true : undefined, // whether the page is getStaticProps gssp: !!getServerSideProps ? true : undefined, // whether the page is getServerSideProps diff --git a/packages/next/src/shared/lib/utils.ts b/packages/next/src/shared/lib/utils.ts index ff1955ab2456c..d0f63dfb57542 100644 --- a/packages/next/src/shared/lib/utils.ts +++ b/packages/next/src/shared/lib/utils.ts @@ -97,6 +97,7 @@ export type NEXT_DATA = { isFallback?: boolean isExperimentalCompile?: boolean dynamicIds?: (string | number)[] + dynamicCSSFiles?: string[] err?: Error & { statusCode?: number source?: typeof COMPILER_NAMES.server | typeof COMPILER_NAMES.edgeServer diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts b/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts index 644a5b7d5411c..320a5cad11eba 100644 --- a/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts +++ b/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts @@ -1,12 +1,10 @@ import { nextTestSetup } from 'e2e-utils' describe('dynamic-css-client-navigation dynamic import', () => { - const { next, isTurbopack } = nextTestSetup({ + const { next } = nextTestSetup({ files: __dirname, }) - const isTurbopackBuild = isTurbopack && process.env.TURBOPACK_BUILD - it('should not remove style when navigating from static imported component to dynamic import', async () => { const browser = await next.browser('/') expect( @@ -21,15 +19,6 @@ describe('dynamic-css-client-navigation dynamic import', () => { `window.getComputedStyle(document.querySelector('button')).backgroundColor` ) - // TODO: remove this condition after fix - if (isTurbopackBuild) { - expect(buttonBgColor).not.toBe('rgb(239, 239, 239)') - expect(buttonBgColor).toBe('rgb(255, 0, 0)') - } else { - // TODO: replace this after fix - // should be red, but is gray now. - expect(buttonBgColor).toBe('rgb(239, 239, 239)') - expect(buttonBgColor).not.toBe('rgb(255, 0, 0)') - } + expect(buttonBgColor).toBe('rgb(255, 0, 0)') }) }) diff --git a/test/production/dynamic-css-client-navigation/next-dynamic.test.ts b/test/production/dynamic-css-client-navigation/next-dynamic.test.ts index 727e87e191e36..ddc4f811d4422 100644 --- a/test/production/dynamic-css-client-navigation/next-dynamic.test.ts +++ b/test/production/dynamic-css-client-navigation/next-dynamic.test.ts @@ -1,12 +1,10 @@ import { nextTestSetup } from 'e2e-utils' describe('dynamic-css-client-navigation next/dynamic', () => { - const { next, isTurbopack } = nextTestSetup({ + const { next } = nextTestSetup({ files: __dirname, }) - const isTurbopackBuild = isTurbopack && process.env.TURBOPACK_BUILD - it('should not remove style when navigating from static imported component to next/dynamic', async () => { const browser = await next.browser('/') expect( @@ -21,16 +19,7 @@ describe('dynamic-css-client-navigation next/dynamic', () => { `window.getComputedStyle(document.querySelector('button')).backgroundColor` ) - // TODO: remove this condition after fix - if (isTurbopackBuild) { - expect(buttonBgColor).not.toBe('rgb(239, 239, 239)') - expect(buttonBgColor).toBe('rgb(255, 0, 0)') - } else { - // TODO: replace this after fix - // should be red, but is gray now. - expect(buttonBgColor).toBe('rgb(239, 239, 239)') - expect(buttonBgColor).not.toBe('rgb(255, 0, 0)') - } + expect(buttonBgColor).toBe('rgb(255, 0, 0)') }) it('should not remove style when navigating from static imported component to next/dynamic with ssr: false', async () => { @@ -47,15 +36,6 @@ describe('dynamic-css-client-navigation next/dynamic', () => { `window.getComputedStyle(document.querySelector('button')).backgroundColor` ) - // TODO: remove this condition after fix - if (isTurbopackBuild) { - expect(buttonBgColor).not.toBe('rgb(239, 239, 239)') - expect(buttonBgColor).toBe('rgb(255, 0, 0)') - } else { - // TODO: replace this after fix - // should be red, but is gray now. - expect(buttonBgColor).toBe('rgb(239, 239, 239)') - expect(buttonBgColor).not.toBe('rgb(255, 0, 0)') - } + expect(buttonBgColor).toBe('rgb(255, 0, 0)') }) }) diff --git a/test/production/dynamic-css-client-navigation/react-lazy.test.ts b/test/production/dynamic-css-client-navigation/react-lazy.test.ts index dfb6bfb6cf7a3..8773e14db6e2c 100644 --- a/test/production/dynamic-css-client-navigation/react-lazy.test.ts +++ b/test/production/dynamic-css-client-navigation/react-lazy.test.ts @@ -1,12 +1,10 @@ import { nextTestSetup } from 'e2e-utils' describe('dynamic-css-client-navigation react lazy', () => { - const { next, isTurbopack } = nextTestSetup({ + const { next } = nextTestSetup({ files: __dirname, }) - const isTurbopackBuild = isTurbopack && process.env.TURBOPACK_BUILD - it('should not remove style when navigating from static imported component to react lazy', async () => { const browser = await next.browser('/') expect( @@ -21,15 +19,6 @@ describe('dynamic-css-client-navigation react lazy', () => { `window.getComputedStyle(document.querySelector('button')).backgroundColor` ) - // TODO: remove this condition after fix - if (isTurbopackBuild) { - expect(buttonBgColor).not.toBe('rgb(239, 239, 239)') - expect(buttonBgColor).toBe('rgb(255, 0, 0)') - } else { - // TODO: replace this after fix - // should be red, but is gray now. - expect(buttonBgColor).toBe('rgb(239, 239, 239)') - expect(buttonBgColor).not.toBe('rgb(255, 0, 0)') - } + expect(buttonBgColor).toBe('rgb(255, 0, 0)') }) }) From 0b1fd2ac18fec2548ee284ba54c9f730222c17f7 Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Thu, 21 Nov 2024 21:51:22 +0900 Subject: [PATCH 3/6] pass allDynamicCssFiles by context not __NEXT_DATA__ Co-authored-by: Tobias Koppers --- packages/next/src/client/index.tsx | 26 +------------------ packages/next/src/pages/_document.tsx | 20 +++++++++----- packages/next/src/server/render.tsx | 7 +++-- .../shared/lib/html-context.shared-runtime.ts | 1 + packages/next/src/shared/lib/utils.ts | 1 - 5 files changed, 18 insertions(+), 37 deletions(-) diff --git a/packages/next/src/client/index.tsx b/packages/next/src/client/index.tsx index 48c44969c8c42..42ebd0da9bb09 100644 --- a/packages/next/src/client/index.tsx +++ b/packages/next/src/client/index.tsx @@ -89,9 +89,6 @@ let headManager: { } let initialMatchesMiddleware = false let lastAppProps: AppProps -let dynamicCSSFiles: string[] -let dynamicCSSFilePaths: Set -let publicAssetPath: string let lastRenderReject: (() => void) | null let devClient: any @@ -205,16 +202,10 @@ export async function initialize(opts: { devClient?: any } = {}): Promise<{ window.__NEXT_DATA__ = initialData defaultLocale = initialData.defaultLocale - dynamicCSSFiles = initialData.dynamicCSSFiles || [] const prefix: string = initialData.assetPrefix || '' // With dynamic assetPrefix it's no longer possible to set assetPrefix at the build time // So, this is how we do it in the client side at runtime - publicAssetPath = prefix + '/_next/' - ;(self as any).__next_set_public_path__(publicAssetPath) //eslint-disable-line - - dynamicCSSFilePaths = new Set( - dynamicCSSFiles.map((cssFile) => `${publicAssetPath}${cssFile}`) - ) + ;(self as any).__next_set_public_path__(`${prefix}/_next/`) //eslint-disable-line // Initialize next/config with the environment configuration setConfig({ @@ -766,21 +757,6 @@ function doRender(input: RenderRouteInfo): Promise { looseToArray( document.querySelectorAll('link[data-n-p]') ).forEach((el) => { - const href = el.getAttribute('href') - if (!href) { - return - } - // When client-navigating to a path which requires the same CSS file, - // `mini-css-extract-plugin` will skip injecting the stylesheet as the - // link tag with expected href is still in the DOM. Therefore we remove - // the `data-n-p` attribute and let the stylesheet remain. This is possible - // because we do not "unload" the dynamic stylesheets during the client - // navigation already. Therefore the result will be the same with removing - // the current stylesheet and then dynamically loading the same stylesheet. - if (dynamicCSSFilePaths.has(href)) { - el.removeAttribute('data-n-p') - return - } el.parentNode!.removeChild(el) }) } diff --git a/packages/next/src/pages/_document.tsx b/packages/next/src/pages/_document.tsx index 531903b03e10f..a28d07eb2ddd6 100644 --- a/packages/next/src/pages/_document.tsx +++ b/packages/next/src/pages/_document.tsx @@ -429,6 +429,7 @@ export class Head extends React.Component { assetPrefix, assetQueryString, dynamicImports, + allDynamicCssFiles, crossOrigin, optimizeCss, } = this.context @@ -438,21 +439,23 @@ export class Head extends React.Component { // Unmanaged files are CSS files that will be handled directly by the // webpack runtime (`mini-css-extract-plugin`). let unmanagedFiles: Set = new Set([]) - let dynamicCssFiles = Array.from( + let localDynamicCssFiles = Array.from( new Set(dynamicImports.filter((file) => file.endsWith('.css'))) ) - if (dynamicCssFiles.length) { + if (localDynamicCssFiles.length) { const existing = new Set(cssFiles) - dynamicCssFiles = dynamicCssFiles.filter( + localDynamicCssFiles = localDynamicCssFiles.filter( (f) => !(existing.has(f) || sharedFiles.has(f)) ) - unmanagedFiles = new Set(dynamicCssFiles) - cssFiles.push(...dynamicCssFiles) + unmanagedFiles = new Set(localDynamicCssFiles) + cssFiles.push(...localDynamicCssFiles) } let cssLinkElements: JSX.Element[] = [] cssFiles.forEach((file) => { const isSharedFile = sharedFiles.has(file) + const isUnmanagedFile = unmanagedFiles.has(file) + const isFileInDynamicCssFiles = allDynamicCssFiles.has(file) if (!optimizeCss) { cssLinkElements.push( @@ -469,7 +472,6 @@ export class Head extends React.Component { ) } - const isUnmanagedFile = unmanagedFiles.has(file) cssLinkElements.push( { )}${assetQueryString}`} crossOrigin={this.props.crossOrigin || crossOrigin} data-n-g={isUnmanagedFile ? undefined : isSharedFile ? '' : undefined} - data-n-p={isUnmanagedFile ? undefined : isSharedFile ? undefined : ''} + data-n-p={ + isSharedFile || isUnmanagedFile || isFileInDynamicCssFiles + ? undefined + : '' + } /> ) }) diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index 0b50230666069..f51534f99da04 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -1416,12 +1416,12 @@ export async function renderToHTMLImpl( } } - const dynamicCSSFiles = new Set() + const allDynamicCssFiles = new Set() Object.values(reactLoadableManifest).forEach(({ files }) => { if (files) { files.forEach((file) => { if (file.endsWith('.css')) { - dynamicCSSFiles.add(file) + allDynamicCssFiles.add(file) } }) } @@ -1457,8 +1457,6 @@ export async function renderToHTMLImpl( dynamicImportsIds.size === 0 ? undefined : Array.from(dynamicImportsIds), - dynamicCSSFiles: - dynamicCSSFiles.size > 0 ? Array.from(dynamicCSSFiles) : undefined, err: renderOpts.err ? serializeError(dev, renderOpts.err) : undefined, // Error if one happened, otherwise don't sent in the resulting HTML gsp: !!getStaticProps ? true : undefined, // whether the page is getStaticProps gssp: !!getServerSideProps ? true : undefined, // whether the page is getServerSideProps @@ -1485,6 +1483,7 @@ export async function renderToHTMLImpl( isDevelopment: !!dev, hybridAmp, dynamicImports: Array.from(dynamicImports), + allDynamicCssFiles, assetPrefix, // Only enabled in production as development mode has features relying on HMR (style injection for example) unstable_runtimeJS: diff --git a/packages/next/src/shared/lib/html-context.shared-runtime.ts b/packages/next/src/shared/lib/html-context.shared-runtime.ts index 2fa019f6e836c..4e0384b9faad9 100644 --- a/packages/next/src/shared/lib/html-context.shared-runtime.ts +++ b/packages/next/src/shared/lib/html-context.shared-runtime.ts @@ -22,6 +22,7 @@ export type HtmlProps = { hybridAmp: boolean isDevelopment: boolean dynamicImports: string[] + allDynamicCssFiles: Set assetPrefix?: string canonicalBase: string headTags: any[] diff --git a/packages/next/src/shared/lib/utils.ts b/packages/next/src/shared/lib/utils.ts index d0f63dfb57542..ff1955ab2456c 100644 --- a/packages/next/src/shared/lib/utils.ts +++ b/packages/next/src/shared/lib/utils.ts @@ -97,7 +97,6 @@ export type NEXT_DATA = { isFallback?: boolean isExperimentalCompile?: boolean dynamicIds?: (string | number)[] - dynamicCSSFiles?: string[] err?: Error & { statusCode?: number source?: typeof COMPILER_NAMES.server | typeof COMPILER_NAMES.edgeServer From fe215f1b2085b72509eb378519d5788a8f6db9b7 Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Wed, 27 Nov 2024 20:25:59 +0900 Subject: [PATCH 4/6] test: add nodejs and edge --- .../dynamic-import/dynamic-import.test.ts | 41 ++++++----- .../src/pages/{ => edge}/dynamic-import.tsx | 2 +- .../dynamic-import/src/pages/edge/index.tsx | 17 +++++ .../src/pages/nodejs/dynamic-import.tsx | 6 ++ .../src/pages/{ => nodejs}/index.tsx | 6 +- .../next-dynamic.test.ts | 69 ++++++++++--------- .../react-lazy.test.ts | 41 ++++++----- .../src/pages/edge/index.tsx | 19 +++++ .../{ => edge}/next-dynamic-ssr-false.tsx | 3 +- .../src/pages/{ => edge}/next-dynamic.tsx | 2 +- .../src/pages/{ => edge}/react-lazy.tsx | 2 +- .../src/pages/index.tsx | 15 ---- .../src/pages/nodejs/index.tsx | 17 +++++ .../pages/nodejs/next-dynamic-ssr-false.tsx | 12 ++++ .../src/pages/nodejs/next-dynamic.tsx | 10 +++ .../src/pages/nodejs/react-lazy.tsx | 15 ++++ 16 files changed, 184 insertions(+), 93 deletions(-) rename test/production/dynamic-css-client-navigation/dynamic-import/src/pages/{ => edge}/dynamic-import.tsx (60%) create mode 100644 test/production/dynamic-css-client-navigation/dynamic-import/src/pages/edge/index.tsx create mode 100644 test/production/dynamic-css-client-navigation/dynamic-import/src/pages/nodejs/dynamic-import.tsx rename test/production/dynamic-css-client-navigation/dynamic-import/src/pages/{ => nodejs}/index.tsx (55%) create mode 100644 test/production/dynamic-css-client-navigation/src/pages/edge/index.tsx rename test/production/dynamic-css-client-navigation/src/pages/{ => edge}/next-dynamic-ssr-false.tsx (69%) rename test/production/dynamic-css-client-navigation/src/pages/{ => edge}/next-dynamic.tsx (71%) rename test/production/dynamic-css-client-navigation/src/pages/{ => edge}/react-lazy.tsx (81%) delete mode 100644 test/production/dynamic-css-client-navigation/src/pages/index.tsx create mode 100644 test/production/dynamic-css-client-navigation/src/pages/nodejs/index.tsx create mode 100644 test/production/dynamic-css-client-navigation/src/pages/nodejs/next-dynamic-ssr-false.tsx create mode 100644 test/production/dynamic-css-client-navigation/src/pages/nodejs/next-dynamic.tsx create mode 100644 test/production/dynamic-css-client-navigation/src/pages/nodejs/react-lazy.tsx diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts b/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts index 320a5cad11eba..538f7ece599d0 100644 --- a/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts +++ b/test/production/dynamic-css-client-navigation/dynamic-import/dynamic-import.test.ts @@ -1,24 +1,27 @@ import { nextTestSetup } from 'e2e-utils' -describe('dynamic-css-client-navigation dynamic import', () => { - const { next } = nextTestSetup({ - files: __dirname, - }) +describe.each(['edge', 'nodejs'])( + 'dynamic-css-client-navigation dynamic import %s', + (runtime) => { + const { next } = nextTestSetup({ + files: __dirname, + }) - it('should not remove style when navigating from static imported component to dynamic import', async () => { - const browser = await next.browser('/') - expect( - await browser - .elementByCss('a[href="/dynamic-import"]') - .click() - .waitForElementByCss('#red-button') - .text() - ).toBe('Red Button') + it(`should not remove style when navigating from static imported component to dynamic import at runtime ${runtime}`, async () => { + const browser = await next.browser(`/${runtime}`) + expect( + await browser + .elementByCss(`a[href="/${runtime}/dynamic-import"]`) + .click() + .waitForElementByCss('#red-button') + .text() + ).toBe('Red Button') - const buttonBgColor = await browser.eval( - `window.getComputedStyle(document.querySelector('button')).backgroundColor` - ) + const buttonBgColor = await browser.eval( + `window.getComputedStyle(document.querySelector('button')).backgroundColor` + ) - expect(buttonBgColor).toBe('rgb(255, 0, 0)') - }) -}) + expect(buttonBgColor).toBe('rgb(255, 0, 0)') + }) + } +) diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/dynamic-import.tsx b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/edge/dynamic-import.tsx similarity index 60% rename from test/production/dynamic-css-client-navigation/dynamic-import/src/pages/dynamic-import.tsx rename to test/production/dynamic-css-client-navigation/dynamic-import/src/pages/edge/dynamic-import.tsx index 12cf5867e938a..23022fb1f19dc 100644 --- a/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/dynamic-import.tsx +++ b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/edge/dynamic-import.tsx @@ -1,5 +1,5 @@ import React from 'react' -import { RedButtonLazy } from '../components/red-button-lazy' +import { RedButtonLazy } from '../../components/red-button-lazy' export default function DynamicImport() { return diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/edge/index.tsx b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/edge/index.tsx new file mode 100644 index 0000000000000..69e19b1cbd7d2 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/edge/index.tsx @@ -0,0 +1,17 @@ +import React from 'react' +import Link from 'next/link' +import { RedButton } from '../../components/red-button' +import { RedButtonLazy } from '../../components/red-button-lazy' + +export default function Home() { + return ( + <> + {/* To reproduce, RedButton and RedButtonLazy should be imported. */} + + + /edge/dynamic-import + + ) +} + +export const runtime = 'experimental-edge' diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/nodejs/dynamic-import.tsx b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/nodejs/dynamic-import.tsx new file mode 100644 index 0000000000000..23022fb1f19dc --- /dev/null +++ b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/nodejs/dynamic-import.tsx @@ -0,0 +1,6 @@ +import React from 'react' +import { RedButtonLazy } from '../../components/red-button-lazy' + +export default function DynamicImport() { + return +} diff --git a/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/index.tsx b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/nodejs/index.tsx similarity index 55% rename from test/production/dynamic-css-client-navigation/dynamic-import/src/pages/index.tsx rename to test/production/dynamic-css-client-navigation/dynamic-import/src/pages/nodejs/index.tsx index 5cf38549d8299..fcb62b225836b 100644 --- a/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/index.tsx +++ b/test/production/dynamic-css-client-navigation/dynamic-import/src/pages/nodejs/index.tsx @@ -1,7 +1,7 @@ import React from 'react' import Link from 'next/link' -import { RedButton } from '../components/red-button' -import { RedButtonLazy } from '../components/red-button-lazy' +import { RedButton } from '../../components/red-button' +import { RedButtonLazy } from '../../components/red-button-lazy' export default function Home() { return ( @@ -9,7 +9,7 @@ export default function Home() { {/* To reproduce, RedButton and RedButtonLazy should be imported. */} - /dynamic-import + /nodejs/dynamic-import ) } diff --git a/test/production/dynamic-css-client-navigation/next-dynamic.test.ts b/test/production/dynamic-css-client-navigation/next-dynamic.test.ts index ddc4f811d4422..f7829244f427e 100644 --- a/test/production/dynamic-css-client-navigation/next-dynamic.test.ts +++ b/test/production/dynamic-css-client-navigation/next-dynamic.test.ts @@ -1,41 +1,44 @@ import { nextTestSetup } from 'e2e-utils' -describe('dynamic-css-client-navigation next/dynamic', () => { - const { next } = nextTestSetup({ - files: __dirname, - }) +describe.each(['edge', 'nodejs'])( + 'dynamic-css-client-navigation next/dynamic %s', + (runtime) => { + const { next } = nextTestSetup({ + files: __dirname, + }) - it('should not remove style when navigating from static imported component to next/dynamic', async () => { - const browser = await next.browser('/') - expect( - await browser - .elementByCss('a[href="/next-dynamic"]') - .click() - .waitForElementByCss('#red-button') - .text() - ).toBe('Red Button') + it(`should not remove style when navigating from static imported component to next/dynamic at runtime ${runtime}`, async () => { + const browser = await next.browser(`/${runtime}`) + expect( + await browser + .elementByCss(`a[href="/${runtime}/next-dynamic"]`) + .click() + .waitForElementByCss('#red-button') + .text() + ).toBe('Red Button') - const buttonBgColor = await browser.eval( - `window.getComputedStyle(document.querySelector('button')).backgroundColor` - ) + const buttonBgColor = await browser.eval( + `window.getComputedStyle(document.querySelector('button')).backgroundColor` + ) - expect(buttonBgColor).toBe('rgb(255, 0, 0)') - }) + expect(buttonBgColor).toBe('rgb(255, 0, 0)') + }) - it('should not remove style when navigating from static imported component to next/dynamic with ssr: false', async () => { - const browser = await next.browser('/') - expect( - await browser - .elementByCss('a[href="/next-dynamic-ssr-false"]') - .click() - .waitForElementByCss('#red-button') - .text() - ).toBe('Red Button') + it(`should not remove style when navigating from static imported component to next/dynamic with ssr: false at runtime ${runtime}`, async () => { + const browser = await next.browser(`/${runtime}`) + expect( + await browser + .elementByCss(`a[href="/${runtime}/next-dynamic-ssr-false"]`) + .click() + .waitForElementByCss('#red-button') + .text() + ).toBe('Red Button') - const buttonBgColor = await browser.eval( - `window.getComputedStyle(document.querySelector('button')).backgroundColor` - ) + const buttonBgColor = await browser.eval( + `window.getComputedStyle(document.querySelector('button')).backgroundColor` + ) - expect(buttonBgColor).toBe('rgb(255, 0, 0)') - }) -}) + expect(buttonBgColor).toBe('rgb(255, 0, 0)') + }) + } +) diff --git a/test/production/dynamic-css-client-navigation/react-lazy.test.ts b/test/production/dynamic-css-client-navigation/react-lazy.test.ts index 8773e14db6e2c..7a85018071826 100644 --- a/test/production/dynamic-css-client-navigation/react-lazy.test.ts +++ b/test/production/dynamic-css-client-navigation/react-lazy.test.ts @@ -1,24 +1,27 @@ import { nextTestSetup } from 'e2e-utils' -describe('dynamic-css-client-navigation react lazy', () => { - const { next } = nextTestSetup({ - files: __dirname, - }) +describe.each(['edge', 'nodejs'])( + 'dynamic-css-client-navigation react lazy %s', + (runtime) => { + const { next } = nextTestSetup({ + files: __dirname, + }) - it('should not remove style when navigating from static imported component to react lazy', async () => { - const browser = await next.browser('/') - expect( - await browser - .elementByCss('a[href="/react-lazy"]') - .click() - .waitForElementByCss('#red-button') - .text() - ).toBe('Red Button') + it(`should not remove style when navigating from static imported component to react lazy at runtime ${runtime}`, async () => { + const browser = await next.browser(`/${runtime}`) + expect( + await browser + .elementByCss(`a[href="/${runtime}/react-lazy"]`) + .click() + .waitForElementByCss('#red-button') + .text() + ).toBe('Red Button') - const buttonBgColor = await browser.eval( - `window.getComputedStyle(document.querySelector('button')).backgroundColor` - ) + const buttonBgColor = await browser.eval( + `window.getComputedStyle(document.querySelector('button')).backgroundColor` + ) - expect(buttonBgColor).toBe('rgb(255, 0, 0)') - }) -}) + expect(buttonBgColor).toBe('rgb(255, 0, 0)') + }) + } +) diff --git a/test/production/dynamic-css-client-navigation/src/pages/edge/index.tsx b/test/production/dynamic-css-client-navigation/src/pages/edge/index.tsx new file mode 100644 index 0000000000000..c0b86ed8e35d4 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/pages/edge/index.tsx @@ -0,0 +1,19 @@ +import React from 'react' +import Link from 'next/link' +import { RedButton } from '../../components/red-button' + +export default function Home() { + return ( + <> + {/* To reproduce, RedButton should be imported. */} + + /edge/next-dynamic + + /edge/next-dynamic-ssr-false + + /edge/react-lazy + + ) +} + +export const runtime = 'experimental-edge' diff --git a/test/production/dynamic-css-client-navigation/src/pages/next-dynamic-ssr-false.tsx b/test/production/dynamic-css-client-navigation/src/pages/edge/next-dynamic-ssr-false.tsx similarity index 69% rename from test/production/dynamic-css-client-navigation/src/pages/next-dynamic-ssr-false.tsx rename to test/production/dynamic-css-client-navigation/src/pages/edge/next-dynamic-ssr-false.tsx index b4ec3bfac8291..176bd6d7ecfcb 100644 --- a/test/production/dynamic-css-client-navigation/src/pages/next-dynamic-ssr-false.tsx +++ b/test/production/dynamic-css-client-navigation/src/pages/edge/next-dynamic-ssr-false.tsx @@ -2,7 +2,8 @@ import React from 'react' import dynamic from 'next/dynamic' const NextDynamicRedButton = dynamic( - () => import('../components/red-button').then((module) => module.RedButton), + () => + import('../../components/red-button').then((module) => module.RedButton), { ssr: false } ) diff --git a/test/production/dynamic-css-client-navigation/src/pages/next-dynamic.tsx b/test/production/dynamic-css-client-navigation/src/pages/edge/next-dynamic.tsx similarity index 71% rename from test/production/dynamic-css-client-navigation/src/pages/next-dynamic.tsx rename to test/production/dynamic-css-client-navigation/src/pages/edge/next-dynamic.tsx index 4ae4907490502..887b325f3772b 100644 --- a/test/production/dynamic-css-client-navigation/src/pages/next-dynamic.tsx +++ b/test/production/dynamic-css-client-navigation/src/pages/edge/next-dynamic.tsx @@ -2,7 +2,7 @@ import React from 'react' import dynamic from 'next/dynamic' const NextDynamicRedButton = dynamic(() => - import('../components/red-button').then((module) => module.RedButton) + import('../../components/red-button').then((module) => module.RedButton) ) export default function NextDynamic() { diff --git a/test/production/dynamic-css-client-navigation/src/pages/react-lazy.tsx b/test/production/dynamic-css-client-navigation/src/pages/edge/react-lazy.tsx similarity index 81% rename from test/production/dynamic-css-client-navigation/src/pages/react-lazy.tsx rename to test/production/dynamic-css-client-navigation/src/pages/edge/react-lazy.tsx index b920554e40801..22dabab19276e 100644 --- a/test/production/dynamic-css-client-navigation/src/pages/react-lazy.tsx +++ b/test/production/dynamic-css-client-navigation/src/pages/edge/react-lazy.tsx @@ -1,7 +1,7 @@ import React, { lazy, Suspense } from 'react' const ReactLazyRedButton = lazy(() => - import('../components/red-button').then((module) => ({ + import('../../components/red-button').then((module) => ({ default: module.RedButton, })) ) diff --git a/test/production/dynamic-css-client-navigation/src/pages/index.tsx b/test/production/dynamic-css-client-navigation/src/pages/index.tsx deleted file mode 100644 index 854254ad65722..0000000000000 --- a/test/production/dynamic-css-client-navigation/src/pages/index.tsx +++ /dev/null @@ -1,15 +0,0 @@ -import React from 'react' -import Link from 'next/link' -import { RedButton } from '../components/red-button' - -export default function Home() { - return ( - <> - {/* To reproduce, RedButton should be imported. */} - - /next-dynamic - /next-dynamic-ssr-false - /react-lazy - - ) -} diff --git a/test/production/dynamic-css-client-navigation/src/pages/nodejs/index.tsx b/test/production/dynamic-css-client-navigation/src/pages/nodejs/index.tsx new file mode 100644 index 0000000000000..c36654820c3c7 --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/pages/nodejs/index.tsx @@ -0,0 +1,17 @@ +import React from 'react' +import Link from 'next/link' +import { RedButton } from '../../components/red-button' + +export default function Home() { + return ( + <> + {/* To reproduce, RedButton should be imported. */} + + /nodejs/next-dynamic + + /nodejs/next-dynamic-ssr-false + + /nodejs/react-lazy + + ) +} diff --git a/test/production/dynamic-css-client-navigation/src/pages/nodejs/next-dynamic-ssr-false.tsx b/test/production/dynamic-css-client-navigation/src/pages/nodejs/next-dynamic-ssr-false.tsx new file mode 100644 index 0000000000000..176bd6d7ecfcb --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/pages/nodejs/next-dynamic-ssr-false.tsx @@ -0,0 +1,12 @@ +import React from 'react' +import dynamic from 'next/dynamic' + +const NextDynamicRedButton = dynamic( + () => + import('../../components/red-button').then((module) => module.RedButton), + { ssr: false } +) + +export default function NextDynamic() { + return +} diff --git a/test/production/dynamic-css-client-navigation/src/pages/nodejs/next-dynamic.tsx b/test/production/dynamic-css-client-navigation/src/pages/nodejs/next-dynamic.tsx new file mode 100644 index 0000000000000..887b325f3772b --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/pages/nodejs/next-dynamic.tsx @@ -0,0 +1,10 @@ +import React from 'react' +import dynamic from 'next/dynamic' + +const NextDynamicRedButton = dynamic(() => + import('../../components/red-button').then((module) => module.RedButton) +) + +export default function NextDynamic() { + return +} diff --git a/test/production/dynamic-css-client-navigation/src/pages/nodejs/react-lazy.tsx b/test/production/dynamic-css-client-navigation/src/pages/nodejs/react-lazy.tsx new file mode 100644 index 0000000000000..22dabab19276e --- /dev/null +++ b/test/production/dynamic-css-client-navigation/src/pages/nodejs/react-lazy.tsx @@ -0,0 +1,15 @@ +import React, { lazy, Suspense } from 'react' + +const ReactLazyRedButton = lazy(() => + import('../../components/red-button').then((module) => ({ + default: module.RedButton, + })) +) + +export default function ReactLazy() { + return ( + + + + ) +} From fc496a4d35cf6e42946d4d68cfd3160d580b6bc7 Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Wed, 27 Nov 2024 23:00:37 +0900 Subject: [PATCH 5/6] use manifest instead of getting dynamic css from loadables on render --- packages/next/src/build/templates/edge-ssr.ts | 2 + .../loaders/next-edge-ssr-loader/render.ts | 8 ++- .../webpack/plugins/middleware-plugin.ts | 11 ++-- .../webpack/plugins/react-loadable-plugin.ts | 61 +++++++++++++++---- packages/next/src/client/route-loader.ts | 1 + packages/next/src/pages/_document.tsx | 6 +- packages/next/src/server/load-components.ts | 16 +++++ packages/next/src/server/render.tsx | 13 +--- packages/next/src/shared/lib/constants.ts | 2 + .../shared/lib/html-context.shared-runtime.ts | 6 +- 10 files changed, 94 insertions(+), 32 deletions(-) diff --git a/packages/next/src/build/templates/edge-ssr.ts b/packages/next/src/build/templates/edge-ssr.ts index 96c0b302244a1..903a6cbef8da6 100644 --- a/packages/next/src/build/templates/edge-ssr.ts +++ b/packages/next/src/build/templates/edge-ssr.ts @@ -89,6 +89,7 @@ const maybeJSONParse = (str?: string) => (str ? JSON.parse(str) : undefined) const buildManifest: BuildManifest = self.__BUILD_MANIFEST as any const reactLoadableManifest = maybeJSONParse(self.__REACT_LOADABLE_MANIFEST) +const dynamicCssManifest = maybeJSONParse(self.__DYNAMIC_CSS_MANIFEST) const subresourceIntegrityManifest = sriEnabled ? maybeJSONParse(self.__SUBRESOURCE_INTEGRITY_MANIFEST) : undefined @@ -106,6 +107,7 @@ const render = getRender({ buildManifest, renderToHTML, reactLoadableManifest, + dynamicCssManifest, subresourceIntegrityManifest, config: nextConfig, buildId: process.env.__NEXT_BUILD_ID!, diff --git a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts index 5d60f000f2148..40d377c9040fb 100644 --- a/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts +++ b/packages/next/src/build/webpack/loaders/next-edge-ssr-loader/render.ts @@ -2,7 +2,10 @@ import type { NextConfigComplete } from '../../../../server/config-shared' import type { DocumentType, AppType } from '../../../../shared/lib/utils' import type { BuildManifest } from '../../../../server/get-page-files' -import type { ReactLoadableManifest } from '../../../../server/load-components' +import type { + DynamicCssManifest, + ReactLoadableManifest, +} from '../../../../server/load-components' import type { ClientReferenceManifest } from '../../plugins/flight-manifest-plugin' import type { NextFontManifest } from '../../plugins/next-font-manifest-plugin' import type { NextFetchEvent } from '../../../../server/web/spec-extension/fetch-event' @@ -31,6 +34,7 @@ export function getRender({ Document, buildManifest, reactLoadableManifest, + dynamicCssManifest, interceptionRouteRewrites, renderToHTML, clientReferenceManifest, @@ -53,6 +57,7 @@ export function getRender({ Document: DocumentType buildManifest: BuildManifest reactLoadableManifest: ReactLoadableManifest + dynamicCssManifest?: DynamicCssManifest subresourceIntegrityManifest?: Record interceptionRouteRewrites?: ManifestRewriteRoute[] clientReferenceManifest?: ClientReferenceManifest @@ -71,6 +76,7 @@ export function getRender({ dev, buildManifest, reactLoadableManifest, + dynamicCssManifest, subresourceIntegrityManifest, Document, App: appMod?.default as AppType, diff --git a/packages/next/src/build/webpack/plugins/middleware-plugin.ts b/packages/next/src/build/webpack/plugins/middleware-plugin.ts index 336220020a02e..46fc77f0ca813 100644 --- a/packages/next/src/build/webpack/plugins/middleware-plugin.ts +++ b/packages/next/src/build/webpack/plugins/middleware-plugin.ts @@ -21,6 +21,7 @@ import { NEXT_FONT_MANIFEST, SERVER_REFERENCE_MANIFEST, INTERCEPTION_ROUTE_REWRITE_MANIFEST, + DYNAMIC_CSS_MANIFEST, } from '../../../shared/lib/constants' import type { MiddlewareConfig } from '../../analysis/get-page-static-info' import type { Telemetry } from '../../../telemetry/storage' @@ -100,9 +101,7 @@ function getEntryFiles( entryFiles: string[], meta: EntryMetadata, hasInstrumentationHook: boolean, - opts: { - sriEnabled: boolean - } + opts: Options ) { const files: string[] = [] if (meta.edgeSSR) { @@ -124,6 +123,9 @@ function getEntryFiles( ) ) } + if (!opts.dev && !meta.edgeSSR.isAppDir) { + files.push(`server/${DYNAMIC_CSS_MANIFEST}.js`) + } files.push( `server/${MIDDLEWARE_BUILD_MANIFEST}.js`, @@ -149,7 +151,7 @@ function getEntryFiles( function getCreateAssets(params: { compilation: webpack.Compilation metadataByEntry: Map - opts: Omit + opts: Options }) { const { compilation, metadataByEntry, opts } = params return (assets: any) => { @@ -810,6 +812,7 @@ export default class MiddlewarePlugin { sriEnabled: this.sriEnabled, rewrites: this.rewrites, edgeEnvironments: this.edgeEnvironments, + dev: this.dev, }, }) ) diff --git a/packages/next/src/build/webpack/plugins/react-loadable-plugin.ts b/packages/next/src/build/webpack/plugins/react-loadable-plugin.ts index 83c814a2b5933..40dd3d06ae693 100644 --- a/packages/next/src/build/webpack/plugins/react-loadable-plugin.ts +++ b/packages/next/src/build/webpack/plugins/react-loadable-plugin.ts @@ -21,9 +21,13 @@ WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWAR // Implementation of this PR: https://github.com/jamiebuilds/react-loadable/pull/132 // Modified to strip out unneeded results for Next's specific use case -import { webpack, sources } from 'next/dist/compiled/webpack/webpack' - +import type { + DynamicCssManifest, + ReactLoadableManifest, +} from '../../../server/load-components' import path from 'path' +import { webpack, sources } from 'next/dist/compiled/webpack/webpack' +import { DYNAMIC_CSS_MANIFEST } from '../../../shared/lib/constants' function getModuleId(compilation: any, module: any): string | number { return compilation.chunkGraph.getModuleId(module) @@ -54,12 +58,20 @@ function buildManifest( _compiler: webpack.Compiler, compilation: webpack.Compilation, projectSrcDir: string | undefined, - dev: boolean -) { + dev: boolean, + shouldCreateDynamicCssManifest: boolean +): { + reactLoadableManifest: ReactLoadableManifest + dynamicCssManifest: DynamicCssManifest +} { if (!projectSrcDir) { - return {} + return { + reactLoadableManifest: {}, + dynamicCssManifest: [], + } } - let manifest: { [k: string]: { id: string | number; files: string[] } } = {} + const dynamicCssManifestSet = new Set() + let manifest: ReactLoadableManifest = {} // This is allowed: // import("./module"); <- ImportDependency @@ -119,6 +131,10 @@ function buildManifest( file.match(/^static\/(chunks|css)\//) ) { files.add(file) + + if (shouldCreateDynamicCssManifest && file.endsWith('.css')) { + dynamicCssManifestSet.add(file) + } } }) } @@ -143,12 +159,16 @@ function buildManifest( // eslint-disable-next-line no-sequences .reduce((a, c) => ((a[c] = manifest[c]), a), {} as any) - return manifest + return { + reactLoadableManifest: manifest, + dynamicCssManifest: Array.from(dynamicCssManifestSet), + } } export class ReactLoadablePlugin { private filename: string private pagesOrAppDir: string | undefined + private isPagesDir: boolean private runtimeAsset?: string private dev: boolean @@ -161,6 +181,7 @@ export class ReactLoadablePlugin { }) { this.filename = opts.filename this.pagesOrAppDir = opts.pagesDir || opts.appDir + this.isPagesDir = Boolean(opts.pagesDir) this.runtimeAsset = opts.runtimeAsset this.dev = opts.dev } @@ -169,23 +190,41 @@ export class ReactLoadablePlugin { const projectSrcDir = this.pagesOrAppDir ? path.dirname(this.pagesOrAppDir) : undefined - const manifest = buildManifest( + const shouldCreateDynamicCssManifest = !this.dev && this.isPagesDir + const { reactLoadableManifest, dynamicCssManifest } = buildManifest( compiler, compilation, projectSrcDir, - this.dev + this.dev, + shouldCreateDynamicCssManifest ) assets[this.filename] = new sources.RawSource( - JSON.stringify(manifest, null, 2) + JSON.stringify(reactLoadableManifest, null, 2) ) if (this.runtimeAsset) { assets[this.runtimeAsset] = new sources.RawSource( `self.__REACT_LOADABLE_MANIFEST=${JSON.stringify( - JSON.stringify(manifest) + JSON.stringify(reactLoadableManifest) )}` ) } + + // This manifest prevents removing server rendered tags after client + // navigation. This is only needed under Pages dir && Production && Webpack. + // x-ref: https://github.com/vercel/next.js/pull/72959 + if (shouldCreateDynamicCssManifest) { + assets[`${DYNAMIC_CSS_MANIFEST}.json`] = new sources.RawSource( + JSON.stringify(dynamicCssManifest, null, 2) + ) + // This is for edge runtime. + assets[`server/${DYNAMIC_CSS_MANIFEST}.js`] = new sources.RawSource( + `self.__DYNAMIC_CSS_MANIFEST=${JSON.stringify( + JSON.stringify(dynamicCssManifest) + )}` + ) + } + return assets } diff --git a/packages/next/src/client/route-loader.ts b/packages/next/src/client/route-loader.ts index 29fa13b272019..2ab34022060a5 100644 --- a/packages/next/src/client/route-loader.ts +++ b/packages/next/src/client/route-loader.ts @@ -19,6 +19,7 @@ declare global { __MIDDLEWARE_MATCHERS?: MiddlewareMatcher[] __MIDDLEWARE_MANIFEST_CB?: Function __REACT_LOADABLE_MANIFEST?: any + __DYNAMIC_CSS_MANIFEST?: any __RSC_MANIFEST?: any __RSC_SERVER_MANIFEST?: any __NEXT_FONT_MANIFEST?: any diff --git a/packages/next/src/pages/_document.tsx b/packages/next/src/pages/_document.tsx index a28d07eb2ddd6..8a593a29d5b1f 100644 --- a/packages/next/src/pages/_document.tsx +++ b/packages/next/src/pages/_document.tsx @@ -429,7 +429,7 @@ export class Head extends React.Component { assetPrefix, assetQueryString, dynamicImports, - allDynamicCssFiles, + dynamicCssManifest, crossOrigin, optimizeCss, } = this.context @@ -455,7 +455,7 @@ export class Head extends React.Component { cssFiles.forEach((file) => { const isSharedFile = sharedFiles.has(file) const isUnmanagedFile = unmanagedFiles.has(file) - const isFileInDynamicCssFiles = allDynamicCssFiles.has(file) + const isFileInDynamicCssManifest = dynamicCssManifest.has(file) if (!optimizeCss) { cssLinkElements.push( @@ -483,7 +483,7 @@ export class Head extends React.Component { crossOrigin={this.props.crossOrigin || crossOrigin} data-n-g={isUnmanagedFile ? undefined : isSharedFile ? '' : undefined} data-n-p={ - isSharedFile || isUnmanagedFile || isFileInDynamicCssFiles + isSharedFile || isUnmanagedFile || isFileInDynamicCssManifest ? undefined : '' } diff --git a/packages/next/src/server/load-components.ts b/packages/next/src/server/load-components.ts index 907d68f3b85b5..e76e806a50291 100644 --- a/packages/next/src/server/load-components.ts +++ b/packages/next/src/server/load-components.ts @@ -19,6 +19,7 @@ import { REACT_LOADABLE_MANIFEST, CLIENT_REFERENCE_MANIFEST, SERVER_REFERENCE_MANIFEST, + DYNAMIC_CSS_MANIFEST, } from '../shared/lib/constants' import { join } from 'path' import { requirePage } from './require' @@ -38,6 +39,12 @@ export type ManifestItem = { } export type ReactLoadableManifest = { [moduleId: string]: ManifestItem } +/** + * This manifest prevents removing server rendered tags after client + * navigation. This is only needed under `Pages dir && Production && Webpack`. + * @see https://github.com/vercel/next.js/pull/72959 + */ +export type DynamicCssManifest = string[] /** * A manifest entry type for the react-loadable-manifest.json. @@ -56,6 +63,7 @@ export type LoadComponentsReturnType = { buildManifest: DeepReadonly subresourceIntegrityManifest?: DeepReadonly> reactLoadableManifest: DeepReadonly + dynamicCssManifest?: DeepReadonly clientReferenceManifest?: DeepReadonly serverActionsManifest?: any Document: DocumentType @@ -147,6 +155,7 @@ async function loadComponentsImpl({ const [ buildManifest, reactLoadableManifest, + dynamicCssManifest, clientReferenceManifest, serverActionsManifest, ] = await Promise.all([ @@ -154,6 +163,12 @@ async function loadComponentsImpl({ loadManifestWithRetries( join(distDir, REACT_LOADABLE_MANIFEST) ), + // This manifest will only exist in Pages dir && Production && Webpack. + isAppPath || process.env.TURBOPACK + ? undefined + : loadManifestWithRetries( + join(distDir, `${DYNAMIC_CSS_MANIFEST}.json`) + ).catch(() => undefined), hasClientManifest ? loadClientReferenceManifest( join( @@ -201,6 +216,7 @@ async function loadComponentsImpl({ Component, buildManifest, reactLoadableManifest, + dynamicCssManifest, pageConfig: ComponentMod.config || {}, ComponentMod, getServerSideProps, diff --git a/packages/next/src/server/render.tsx b/packages/next/src/server/render.tsx index f51534f99da04..b2022d743f53d 100644 --- a/packages/next/src/server/render.tsx +++ b/packages/next/src/server/render.tsx @@ -1416,17 +1416,6 @@ export async function renderToHTMLImpl( } } - const allDynamicCssFiles = new Set() - Object.values(reactLoadableManifest).forEach(({ files }) => { - if (files) { - files.forEach((file) => { - if (file.endsWith('.css')) { - allDynamicCssFiles.add(file) - } - }) - } - }) - const hybridAmp = ampState.hybrid const docComponentsRendered: DocumentProps['docComponentsRendered'] = {} @@ -1483,7 +1472,7 @@ export async function renderToHTMLImpl( isDevelopment: !!dev, hybridAmp, dynamicImports: Array.from(dynamicImports), - allDynamicCssFiles, + dynamicCssManifest: new Set(renderOpts.dynamicCssManifest || []), assetPrefix, // Only enabled in production as development mode has features relying on HMR (style injection for example) unstable_runtimeJS: diff --git a/packages/next/src/shared/lib/constants.ts b/packages/next/src/shared/lib/constants.ts index c1b6926434e60..82f1b064100c1 100644 --- a/packages/next/src/shared/lib/constants.ts +++ b/packages/next/src/shared/lib/constants.ts @@ -75,6 +75,8 @@ export const MIDDLEWARE_REACT_LOADABLE_MANIFEST = // server/interception-route-rewrite-manifest.js export const INTERCEPTION_ROUTE_REWRITE_MANIFEST = 'interception-route-rewrite-manifest' +// server/dynamic-css-manifest.js +export const DYNAMIC_CSS_MANIFEST = 'dynamic-css-manifest' // static/runtime/main.js export const CLIENT_STATIC_FILES_RUNTIME_MAIN = `main` diff --git a/packages/next/src/shared/lib/html-context.shared-runtime.ts b/packages/next/src/shared/lib/html-context.shared-runtime.ts index 4e0384b9faad9..ad12c02db6b4a 100644 --- a/packages/next/src/shared/lib/html-context.shared-runtime.ts +++ b/packages/next/src/shared/lib/html-context.shared-runtime.ts @@ -22,7 +22,11 @@ export type HtmlProps = { hybridAmp: boolean isDevelopment: boolean dynamicImports: string[] - allDynamicCssFiles: Set + /** + * This manifest is only needed for Pages dir, Production, Webpack + * @see https://github.com/vercel/next.js/pull/72959 + */ + dynamicCssManifest: Set assetPrefix?: string canonicalBase: string headTags: any[] From e8a8d12a88965602915410caf551990bc0c2067f Mon Sep 17 00:00:00 2001 From: devjiwonchoi Date: Fri, 29 Nov 2024 13:44:46 +0900 Subject: [PATCH 6/6] add manifest for standalone mode Co-authored-by: JJ Kasper --- packages/next/src/build/index.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index c5446fdd5b7c5..2dfa80dacd8ac 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -79,6 +79,7 @@ import { FUNCTIONS_CONFIG_MANIFEST, UNDERSCORE_NOT_FOUND_ROUTE_ENTRY, UNDERSCORE_NOT_FOUND_ROUTE, + DYNAMIC_CSS_MANIFEST, } from '../shared/lib/constants' import { getSortedRoutes, @@ -2613,6 +2614,12 @@ export default async function build( ), ] : []), + ...(pagesDir && !turboNextBuild + ? [ + DYNAMIC_CSS_MANIFEST + '.json', + path.join(SERVER_DIRECTORY, DYNAMIC_CSS_MANIFEST + '.js'), + ] + : []), REACT_LOADABLE_MANIFEST, BUILD_ID_FILE, path.join(SERVER_DIRECTORY, NEXT_FONT_MANIFEST + '.js'),