Skip to content

Commit

Permalink
Fix ASL bundling for dynamic css (#64451)
Browse files Browse the repository at this point in the history
### 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
  • Loading branch information
huozhi committed Apr 17, 2024
1 parent 07a0700 commit 8b82225
Show file tree
Hide file tree
Showing 12 changed files with 114 additions and 16 deletions.
56 changes: 55 additions & 1 deletion packages/next-swc/crates/next-api/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -359,8 +371,30 @@ impl AppProject {

#[turbo_tasks::function]
fn edge_route_module_context(self: Vc<Self>) -> Vc<ModuleAssetContext> {
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(),
Expand Down Expand Up @@ -435,6 +469,16 @@ impl AppProject {
)
}

#[turbo_tasks::function]
fn shared_transition(self: Vc<Self>) -> Vc<ContextTransition> {
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<Self>) -> Vc<ContextTransition> {
ContextTransition::new(
Expand All @@ -445,6 +489,16 @@ impl AppProject {
)
}

#[turbo_tasks::function]
fn edge_shared_transition(self: Vc<Self>) -> Vc<ContextTransition> {
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<Self>) -> Result<Vc<RuntimeEntries>> {
Ok(get_server_runtime_entries(
Expand Down
4 changes: 2 additions & 2 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1383,15 +1383,14 @@ 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
// opted out of bundling.
and: [
codeCondition.test,
{
not: [optOutBundlingPackageRegex],
not: [optOutBundlingPackageRegex, asyncStoragesRegex],
},
],
},
Expand Down Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import type { ActionAsyncStorage } from './action-async-storage.external'
import { createAsyncLocalStorage } from './async-local-storage'

export const actionAsyncStorage: ActionAsyncStorage = createAsyncLocalStorage()
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
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
}

export type ActionAsyncStorage = AsyncLocalStorage<ActionStore>

export const actionAsyncStorage: ActionAsyncStorage = createAsyncLocalStorage()
export { actionAsyncStorage }
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { createAsyncLocalStorage } from './async-local-storage'
import type { RequestAsyncStorage } from './request-async-storage.external'

export const requestAsyncStorage: RequestAsyncStorage =
createAsyncLocalStorage()
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -20,8 +23,7 @@ export interface RequestStore {

export type RequestAsyncStorage = AsyncLocalStorage<RequestStore>

export const requestAsyncStorage: RequestAsyncStorage =
createAsyncLocalStorage()
export { requestAsyncStorage }

export function getExpectedRequestStore(callingExpression: string) {
const store = requestAsyncStorage.getStore()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import type { StaticGenerationAsyncStorage } from './static-generation-async-storage.external'
import { createAsyncLocalStorage } from './async-local-storage'

export const staticGenerationAsyncStorage: StaticGenerationAsyncStorage =
createAsyncLocalStorage()
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -53,5 +56,4 @@ export interface StaticGenerationStore {
export type StaticGenerationAsyncStorage =
AsyncLocalStorage<StaticGenerationStore>

export const staticGenerationAsyncStorage: StaticGenerationAsyncStorage =
createAsyncLocalStorage()
export { staticGenerationAsyncStorage }
1 change: 0 additions & 1 deletion packages/next/src/lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions packages/next/src/shared/lib/lazy-dynamic/preload-css.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/dynamic-css/app/ssr/edge/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export { default } from '../page'

export const runtime = 'edge'
22 changes: 22 additions & 0 deletions test/e2e/app-dir/dynamic-css/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand All @@ -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')
})
}
)

0 comments on commit 8b82225

Please sign in to comment.