Skip to content

Commit 902b58c

Browse files
authored
Turbopack: more module ident collisions (#78207)
Fixes two more module ident collisions 1. `ContextTransition` is a massive footgun, because it reuses the transitions from the parent context. So it essentially always creates a new context (which is not actually what we want), e.g. here this created two different `app-edge-shared` contexts because `app-edge-rsc` and `app-edge-ssr` have different transitions. I hope this wasn't by design? ``` next/dist/esm/server/app-render/after-task-async-storage.external.js [app-edge-rsc] -> next/dist/esm/server/app-render/after-task-async-storage-instance.js [app-edge-shared] next/dist/esm/server/app-render/after-task-async-storage.external.js [app-edge-ssr] -> next/dist/esm/server/app-render/after-task-async-storage-instance.js [app-edge-shared] ``` 2. The client module proxy module was missing its layer in the identifier (the layer of the proxy module, i.e. `app-edge-rsc` or `app-rsc`). It's correct that there are two different modules.
1 parent 2a91953 commit 902b58c

File tree

5 files changed

+75
-97
lines changed

5 files changed

+75
-97
lines changed

crates/next-api/src/app.rs

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ use turbo_tasks_fs::{File, FileContent, FileSystemPath};
4747
use turbopack::{
4848
module_options::{transition_rule::TransitionRule, ModuleOptionsContext, RuleCondition},
4949
resolve_options_context::ResolveOptionsContext,
50-
transition::{ContextTransition, FullContextTransition, Transition, TransitionOptions},
50+
transition::{FullContextTransition, Transition, TransitionOptions},
5151
ModuleAssetContext,
5252
};
5353
use turbopack_core::{
@@ -663,13 +663,22 @@ impl AppProject {
663663
}
664664

665665
#[turbo_tasks::function]
666-
fn shared_transition(self: Vc<Self>) -> Vc<ContextTransition> {
667-
ContextTransition::new(
666+
async fn shared_module_context(self: Vc<Self>) -> Result<Vc<ModuleAssetContext>> {
667+
Ok(ModuleAssetContext::new(
668+
TransitionOptions {
669+
..Default::default()
670+
}
671+
.cell(),
668672
self.project().server_compile_time_info(),
669673
self.ssr_module_options_context(),
670674
self.ssr_resolve_options_context(),
671675
Vc::cell("app-shared".into()),
672-
)
676+
))
677+
}
678+
679+
#[turbo_tasks::function]
680+
fn shared_transition(self: Vc<Self>) -> Vc<Box<dyn Transition>> {
681+
Vc::upcast(FullContextTransition::new(self.shared_module_context()))
673682
}
674683

675684
#[turbo_tasks::function]
@@ -714,13 +723,24 @@ impl AppProject {
714723
}
715724

716725
#[turbo_tasks::function]
717-
fn edge_shared_transition(self: Vc<Self>) -> Vc<ContextTransition> {
718-
ContextTransition::new(
726+
async fn edge_shared_module_context(self: Vc<Self>) -> Result<Vc<ModuleAssetContext>> {
727+
Ok(ModuleAssetContext::new(
728+
TransitionOptions {
729+
..Default::default()
730+
}
731+
.cell(),
719732
self.project().edge_compile_time_info(),
720733
self.edge_ssr_module_options_context(),
721734
self.edge_ssr_resolve_options_context(),
722735
Vc::cell("app-edge-shared".into()),
723-
)
736+
))
737+
}
738+
739+
#[turbo_tasks::function]
740+
fn edge_shared_transition(self: Vc<Self>) -> Vc<Box<dyn Transition>> {
741+
Vc::upcast(FullContextTransition::new(
742+
self.edge_shared_module_context(),
743+
))
724744
}
725745

726746
#[turbo_tasks::function]

crates/next-api/src/pages.rs

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ use turbo_tasks_fs::{
3939
use turbopack::{
4040
module_options::ModuleOptionsContext,
4141
resolve_options_context::ResolveOptionsContext,
42-
transition::{ContextTransition, TransitionOptions},
42+
transition::{FullContextTransition, Transition, TransitionOptions},
4343
ModuleAssetContext,
4444
};
4545
use turbopack_core::{
@@ -290,7 +290,27 @@ impl PagesProject {
290290
}
291291

292292
#[turbo_tasks::function]
293-
async fn transitions(self: Vc<Self>) -> Result<Vc<TransitionOptions>> {
293+
async fn client_transitions(self: Vc<Self>) -> Result<Vc<TransitionOptions>> {
294+
Ok(TransitionOptions {
295+
named_transitions: [
296+
(
297+
"next-dynamic".into(),
298+
ResolvedVc::upcast(NextDynamicTransition::new_marker().to_resolved().await?),
299+
),
300+
(
301+
"next-dynamic-client".into(),
302+
ResolvedVc::upcast(NextDynamicTransition::new_marker().to_resolved().await?),
303+
),
304+
]
305+
.into_iter()
306+
.collect(),
307+
..Default::default()
308+
}
309+
.cell())
310+
}
311+
312+
#[turbo_tasks::function]
313+
async fn server_transitions(self: Vc<Self>) -> Result<Vc<TransitionOptions>> {
294314
Ok(TransitionOptions {
295315
named_transitions: [
296316
(
@@ -314,13 +334,8 @@ impl PagesProject {
314334
}
315335

316336
#[turbo_tasks::function]
317-
fn client_transition(self: Vc<Self>) -> Vc<ContextTransition> {
318-
ContextTransition::new(
319-
self.project().client_compile_time_info(),
320-
self.client_module_options_context(),
321-
self.client_resolve_options_context(),
322-
client_layer(),
323-
)
337+
fn client_transition(self: Vc<Self>) -> Vc<Box<dyn Transition>> {
338+
Vc::upcast(FullContextTransition::new(self.client_module_context()))
324339
}
325340

326341
#[turbo_tasks::function]
@@ -353,20 +368,20 @@ impl PagesProject {
353368
}
354369

355370
#[turbo_tasks::function]
356-
pub(super) fn client_module_context(self: Vc<Self>) -> Vc<Box<dyn AssetContext>> {
357-
Vc::upcast(ModuleAssetContext::new(
358-
self.transitions(),
371+
pub(super) fn client_module_context(self: Vc<Self>) -> Vc<ModuleAssetContext> {
372+
ModuleAssetContext::new(
373+
self.client_transitions(),
359374
self.project().client_compile_time_info(),
360375
self.client_module_options_context(),
361376
self.client_resolve_options_context(),
362377
client_layer(),
363-
))
378+
)
364379
}
365380

366381
#[turbo_tasks::function]
367382
pub(super) fn ssr_module_context(self: Vc<Self>) -> Vc<ModuleAssetContext> {
368383
ModuleAssetContext::new(
369-
self.transitions(),
384+
self.server_transitions(),
370385
self.project().server_compile_time_info(),
371386
self.ssr_module_options_context(),
372387
self.ssr_resolve_options_context(),
@@ -379,7 +394,7 @@ impl PagesProject {
379394
#[turbo_tasks::function]
380395
pub(super) fn api_module_context(self: Vc<Self>) -> Vc<ModuleAssetContext> {
381396
ModuleAssetContext::new(
382-
self.transitions(),
397+
self.server_transitions(),
383398
self.project().server_compile_time_info(),
384399
self.api_module_options_context(),
385400
self.ssr_resolve_options_context(),
@@ -390,7 +405,7 @@ impl PagesProject {
390405
#[turbo_tasks::function]
391406
pub(super) fn ssr_data_module_context(self: Vc<Self>) -> Vc<ModuleAssetContext> {
392407
ModuleAssetContext::new(
393-
self.transitions(),
408+
self.server_transitions(),
394409
self.project().server_compile_time_info(),
395410
self.ssr_data_module_options_context(),
396411
self.ssr_resolve_options_context(),
@@ -566,7 +581,7 @@ impl PagesProject {
566581
self.project().next_config(),
567582
self.project().execution_context(),
568583
);
569-
Ok(client_runtime_entries.resolve_entries(self.client_module_context()))
584+
Ok(client_runtime_entries.resolve_entries(Vc::upcast(self.client_module_context())))
570585
}
571586

572587
#[turbo_tasks::function]
@@ -615,7 +630,7 @@ impl PagesProject {
615630

616631
#[turbo_tasks::function]
617632
pub async fn client_main_module(self: Vc<Self>) -> Result<Vc<Box<dyn Module>>> {
618-
let client_module_context = self.client_module_context();
633+
let client_module_context = Vc::upcast(self.client_module_context());
619634

620635
let client_main_module = esm_resolve(
621636
Vc::upcast(PlainResolveOrigin::new(
@@ -722,7 +737,7 @@ impl PageEndpoint {
722737
async fn client_module(self: Vc<Self>) -> Result<Vc<Box<dyn Module>>> {
723738
let this = self.await?;
724739
let page_loader = create_page_loader_entry_module(
725-
this.pages_project.client_module_context(),
740+
Vc::upcast(this.pages_project.client_module_context()),
726741
self.source(),
727742
*this.pathname,
728743
);

crates/next-core/src/next_client_reference/ecmascript_client_reference/ecmascript_client_reference_module.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ impl EcmascriptClientReferenceModule {
179179

180180
#[turbo_tasks::function]
181181
fn client_reference_modifier() -> Vc<RcStr> {
182-
Vc::cell("client reference/proxy".into())
182+
Vc::cell("client reference proxy".into())
183183
}
184184

185185
#[turbo_tasks::function]
@@ -200,7 +200,9 @@ pub static ECMASCRIPT_CLIENT_REFERENCE_MERGE_TAG_SSR: Lazy<RcStr> = Lazy::new(||
200200
impl Module for EcmascriptClientReferenceModule {
201201
#[turbo_tasks::function]
202202
fn ident(&self) -> Vc<AssetIdent> {
203-
self.server_ident.with_modifier(client_reference_modifier())
203+
self.server_ident
204+
.with_modifier(client_reference_modifier())
205+
.with_layer(self.server_asset_context.layer())
204206
}
205207

206208
#[turbo_tasks::function]

turbopack/crates/turbopack/src/transition/context_transition.rs

Lines changed: 0 additions & 66 deletions
This file was deleted.

turbopack/crates/turbopack/src/transition/mod.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
1-
pub(crate) mod context_transition;
21
pub(crate) mod full_context_transition;
32

43
use anyhow::Result;
5-
pub use context_transition::ContextTransition;
64
pub use full_context_transition::FullContextTransition;
75
use rustc_hash::FxHashMap;
86
use turbo_rcstr::RcStr;
@@ -57,6 +55,14 @@ pub trait Transition {
5755
resolve_options_context
5856
}
5957

58+
/// Apply modifications/wrapping to the transition options
59+
fn process_transition_options(
60+
self: Vc<Self>,
61+
transition_options: Vc<TransitionOptions>,
62+
) -> Vc<TransitionOptions> {
63+
transition_options
64+
}
65+
6066
/// Apply modifications/wrapping to the final asset
6167
fn process_module(
6268
self: Vc<Self>,
@@ -79,8 +85,9 @@ pub trait Transition {
7985
let resolve_options_context =
8086
self.process_resolve_options_context(*module_asset_context.resolve_options_context);
8187
let layer = self.process_layer(*module_asset_context.layer);
88+
let transition_options = self.process_transition_options(*module_asset_context.transitions);
8289
let module_asset_context = ModuleAssetContext::new(
83-
*module_asset_context.transitions,
90+
transition_options,
8491
compile_time_info,
8592
module_options_context,
8693
resolve_options_context,

0 commit comments

Comments
 (0)