Skip to content

Commit

Permalink
[Turbopack] place style imports in RSC in the client context (vercel#…
Browse files Browse the repository at this point in the history
…69952)

### Why?

Styles imported from RSC are processed twice by webpack loaders and
postcss. This is not only expensive, but can also lead to hanging
problems if both executions doesn't result in the same code (e. g. when
using randomness in webpack loaders).

### What?

Add a transition to styles to place them in the client context when
import on RSC side.

fixes vercel#69643
fixes PACK-3243
  • Loading branch information
sokra authored Sep 12, 2024
1 parent 45fc346 commit 0731c88
Show file tree
Hide file tree
Showing 15 changed files with 75 additions and 213 deletions.
18 changes: 17 additions & 1 deletion crates/next-api/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use turbo_tasks::{trace::TraceRawVcs, Completion, RcStr, TryJoinIterExt, Value,
use turbo_tasks_env::{CustomProcessEnv, ProcessEnv};
use turbo_tasks_fs::{File, FileContent, FileSystemPath};
use turbopack::{
module_options::ModuleOptionsContext,
module_options::{transition_rule::TransitionRule, ModuleOptionsContext, RuleCondition},
resolve_options_context::ResolveOptionsContext,
transition::{ContextTransition, FullContextTransition, Transition, TransitionOptions},
ModuleAssetContext,
Expand Down Expand Up @@ -118,6 +118,14 @@ impl AppProject {

pub(crate) const ECMASCRIPT_CLIENT_TRANSITION_NAME: &str = "next-ecmascript-client-reference";

fn styles_rule_condition() -> RuleCondition {
RuleCondition::any(vec![
RuleCondition::ResourcePathEndsWith(".css".into()),
RuleCondition::ResourcePathEndsWith(".scss".into()),
RuleCondition::ResourcePathEndsWith(".sass".into()),
])
}

#[turbo_tasks::value_impl]
impl AppProject {
#[turbo_tasks::function]
Expand Down Expand Up @@ -305,6 +313,10 @@ impl AppProject {
ModuleAssetContext::new(
TransitionOptions {
named_transitions: transitions,
transition_rules: vec![TransitionRule::new(
styles_rule_condition(),
Vc::upcast(self.client_transition()),
)],
..Default::default()
}
.cell(),
Expand Down Expand Up @@ -339,6 +351,10 @@ impl AppProject {
ModuleAssetContext::new(
TransitionOptions {
named_transitions: transitions,
transition_rules: vec![TransitionRule::new(
styles_rule_condition(),
Vc::upcast(self.client_transition()),
)],
..Default::default()
}
.cell(),
Expand Down
12 changes: 4 additions & 8 deletions crates/next-core/src/next_app/app_client_references_chunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,9 @@ pub async fn get_app_client_references_chunks(
},
)
}
ClientReferenceType::CssClientReference(css_client_reference) => {
let css_client_reference_ref = css_client_reference.await?;
ClientReferenceType::CssClientReference(css_module) => {
let client_chunk_group = client_chunking_context
.root_chunk_group(Vc::upcast(
css_client_reference_ref.client_module,
))
.root_chunk_group(Vc::upcast(css_module))
.await?;

(
Expand Down Expand Up @@ -222,9 +219,8 @@ pub async fn get_app_client_references_chunks(
ecmascript_client_reference.await?;
Vc::upcast(ecmascript_client_reference_ref.client_module)
}
ClientReferenceType::CssClientReference(css_client_reference) => {
let css_client_reference_ref = css_client_reference.await?;
Vc::upcast(css_client_reference_ref.client_module)
ClientReferenceType::CssClientReference(css_module) => {
Vc::upcast(*css_module)
}
})
})
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

2 changes: 0 additions & 2 deletions crates/next-core/src/next_client_reference/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
pub(crate) mod css_client_reference;
pub(crate) mod ecmascript_client_reference;
pub(crate) mod visit_client_reference;

pub use css_client_reference::css_client_reference_module::CssClientReferenceModule;
pub use ecmascript_client_reference::{
ecmascript_client_reference_module::EcmascriptClientReferenceModule,
ecmascript_client_reference_transition::NextEcmascriptClientReferenceTransition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@ use turbo_tasks::{
trace::TraceRawVcs,
RcStr, ReadRef, TryJoinIterExt, ValueToString, Vc,
};
use turbopack::css::CssModuleAsset;
use turbopack_core::{
module::{Module, Modules},
reference::primary_referenced_modules,
};

use super::{
css_client_reference::css_client_reference_module::CssClientReferenceModule,
ecmascript_client_reference::ecmascript_client_reference_module::EcmascriptClientReferenceModule,
};
use super::ecmascript_client_reference::ecmascript_client_reference_module::EcmascriptClientReferenceModule;
use crate::next_server_component::server_component_module::NextServerComponentModule;

#[derive(
Expand All @@ -44,7 +42,7 @@ impl ClientReference {
)]
pub enum ClientReferenceType {
EcmascriptClientReference(Vc<EcmascriptClientReferenceModule>),
CssClientReference(Vc<CssClientReferenceModule>),
CssClientReference(Vc<CssModuleAsset>),
}

#[turbo_tasks::value]
Expand Down Expand Up @@ -196,7 +194,7 @@ impl Visit<VisitClientReferenceNode> for VisitClientReference {
}

if let Some(css_client_reference_asset) =
Vc::try_resolve_downcast_type::<CssClientReferenceModule>(module).await?
Vc::try_resolve_downcast_type::<CssModuleAsset>(module).await?
{
return Ok(VisitClientReferenceNode {
server_component: node.server_component,
Expand Down
20 changes: 2 additions & 18 deletions crates/next-core/src/next_server/transforms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use turbopack::module_options::ModuleRule;

use crate::{
mode::NextMode,
next_client_reference::css_client_reference::css_client_reference_rule::get_next_css_client_reference_transforms_rule,
next_config::NextConfig,
next_server::context::ServerContextType,
next_shared::transforms::{
Expand Down Expand Up @@ -91,19 +90,12 @@ pub async fn get_next_server_transforms_rules(

false
}
ServerContextType::AppRSC {
client_transition, ..
} => {
ServerContextType::AppRSC { .. } => {
rules.push(get_server_actions_transform_rule(
ActionsTransform::Server,
mdx_rs,
));

if let Some(client_transition) = client_transition {
rules.push(get_next_css_client_reference_transforms_rule(
client_transition,
));
}
is_app_dir = true;

true
Expand Down Expand Up @@ -164,16 +156,8 @@ pub async fn get_next_server_internal_transforms_rules(
ServerContextType::AppSSR { .. } => {
rules.push(get_next_font_transform_rule(mdx_rs));
}
ServerContextType::AppRSC {
client_transition, ..
} => {
ServerContextType::AppRSC { .. } => {
rules.push(get_next_font_transform_rule(mdx_rs));
if let Some(client_transition) = client_transition {
rules.push(get_next_css_client_reference_transforms_rule(
client_transition,
));
}
{}
}
ServerContextType::AppRoute { .. } => {}
ServerContextType::Middleware { .. } => {}
Expand Down
8 changes: 8 additions & 0 deletions test/e2e/app-dir/random-in-sass/app/layout.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { ReactNode } from 'react'
export default function Root({ children }: { children: ReactNode }) {
return (
<html>
<body>{children}</body>
</html>
)
}
7 changes: 7 additions & 0 deletions test/e2e/app-dir/random-in-sass/app/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import dynamic from 'next/dynamic'

const DynamicComponent = dynamic(() => import('../components/hello'))

export default function Page() {
return <DynamicComponent />
}
4 changes: 4 additions & 0 deletions test/e2e/app-dir/random-in-sass/components/hello.module.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
.hello {
font-size: random(100) + px;
color: rgb(0, 255, 0);
}
5 changes: 5 additions & 0 deletions test/e2e/app-dir/random-in-sass/components/hello.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import styles from './hello.module.scss'

export default function Problem(props) {
return <p className={styles.hello}>Hello World</p>
}
6 changes: 6 additions & 0 deletions test/e2e/app-dir/random-in-sass/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/**
* @type {import('next').NextConfig}
*/
const nextConfig = {}

module.exports = nextConfig
Loading

0 comments on commit 0731c88

Please sign in to comment.