From d11be829392aa2b7f798456181f00cc5ebc54686 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Fri, 4 Nov 2022 01:03:54 +0100 Subject: [PATCH 1/9] wip: colocate styles with special entries --- .../build/webpack/loaders/next-app-loader.ts | 3 +- .../plugins/flight-client-entry-plugin.ts | 40 ++++++------ .../next/client/components/error-boundary.tsx | 18 +++-- .../next/client/components/layout-router.tsx | 4 +- packages/next/server/app-render.tsx | 65 +++++++++++++++++-- .../app/app/error/client-component/error.js | 4 +- .../error/client-component/style.module.css | 3 + .../app/loading-bug/[categorySlug]/loading.js | 2 + .../app/loading-bug/[categorySlug]/style.css | 3 + .../template/clientcomponent/style.module.css | 3 + .../app/template/clientcomponent/template.js | 5 +- .../template/servercomponent/style.module.css | 3 + .../app/template/servercomponent/template.js | 4 +- 13 files changed, 122 insertions(+), 35 deletions(-) create mode 100644 test/e2e/app-dir/app/app/error/client-component/style.module.css create mode 100644 test/e2e/app-dir/app/app/loading-bug/[categorySlug]/style.css create mode 100644 test/e2e/app-dir/app/app/template/clientcomponent/style.module.css create mode 100644 test/e2e/app-dir/app/app/template/servercomponent/style.module.css diff --git a/packages/next/build/webpack/loaders/next-app-loader.ts b/packages/next/build/webpack/loaders/next-app-loader.ts index d5191c03ac2efd6..fa2f4e05aee0ac3 100644 --- a/packages/next/build/webpack/loaders/next-app-loader.ts +++ b/packages/next/build/webpack/loaders/next-app-loader.ts @@ -25,6 +25,7 @@ export type ComponentsType = { readonly [componentKey in ValueOf]?: ComponentModule } & { readonly layoutOrPagePath?: string + readonly filePath?: string readonly page?: ComponentModule } @@ -108,7 +109,7 @@ async function createTreeCodeFromPath({ return `${ file === FILE_TYPES.layout ? `layoutOrPagePath: ${JSON.stringify(filePath)},` - : '' + : `filePath: ${JSON.stringify(filePath)},` }'${file}': () => require(${JSON.stringify(filePath)}),` }) .join('\n')} diff --git a/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts b/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts index c048c8b9081797b..2ddbcb163343473 100644 --- a/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts +++ b/packages/next/build/webpack/plugins/flight-client-entry-plugin.ts @@ -179,17 +179,18 @@ export class FlightClientEntryPlugin { for (const connection of compilation.moduleGraph.getOutgoingConnections( entryModule )) { - const layoutOrPageDependency = connection.dependency - const layoutOrPageRequest = connection.dependency.request + // Entry can be any user defined entry files such as layout, page, error, loading, etc. + const entryDependency = connection.dependency + const entryRequest = connection.dependency.request const [clientComponentImports] = this.collectClientComponentsAndCSSForDependency({ - layoutOrPageRequest, + entryRequest, compilation, - dependency: layoutOrPageDependency, + dependency: entryDependency, }) - const isAbsoluteRequest = path.isAbsolute(layoutOrPageRequest) + const isAbsoluteRequest = path.isAbsolute(entryRequest) // Next.js internals are put into a separate entry. if (!isAbsoluteRequest) { @@ -200,8 +201,8 @@ export class FlightClientEntryPlugin { } const relativeRequest = isAbsoluteRequest - ? path.relative(compilation.options.context, layoutOrPageRequest) - : layoutOrPageRequest + ? path.relative(compilation.options.context, entryRequest) + : entryRequest // Replace file suffix as `.js` will be added. const bundlePath = relativeRequest.replace(/\.(js|ts)x?$/, '') @@ -308,14 +309,14 @@ export class FlightClientEntryPlugin { for (const connection of compilation.moduleGraph.getOutgoingConnections( entryModule )) { - const layoutOrPageDependency = connection.dependency - const layoutOrPageRequest = connection.dependency.request + const entryDependency = connection.dependency + const entryRequest = connection.dependency.request const [, cssImports] = this.collectClientComponentsAndCSSForDependency({ - layoutOrPageRequest, + entryRequest, compilation, - dependency: layoutOrPageDependency, + dependency: entryDependency, clientEntryDependencyMap, }) @@ -353,12 +354,12 @@ export class FlightClientEntryPlugin { } collectClientComponentsAndCSSForDependency({ - layoutOrPageRequest, + entryRequest, compilation, dependency, clientEntryDependencyMap, }: { - layoutOrPageRequest: string + entryRequest: string compilation: any dependency: any /* Dependency */ clientEntryDependencyMap?: Record @@ -397,15 +398,15 @@ export class FlightClientEntryPlugin { : mod.resourceResolveData?.path + mod.resourceResolveData?.query // Ensure module is not walked again if it's already been visited - if (!visitedBySegment[layoutOrPageRequest]) { - visitedBySegment[layoutOrPageRequest] = new Set() + if (!visitedBySegment[entryRequest]) { + visitedBySegment[entryRequest] = new Set() } const storeKey = (inClientComponentBoundary ? '0' : '1') + ':' + modRequest - if (!modRequest || visitedBySegment[layoutOrPageRequest].has(storeKey)) { + if (!modRequest || visitedBySegment[entryRequest].has(storeKey)) { return } - visitedBySegment[layoutOrPageRequest].add(storeKey) + visitedBySegment[entryRequest].add(storeKey) const isClientComponent = isClientComponentModule(mod) @@ -425,9 +426,8 @@ export class FlightClientEntryPlugin { } } - serverCSSImports[layoutOrPageRequest] = - serverCSSImports[layoutOrPageRequest] || [] - serverCSSImports[layoutOrPageRequest].push(modRequest) + serverCSSImports[entryRequest] = serverCSSImports[entryRequest] || [] + serverCSSImports[entryRequest].push(modRequest) } // Check if request is for css file. diff --git a/packages/next/client/components/error-boundary.tsx b/packages/next/client/components/error-boundary.tsx index 649a7bd73721f67..1fe4bacf53dea05 100644 --- a/packages/next/client/components/error-boundary.tsx +++ b/packages/next/client/components/error-boundary.tsx @@ -6,6 +6,7 @@ export type ErrorComponent = React.ComponentType<{ }> interface ErrorBoundaryProps { errorComponent: ErrorComponent + errorStyles: React.ReactNode | undefined } /** @@ -32,10 +33,13 @@ class ErrorBoundaryHandler extends React.Component< render() { if (this.state.error) { return ( - + <> + {this.props.errorStyles} + + ) } @@ -49,11 +53,15 @@ class ErrorBoundaryHandler extends React.Component< */ export function ErrorBoundary({ errorComponent, + errorStyles, children, }: ErrorBoundaryProps & { children: React.ReactNode }): JSX.Element { if (errorComponent) { return ( - + {children} ) diff --git a/packages/next/client/components/layout-router.tsx b/packages/next/client/components/layout-router.tsx index dc8cb0c5139a0cf..84b16ee800740f8 100644 --- a/packages/next/client/components/layout-router.tsx +++ b/packages/next/client/components/layout-router.tsx @@ -375,6 +375,7 @@ export default function OuterLayoutRouter({ segmentPath, childProp, error, + errorStyles, loading, hasLoading, template, @@ -385,6 +386,7 @@ export default function OuterLayoutRouter({ segmentPath: FlightSegmentPath childProp: ChildProp error: ErrorComponent + errorStyles: React.ReactNode | undefined template: React.ReactNode loading: React.ReactNode | undefined hasLoading: boolean @@ -442,7 +444,7 @@ export default function OuterLayoutRouter({ + diff --git a/packages/next/server/app-render.tsx b/packages/next/server/app-render.tsx index 277420c014b2fc7..c3d95210c8286e3 100644 --- a/packages/next/server/app-render.tsx +++ b/packages/next/server/app-render.tsx @@ -957,6 +957,51 @@ export async function renderToHTMLOrFlight( const assetPrefix = renderOpts.assetPrefix || '' + const createFragmentTypeWithStyles = async ( + filePath?: string, + getter?: () => any, + styleOnly?: boolean + ): Promise => { + if (!filePath || (!getter && !styleOnly)) return + + const styles = getCssInlinedLinkTags( + serverComponentManifest, + serverCSSManifest, + filePath, + serverCSSForEntries + ) + const cacheBustingUrlSuffix = dev ? `?ts=${Date.now()}` : '' + + const styleElements = styles + ? styles.map((href, index) => ( + + )) + : null + + if (styleOnly) { + return styleElements + } + + if (!getter) return + const Comp = await interopDefault(getter()) + + return () => ( + <> + {styleElements} + + + ) + } + /** * Use the provided loader tree to create the React Component tree. */ @@ -966,6 +1011,7 @@ export async function renderToHTMLOrFlight( segment, parallelRoutes, { + filePath, layoutOrPagePath, layout, template, @@ -1002,11 +1048,17 @@ export async function renderToHTMLOrFlight( serverCSSForEntries, layoutOrPagePath ) - const Template = template - ? await interopDefault(template()) - : React.Fragment + + const Template = + (await createFragmentTypeWithStyles(filePath, template)) || + React.Fragment const ErrorComponent = error ? await interopDefault(error()) : undefined - const Loading = loading ? await interopDefault(loading()) : undefined + const errorStyles = + filePath && error + ? await createFragmentTypeWithStyles(filePath, undefined, true) + : undefined + + const Loading = await createFragmentTypeWithStyles(filePath, loading) const isLayout = typeof layout !== 'undefined' const isPage = typeof page !== 'undefined' const layoutOrPageMod = isLayout @@ -1014,6 +1066,7 @@ export async function renderToHTMLOrFlight( : isPage ? await page() : undefined + /** * Checks if the current segment is a root layout. */ @@ -1025,7 +1078,7 @@ export async function renderToHTMLOrFlight( rootLayoutIncluded || rootLayoutAtThisLevel const NotFound = notFound - ? await interopDefault(notFound()) + ? await createFragmentTypeWithStyles(filePath, notFound) : rootLayoutAtThisLevel ? DefaultNotFound : undefined @@ -1141,6 +1194,7 @@ export async function renderToHTMLOrFlight( loading={Loading ? : undefined} hasLoading={Boolean(Loading)} error={ErrorComponent} + errorStyles={errorStyles} template={ } + templateStyles={templateStyles} notFound={NotFound ? : undefined} + notFoundStyles={notFoundStyles} childProp={childProp} rootLayoutIncluded={rootLayoutIncludedAtThisLevelOrAbove} />, @@ -1247,6 +1234,7 @@ export async function renderToHTMLOrFlight( error={ErrorComponent} errorStyles={errorStyles} loading={Loading ? : undefined} + loadingStyles={loadingStyles} // TODO-APP: Add test for loading returning `undefined`. This currently can't be tested as the `webdriver()` tab will wait for the full page to load before returning. hasLoading={Boolean(Loading)} template={ @@ -1254,7 +1242,9 @@ export async function renderToHTMLOrFlight( } + templateStyles={templateStyles} notFound={NotFound ? : undefined} + notFoundStyles={notFoundStyles} childProp={childProp} rootLayoutIncluded={rootLayoutIncludedAtThisLevelOrAbove} />, From 4fbbfe857fa410ef2e476ab38b37cfe6b369f526 Mon Sep 17 00:00:00 2001 From: Shu Ding Date: Tue, 8 Nov 2022 13:30:08 +0100 Subject: [PATCH 9/9] fix test --- test/e2e/app-dir/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index a0fdd18d6e085bc..c81fb2ea0da2543 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1513,7 +1513,7 @@ describe('app dir', () => { const html = await renderViaHTTP(next.url, '/loading-bug/hi') // The link tag should be included together with loading expect(html).toMatch( - /

Loading...<\/h2>/ + /

Loading...<\/h2>/ ) })