From 8b82225fea3a77084da55bef052b85d98f1dbf6d Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 17 Apr 2024 10:18:09 +0200 Subject: [PATCH] Fix ASL bundling for dynamic css (#64451) ### Why For app page rendering on edge, the `AsyncLocalStorage` (ALS) should be bundled as same instance across layers. We're accessing the ALS in `next/dynamic` modules during SSR for preloading CSS chunks. There's a bug that we can't get the ALS store during SSR in edge, I digged into it and found the root cause is: We have both import paths: `module (rsc layer) -> request ALS (shared layer)` `module (ssr layer) -> request ALS (shared layer)` We expect the ALS to be the same module since we're using the same layer but found that they're treated as different modules due to applying another loader transform on ssr layer. They're resulted in the same `shared` layer, but with different resource queries. This PR excluded that transform so now they're identical across layers. ### What For webpack, we aligned the loaders applying to the async local storage, so that they're resolved as the same module now. For turbopack, we leverage module transition, sort of creating a new `app-shared` layer for these modules, and apply the transition to all async local storage instances therefore the instances of them are only bundled once. To make the turbopack chanegs work, we change how the async local storage modules defined, separate the instance into a single file and mark it as "next-shared" layer with import: ``` any module -> async local storage --- use transition, specify "next-shared" layer ---> async local storage instance ``` Closes NEXT-3085 --- packages/next-swc/crates/next-api/src/app.rs | 56 ++++++++++++++++++- packages/next/src/build/webpack-config.ts | 4 +- .../action-async-storage-instance.ts | 4 ++ .../action-async-storage.external.ts | 7 ++- .../request-async-storage-instance.ts | 5 ++ .../request-async-storage.external.ts | 8 ++- ...tatic-generation-async-storage-instance.ts | 5 ++ ...tatic-generation-async-storage.external.ts | 8 ++- packages/next/src/lib/constants.ts | 1 - .../shared/lib/lazy-dynamic/preload-css.tsx | 7 +-- .../app-dir/dynamic-css/app/ssr/edge/page.js | 3 + test/e2e/app-dir/dynamic-css/index.test.ts | 22 ++++++++ 12 files changed, 114 insertions(+), 16 deletions(-) create mode 100644 packages/next/src/client/components/action-async-storage-instance.ts create mode 100644 packages/next/src/client/components/request-async-storage-instance.ts create mode 100644 packages/next/src/client/components/static-generation-async-storage-instance.ts create mode 100644 test/e2e/app-dir/dynamic-css/app/ssr/edge/page.js diff --git a/packages/next-swc/crates/next-api/src/app.rs b/packages/next-swc/crates/next-api/src/app.rs index 9f82537007f13..1fda6368d3aed 100644 --- a/packages/next-swc/crates/next-api/src/app.rs +++ b/packages/next-swc/crates/next-api/src/app.rs @@ -283,6 +283,10 @@ impl AppProject { ))), ), ("next-ssr".to_string(), Vc::upcast(self.ssr_transition())), + ( + "next-shared".to_string(), + Vc::upcast(self.shared_transition()), + ), ] .into_iter() .collect(); @@ -315,6 +319,10 @@ impl AppProject { "next-ssr".to_string(), Vc::upcast(self.edge_ssr_transition()), ), + ( + "next-shared".to_string(), + Vc::upcast(self.edge_shared_transition()), + ), ] .into_iter() .collect(); @@ -344,6 +352,10 @@ impl AppProject { ))), ), ("next-ssr".to_string(), Vc::upcast(self.ssr_transition())), + ( + "next-shared".to_string(), + Vc::upcast(self.shared_transition()), + ), ] .into_iter() .collect(); @@ -359,8 +371,30 @@ impl AppProject { #[turbo_tasks::function] fn edge_route_module_context(self: Vc) -> Vc { + let transitions = [ + ( + ECMASCRIPT_CLIENT_TRANSITION_NAME.to_string(), + Vc::upcast(NextEcmascriptClientReferenceTransition::new( + Vc::upcast(self.client_transition()), + self.edge_ssr_transition(), + )), + ), + ( + "next-dynamic".to_string(), + Vc::upcast(NextDynamicTransition::new(Vc::upcast( + self.client_transition(), + ))), + ), + ("next-ssr".to_string(), Vc::upcast(self.ssr_transition())), + ( + "next-shared".to_string(), + Vc::upcast(self.edge_shared_transition()), + ), + ] + .into_iter() + .collect(); ModuleAssetContext::new( - Default::default(), + Vc::cell(transitions), self.project().edge_compile_time_info(), self.edge_route_module_options_context(), self.edge_route_resolve_options_context(), @@ -435,6 +469,16 @@ impl AppProject { ) } + #[turbo_tasks::function] + fn shared_transition(self: Vc) -> Vc { + ContextTransition::new( + self.project().server_compile_time_info(), + self.ssr_module_options_context(), + self.ssr_resolve_options_context(), + Vc::cell("app-shared".to_string()), + ) + } + #[turbo_tasks::function] fn edge_ssr_transition(self: Vc) -> Vc { ContextTransition::new( @@ -445,6 +489,16 @@ impl AppProject { ) } + #[turbo_tasks::function] + fn edge_shared_transition(self: Vc) -> Vc { + ContextTransition::new( + self.project().edge_compile_time_info(), + self.edge_ssr_module_options_context(), + self.edge_ssr_resolve_options_context(), + Vc::cell("app-edge-shared".to_string()), + ) + } + #[turbo_tasks::function] async fn runtime_entries(self: Vc) -> Result> { Ok(get_server_runtime_entries( diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 0f48f804c567d..9816b5ebb41c9 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -1383,7 +1383,6 @@ export default async function getBaseWebpackConfig( // Alias react for switching between default set and share subset. oneOf: [ { - exclude: asyncStoragesRegex, issuerLayer: isWebpackServerOnlyLayer, test: { // Resolve it if it is a source code file, and it has NOT been @@ -1391,7 +1390,7 @@ export default async function getBaseWebpackConfig( and: [ codeCondition.test, { - not: [optOutBundlingPackageRegex], + not: [optOutBundlingPackageRegex, asyncStoragesRegex], }, ], }, @@ -1499,6 +1498,7 @@ export default async function getBaseWebpackConfig( { test: codeCondition.test, issuerLayer: WEBPACK_LAYERS.serverSideRendering, + exclude: asyncStoragesRegex, use: appSSRLayerLoaders, resolve: { mainFields: getMainField(compilerType, true), diff --git a/packages/next/src/client/components/action-async-storage-instance.ts b/packages/next/src/client/components/action-async-storage-instance.ts new file mode 100644 index 0000000000000..161879ae2d760 --- /dev/null +++ b/packages/next/src/client/components/action-async-storage-instance.ts @@ -0,0 +1,4 @@ +import type { ActionAsyncStorage } from './action-async-storage.external' +import { createAsyncLocalStorage } from './async-local-storage' + +export const actionAsyncStorage: ActionAsyncStorage = createAsyncLocalStorage() diff --git a/packages/next/src/client/components/action-async-storage.external.ts b/packages/next/src/client/components/action-async-storage.external.ts index d34b8225c818b..beaac5d06902f 100644 --- a/packages/next/src/client/components/action-async-storage.external.ts +++ b/packages/next/src/client/components/action-async-storage.external.ts @@ -1,6 +1,9 @@ import type { AsyncLocalStorage } from 'async_hooks' -import { createAsyncLocalStorage } from './async-local-storage' +// Share the instance module in the next-shared layer +// eslint-disable-next-line @typescript-eslint/no-unused-expressions +;('TURBOPACK { transition: next-shared }') +import { actionAsyncStorage } from './action-async-storage-instance' export interface ActionStore { readonly isAction?: boolean readonly isAppRoute?: boolean @@ -8,4 +11,4 @@ export interface ActionStore { export type ActionAsyncStorage = AsyncLocalStorage -export const actionAsyncStorage: ActionAsyncStorage = createAsyncLocalStorage() +export { actionAsyncStorage } diff --git a/packages/next/src/client/components/request-async-storage-instance.ts b/packages/next/src/client/components/request-async-storage-instance.ts new file mode 100644 index 0000000000000..a9b6708432987 --- /dev/null +++ b/packages/next/src/client/components/request-async-storage-instance.ts @@ -0,0 +1,5 @@ +import { createAsyncLocalStorage } from './async-local-storage' +import type { RequestAsyncStorage } from './request-async-storage.external' + +export const requestAsyncStorage: RequestAsyncStorage = + createAsyncLocalStorage() diff --git a/packages/next/src/client/components/request-async-storage.external.ts b/packages/next/src/client/components/request-async-storage.external.ts index dbd33809881c3..0af201362a3da 100644 --- a/packages/next/src/client/components/request-async-storage.external.ts +++ b/packages/next/src/client/components/request-async-storage.external.ts @@ -4,7 +4,10 @@ import type { ResponseCookies } from '../../server/web/spec-extension/cookies' import type { ReadonlyHeaders } from '../../server/web/spec-extension/adapters/headers' import type { ReadonlyRequestCookies } from '../../server/web/spec-extension/adapters/request-cookies' -import { createAsyncLocalStorage } from './async-local-storage' +// Share the instance module in the next-shared layer +// eslint-disable-next-line @typescript-eslint/no-unused-expressions +;('TURBOPACK { transition: next-shared }') +import { requestAsyncStorage } from './request-async-storage-instance' import type { DeepReadonly } from '../../shared/lib/deep-readonly' export interface RequestStore { @@ -20,8 +23,7 @@ export interface RequestStore { export type RequestAsyncStorage = AsyncLocalStorage -export const requestAsyncStorage: RequestAsyncStorage = - createAsyncLocalStorage() +export { requestAsyncStorage } export function getExpectedRequestStore(callingExpression: string) { const store = requestAsyncStorage.getStore() diff --git a/packages/next/src/client/components/static-generation-async-storage-instance.ts b/packages/next/src/client/components/static-generation-async-storage-instance.ts new file mode 100644 index 0000000000000..3ac18d8f1c2ef --- /dev/null +++ b/packages/next/src/client/components/static-generation-async-storage-instance.ts @@ -0,0 +1,5 @@ +import type { StaticGenerationAsyncStorage } from './static-generation-async-storage.external' +import { createAsyncLocalStorage } from './async-local-storage' + +export const staticGenerationAsyncStorage: StaticGenerationAsyncStorage = + createAsyncLocalStorage() diff --git a/packages/next/src/client/components/static-generation-async-storage.external.ts b/packages/next/src/client/components/static-generation-async-storage.external.ts index 4433992a8f331..2a21c77357dfe 100644 --- a/packages/next/src/client/components/static-generation-async-storage.external.ts +++ b/packages/next/src/client/components/static-generation-async-storage.external.ts @@ -5,7 +5,10 @@ import type { FetchMetrics } from '../../server/base-http' import type { Revalidate } from '../../server/lib/revalidate' import type { PrerenderState } from '../../server/app-render/dynamic-rendering' -import { createAsyncLocalStorage } from './async-local-storage' +// Share the instance module in the next-shared layer +// eslint-disable-next-line @typescript-eslint/no-unused-expressions +;('TURBOPACK { transition: next-shared }') +import { staticGenerationAsyncStorage } from './static-generation-async-storage-instance' export interface StaticGenerationStore { readonly isStaticGeneration: boolean @@ -53,5 +56,4 @@ export interface StaticGenerationStore { export type StaticGenerationAsyncStorage = AsyncLocalStorage -export const staticGenerationAsyncStorage: StaticGenerationAsyncStorage = - createAsyncLocalStorage() +export { staticGenerationAsyncStorage } diff --git a/packages/next/src/lib/constants.ts b/packages/next/src/lib/constants.ts index 7c8c320e256b0..ed3679d06273d 100644 --- a/packages/next/src/lib/constants.ts +++ b/packages/next/src/lib/constants.ts @@ -170,7 +170,6 @@ const WEBPACK_LAYERS = { clientOnly: [ WEBPACK_LAYERS_NAMES.serverSideRendering, WEBPACK_LAYERS_NAMES.appPagesBrowser, - WEBPACK_LAYERS_NAMES.shared, ], nonClientServerTarget: [ // middleware and pages api diff --git a/packages/next/src/shared/lib/lazy-dynamic/preload-css.tsx b/packages/next/src/shared/lib/lazy-dynamic/preload-css.tsx index cc1549e970cc0..aac260fc2b251 100644 --- a/packages/next/src/shared/lib/lazy-dynamic/preload-css.tsx +++ b/packages/next/src/shared/lib/lazy-dynamic/preload-css.tsx @@ -1,15 +1,14 @@ 'use client' +import { getExpectedRequestStore } from '../../../client/components/request-async-storage.external' + export function PreloadCss({ moduleIds }: { moduleIds: string[] | undefined }) { // Early return in client compilation and only load requestStore on server side if (typeof window !== 'undefined') { return null } - const { - getExpectedRequestStore, - } = require('../../../client/components/request-async-storage.external') - const requestStore = getExpectedRequestStore() + const requestStore = getExpectedRequestStore('next/dynamic css') const allFiles = [] // Search the current dynamic call unique key id in react loadable manifest, diff --git a/test/e2e/app-dir/dynamic-css/app/ssr/edge/page.js b/test/e2e/app-dir/dynamic-css/app/ssr/edge/page.js new file mode 100644 index 0000000000000..943a37c4b4230 --- /dev/null +++ b/test/e2e/app-dir/dynamic-css/app/ssr/edge/page.js @@ -0,0 +1,3 @@ +export { default } from '../page' + +export const runtime = 'edge' diff --git a/test/e2e/app-dir/dynamic-css/index.test.ts b/test/e2e/app-dir/dynamic-css/index.test.ts index 2892e69fbc466..c44cb4a82bfaa 100644 --- a/test/e2e/app-dir/dynamic-css/index.test.ts +++ b/test/e2e/app-dir/dynamic-css/index.test.ts @@ -31,6 +31,23 @@ createNextDescribe( }) }) + it('should only apply corresponding css for page loaded in edge runtime', async () => { + const browser = await next.browser('/ssr/edge') + await retry(async () => { + expect( + await browser.eval( + `window.getComputedStyle(document.querySelector('.text')).color` + ) + ).toBe('rgb(255, 0, 0)') + // Default border width, which is not effected by bar.css that is not loaded in /ssr + expect( + await browser.eval( + `window.getComputedStyle(document.querySelector('.text')).borderWidth` + ) + ).toBe('0px') + }) + }) + it('should only apply corresponding css for page loaded that /another', async () => { const browser = await next.browser('/another') await retry(async () => { @@ -47,5 +64,10 @@ createNextDescribe( ).toBe('1px') }) }) + + it('should not throw with accessing to ALS in preload css', async () => { + const output = next.cliOutput + expect(output).not.toContain('was called outside a request scope') + }) } )