Skip to content

Commit

Permalink
Fix css FOUC in dynamic component (#64294)
Browse files Browse the repository at this point in the history
### What

CSS imports in components that loaded by `next/dynamic` in client
components will cause the css are missing initial in
SSR, and loaded later on client side which will lead to FOUC. This PR
fixes the issue and get CSS preloaded in the SSR for dynamic components.

### Why

The CSS from client components that created through `next/dynamic` are
not collected in the SSR, unlike RSC rendering we already collect the
CSS resources for each entry so we included them in the server rendering
so the styles are availble at that time. But for client components, we
didn't traverse all the client components and collect the CSS resources.

In pages router we kinda collect all the dynamic imports and preload
them during SSR, but this approach is not able to be applied to app
router due to different architecture. Since we already have all the
dynamic imports info and their related chunks in
react-loadable-manifest, so we can do the similar "preloading" thing in
app router. We use the current dynamic module key (`app/page.js ->
../components/foo.js`) which created by SWC transform and match it in
the react loadable manifest that accessed from `AsyncLocalStorage`, to
get the css files created by webpack then render them as preload
styleshee links. In this way we can SSR all the related CSS resources
for dynamic client components.

The reason we pass down the react loadable manifest through
`AsyncLocalStorage` is that it's sort of exclude the manifest from RSC
payload as it's not required for hydration, but only required for SSR.

Note: this issue only occurred in dynamic rendering case for client
components.

### Other Changes Overview

- Change the react loadable manifest key from pages dir based relative
path to a source dir based relative path, to support cases with both
directory or only one of them

Closes NEXT-2578
Fixes #61212
Fixes #61111
Fixes #62940

Replacement for #64021 but only with production test
  • Loading branch information
huozhi authored Apr 10, 2024
1 parent bccf55c commit 142050d
Show file tree
Hide file tree
Showing 45 changed files with 283 additions and 95 deletions.
8 changes: 4 additions & 4 deletions packages/next-swc/crates/next-api/src/pages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -924,7 +924,7 @@ impl PageEndpoint {
) -> Result<Vc<OutputAssets>> {
let this = self.await?;
let node_root = this.pages_project.project().node_root();
let pages_dir = this.pages_project.pages_dir().await?;
let project_src_dir = this.pages_project.pages_dir().parent().await?;

let dynamic_import_entries = &*dynamic_import_entries.await?;

Expand All @@ -937,12 +937,12 @@ impl PageEndpoint {
let chunk_output = chunk_output.await?;
output.extend(chunk_output.iter().copied());

// https://github.com/vercel/next.js/blob/b7c85b87787283d8fb86f705f67bdfabb6b654bb/packages/next-swc/crates/next-transform-dynamic/src/lib.rs#L230
// For the pages dir, next_dynamic transform puts relative paths to the pages
// https://github.com/vercel/next.js/blob/canary/packages/next/src/build/webpack/plugins/react-loadable-plugin.ts
// next_dynamic transform puts relative paths to the project dir
// dir for the origin import.
let id = format!(
"{} -> {}",
pages_dir
project_src_dir
.get_path_to(origin_path)
.map_or_else(|| origin_path.to_string(), |path| path.to_string()),
import
Expand Down
10 changes: 6 additions & 4 deletions packages/next-swc/crates/next-core/src/next_client/transforms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub async fn get_next_client_transforms_rules(
let mdx_rs = *next_config.mdx_rs().await?;
rules.push(get_next_font_transform_rule(mdx_rs));

let pages_dir = match context_ty {
let pages_or_app_dir = match context_ty {
ClientContextType::Pages { pages_dir } => {
if !foreign_code {
rules.push(
Expand All @@ -59,12 +59,12 @@ pub async fn get_next_client_transforms_rules(
}
Some(pages_dir)
}
ClientContextType::App { .. } => {
ClientContextType::App { app_dir } => {
rules.push(get_server_actions_transform_rule(
ActionsTransform::Client,
mdx_rs,
));
None
Some(app_dir)
}
ClientContextType::Fallback | ClientContextType::Other => None,
};
Expand All @@ -74,7 +74,9 @@ pub async fn get_next_client_transforms_rules(
rules.push(get_next_cjs_optimizer_rule(mdx_rs));
rules.push(get_next_pure_rule(mdx_rs));

rules.push(get_next_dynamic_transform_rule(false, false, pages_dir, mode, mdx_rs).await?);
rules.push(
get_next_dynamic_transform_rule(false, false, pages_or_app_dir, mode, mdx_rs).await?,
);

rules.push(get_next_image_rule());
rules.push(get_next_page_static_info_assert_rule(
Expand Down
22 changes: 15 additions & 7 deletions packages/next-swc/crates/next-core/src/next_server/transforms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub async fn get_next_server_transforms_rules(
));
}

let (is_server_components, pages_dir) = match context_ty {
let (is_server_components, pages_or_app_dir) = match context_ty {
ServerContextType::Pages { pages_dir } | ServerContextType::PagesApi { pages_dir } => {
if !foreign_code {
rules.push(get_next_disallow_export_all_in_page_rule(
Expand All @@ -77,18 +77,20 @@ pub async fn get_next_server_transforms_rules(
}
(false, Some(pages_dir))
}
ServerContextType::AppSSR { .. } => {
ServerContextType::AppSSR { app_dir } => {
// Yah, this is SSR, but this is still treated as a Client transform layer.
// need to apply to foreign code too
rules.push(get_server_actions_transform_rule(
ActionsTransform::Client,
mdx_rs,
));

(false, None)
(false, Some(app_dir))
}
ServerContextType::AppRSC {
client_transition, ..
app_dir,
client_transition,
..
} => {
rules.push(get_server_actions_transform_rule(
ActionsTransform::Server,
Expand All @@ -100,7 +102,7 @@ pub async fn get_next_server_transforms_rules(
client_transition,
));
}
(true, None)
(true, Some(app_dir))
}
ServerContextType::AppRoute { .. } => (false, None),
ServerContextType::Middleware { .. } | ServerContextType::Instrumentation { .. } => {
Expand All @@ -110,8 +112,14 @@ pub async fn get_next_server_transforms_rules(

if !foreign_code {
rules.push(
get_next_dynamic_transform_rule(true, is_server_components, pages_dir, mode, mdx_rs)
.await?,
get_next_dynamic_transform_rule(
true,
is_server_components,
pages_or_app_dir,
mode,
mdx_rs,
)
.await?,
);

rules.push(get_next_amp_attr_rule(mdx_rs));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ use crate::mode::NextMode;
pub async fn get_next_dynamic_transform_rule(
is_server_compiler: bool,
is_react_server_layer: bool,
pages_dir: Option<Vc<FileSystemPath>>,
pages_or_app_dir: Option<Vc<FileSystemPath>>,
mode: Vc<NextMode>,
enable_mdx_rs: bool,
) -> Result<ModuleRule> {
let dynamic_transform = EcmascriptInputTransform::Plugin(Vc::cell(Box::new(NextJsDynamic {
is_server_compiler,
is_react_server_layer,
pages_dir: match pages_dir {
pages_or_app_dir: match pages_or_app_dir {
None => None,
Some(path) => Some(path.await?.path.clone().into()),
},
Expand All @@ -52,7 +52,7 @@ pub async fn get_next_dynamic_transform_rule(
struct NextJsDynamic {
is_server_compiler: bool,
is_react_server_layer: bool,
pages_dir: Option<PathBuf>,
pages_or_app_dir: Option<PathBuf>,
mode: NextMode,
}

Expand All @@ -67,7 +67,7 @@ impl CustomTransformer for NextJsDynamic {
false,
NextDynamicMode::Webpack,
FileName::Real(ctx.file_path_str.into()),
self.pages_dir.clone(),
self.pages_or_app_dir.clone(),
));

Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ where
opts.prefer_esm,
NextDynamicMode::Webpack,
file.name.clone(),
opts.pages_dir.clone()
opts.pages_dir.clone().or_else(|| opts.app_dir.clone()),
),
Optional::new(
crate::transforms::page_config::page_config(opts.is_development, opts.is_page_file),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@ pub fn next_dynamic(
prefer_esm: bool,
mode: NextDynamicMode,
filename: FileName,
pages_dir: Option<PathBuf>,
pages_or_app_dir: Option<PathBuf>,
) -> impl Fold {
NextDynamicPatcher {
is_development,
is_server_compiler,
is_react_server_layer,
prefer_esm,
pages_dir,
pages_or_app_dir,
filename,
dynamic_bindings: vec![],
is_next_dynamic_first_arg: false,
Expand Down Expand Up @@ -81,7 +81,7 @@ struct NextDynamicPatcher {
is_server_compiler: bool,
is_react_server_layer: bool,
prefer_esm: bool,
pages_dir: Option<PathBuf>,
pages_or_app_dir: Option<PathBuf>,
filename: FileName,
dynamic_bindings: Vec<Id>,
is_next_dynamic_first_arg: bool,
Expand Down Expand Up @@ -216,6 +216,11 @@ impl Fold for NextDynamicPatcher {
return expr;
};

let project_dir = match self.pages_or_app_dir.as_deref() {
Some(pages_or_app) => pages_or_app.parent(),
_ => None,
};

// dev client or server:
// loadableGenerated: {
// modules:
Expand All @@ -233,7 +238,7 @@ impl Fold for NextDynamicPatcher {
"$left + $right" as Expr,
left: Expr = format!(
"{} -> ",
rel_filename(self.pages_dir.as_deref(), &self.filename)
rel_filename(project_dir, &self.filename)
)
.into(),
right: Expr = dynamically_imported_specifier.clone().into(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import dynamic from 'next/dynamic';
export const NextDynamicNoSSRServerComponent = dynamic(()=>import('../text-dynamic-no-ssr-server'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../text-dynamic-no-ssr-server"
"src/some-file.js -> " + "../text-dynamic-no-ssr-server"
]
},
ssr: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import dynamic from 'next/dynamic';
export const NextDynamicNoSSRServerComponent = dynamic(()=>import('../text-dynamic-no-ssr-server'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../text-dynamic-no-ssr-server"
"src/some-file.js -> " + "../text-dynamic-no-ssr-server"
]
},
ssr: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import dynamic from 'next/dynamic';
export const NextDynamicNoSSRServerComponent = dynamic(()=>import('../text-dynamic-no-ssr-server'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../text-dynamic-no-ssr-server"
"src/some-file.js -> " + "../text-dynamic-no-ssr-server"
]
},
ssr: false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import dynamic1 from 'next/dynamic';
import dynamic2 from 'next/dynamic';
const DynamicComponent1 = dynamic1(()=>import('../components/hello1')
, {
const DynamicComponent1 = dynamic1(()=>import('../components/hello1'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello1"
"src/some-file.js -> " + "../components/hello1"
]
}
});
const DynamicComponent2 = dynamic2(()=>import('../components/hello2')
, {
const DynamicComponent2 = dynamic2(()=>import('../components/hello2'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello2"
"src/some-file.js -> " + "../components/hello2"
]
}
});
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import dynamic1 from 'next/dynamic';
import dynamic2 from 'next/dynamic';
const DynamicComponent1 = dynamic1(()=>import('../components/hello1')
, {
const DynamicComponent1 = dynamic1(()=>import('../components/hello1'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello1"
"src/some-file.js -> " + "../components/hello1"
]
}
});
const DynamicComponent2 = dynamic2(()=>import('../components/hello2')
, {
const DynamicComponent2 = dynamic2(()=>import('../components/hello2'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello2"
"src/some-file.js -> " + "../components/hello2"
]
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import dynamic from 'next/dynamic';
export const NextDynamicNoSSRServerComponent = dynamic(()=>import('../text-dynamic-no-ssr-server'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../text-dynamic-no-ssr-server"
"src/some-file.js -> " + "../text-dynamic-no-ssr-server"
]
},
ssr: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import dynamic from 'next/dynamic';
export const NextDynamicNoSSRServerComponent = dynamic(()=>import('../text-dynamic-no-ssr-server'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../text-dynamic-no-ssr-server"
"src/some-file.js -> " + "../text-dynamic-no-ssr-server"
]
},
ssr: false
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import dynamic from 'next/dynamic';
import somethingElse from 'something-else';
const DynamicComponent = dynamic(()=>import('../components/hello')
, {
const DynamicComponent = dynamic(()=>import('../components/hello'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello"
"src/some-file.js -> " + "../components/hello"
]
}
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import dynamic from 'next/dynamic';
import somethingElse from 'something-else';
const DynamicComponent = dynamic(()=>import('../components/hello')
, {
const DynamicComponent = dynamic(()=>import('../components/hello'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello"
"src/some-file.js -> " + "../components/hello"
]
}
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import dynamic from 'next/dynamic';
const DynamicComponent = dynamic(()=>import('../components/hello')
, {
const DynamicComponent = dynamic(()=>import('../components/hello'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello"
"src/some-file.js -> " + "../components/hello"
]
}
});
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import dynamic from 'next/dynamic';
const DynamicComponent = dynamic(()=>import('../components/hello')
, {
const DynamicComponent = dynamic(()=>import('../components/hello'), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello"
"src/some-file.js -> " + "../components/hello"
]
}
});
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import dynamic from 'next/dynamic';
const DynamicComponent = dynamic(()=>import(`../components/hello`), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello"
"src/some-file.js -> " + "../components/hello"
]
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import dynamic from 'next/dynamic';
const DynamicComponent = dynamic(()=>import(`../components/hello`), {
loadableGenerated: {
modules: [
"some-file.js -> " + "../components/hello"
"src/some-file.js -> " + "../components/hello"
]
}
});
Expand Down
Loading

0 comments on commit 142050d

Please sign in to comment.