Skip to content

Commit

Permalink
Turbopack: allow specified AssetIdents for naming chunk groups (#68917)
Browse files Browse the repository at this point in the history
Previously, in a number of situations, we would implicitly use the entry
module’s ident to name the chunk group. This adds the ability to specify
a different name.

Now, for evaluated chunk groups for pages, we use the page source’s
ident. This prevents changes in loader chunk groups from changing the
chunk name, which breaks HMR.

We still use the entry module’s ident for most other cases now, but it
is now explicitly provided.

Test Plan: `TURBOPACK=1 pnpm test-dev
test/development/acceptance/ReactRefreshModule.test.ts` with #68698
applied

---------

Co-authored-by: Tobias Koppers <[email protected]>
  • Loading branch information
wbinnssmith and sokra authored Sep 9, 2024
1 parent f9148a2 commit fcde8fc
Show file tree
Hide file tree
Showing 48 changed files with 303 additions and 69 deletions.
25 changes: 20 additions & 5 deletions crates/next-api/src/pages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use indexmap::IndexMap;
use next_core::{
all_assets_from_entries, create_page_loader_entry_module, get_asset_path_from_pathname,
get_edge_resolve_options_context,
hmr_entry::HmrEntryModule,
mode::NextMode,
next_client::{
get_client_module_options_context, get_client_resolve_options_context,
Expand Down Expand Up @@ -45,6 +46,7 @@ use turbopack_core::{
},
context::AssetContext,
file_source::FileSource,
ident::AssetIdent,
issue::IssueSeverity,
module::{Module, Modules},
output::{OutputAsset, OutputAssets},
Expand Down Expand Up @@ -644,11 +646,24 @@ impl PageEndpoint {
#[turbo_tasks::function]
async fn client_module(self: Vc<Self>) -> Result<Vc<Box<dyn Module>>> {
let this = self.await?;
Ok(create_page_loader_entry_module(
let page_loader = create_page_loader_entry_module(
this.pages_project.client_module_context(),
self.source(),
this.pathname,
))
);
if matches!(
*this.pages_project.project().next_mode().await?,
NextMode::Development
) {
if let Some(chunkable) = Vc::try_resolve_downcast(page_loader).await? {
return Ok(Vc::upcast(HmrEntryModule::new(
AssetIdent::from_path(this.page.await?.base_path),
chunkable,
true,
)));
}
}
Ok(page_loader)
}

#[turbo_tasks::function]
Expand All @@ -662,19 +677,19 @@ impl PageEndpoint {
let Some(client_module) =
Vc::try_resolve_sidecast::<Box<dyn EvaluatableAsset>>(client_module).await?
else {
bail!("expected an ECMAScript module asset");
bail!("expected an evaluateable asset");
};

let Some(client_main_module) =
Vc::try_resolve_sidecast::<Box<dyn EvaluatableAsset>>(client_main_module).await?
else {
bail!("expected an ECMAScript module asset");
bail!("expected an evaluateable asset");
};

let client_chunking_context = this.pages_project.project().client_chunking_context();

let client_chunks = client_chunking_context.evaluated_chunk_group_assets(
client_module.ident(),
AssetIdent::from_path(this.page.await?.base_path),
this.pages_project
.client_runtime_entries()
.with_entry(client_main_module)
Expand Down
216 changes: 216 additions & 0 deletions crates/next-core/src/hmr_entry.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
use std::io::Write;

use anyhow::Result;
use turbo_tasks::{RcStr, ValueToString, Vc};
use turbo_tasks_fs::{glob::Glob, rope::RopeBuilder};
use turbopack_core::{
asset::{Asset, AssetContent},
chunk::{
ChunkItem, ChunkType, ChunkableModule, ChunkableModuleReference, ChunkingContext,
EvaluatableAsset,
},
ident::AssetIdent,
module::Module,
reference::{ModuleReference, ModuleReferences},
resolve::ModuleResolveResult,
};
use turbopack_ecmascript::{
chunk::{
EcmascriptChunkItem, EcmascriptChunkItemContent, EcmascriptChunkItemOptions,
EcmascriptChunkPlaceable, EcmascriptChunkType, EcmascriptExports,
},
utils::StringifyJs,
};

#[turbo_tasks::function]
fn modifier() -> Vc<RcStr> {
Vc::cell("hmr-entry".into())
}

#[turbo_tasks::value(shared)]
pub struct HmrEntryModule {
pub ident: Vc<AssetIdent>,
pub module: Vc<Box<dyn ChunkableModule>>,
pub force_reload: bool,
}

#[turbo_tasks::value_impl]
impl HmrEntryModule {
#[turbo_tasks::function]
pub fn new(
ident: Vc<AssetIdent>,
module: Vc<Box<dyn ChunkableModule>>,
force_reload: bool,
) -> Vc<Self> {
Self {
ident,
module,
force_reload,
}
.cell()
}
}

#[turbo_tasks::value_impl]
impl Module for HmrEntryModule {
#[turbo_tasks::function]
fn ident(&self) -> Vc<AssetIdent> {
self.ident.with_modifier(modifier())
}

#[turbo_tasks::function]
fn references(&self) -> Vc<ModuleReferences> {
Vc::cell(vec![Vc::upcast(HmrEntryModuleReference::new(Vc::upcast(
self.module,
)))])
}
}

#[turbo_tasks::value_impl]
impl ChunkableModule for HmrEntryModule {
#[turbo_tasks::function]
fn as_chunk_item(
self: Vc<Self>,
chunking_context: Vc<Box<dyn ChunkingContext>>,
) -> Vc<Box<dyn ChunkItem>> {
Vc::upcast(
HmrEntryChunkItem {
module: self,
chunking_context,
}
.cell(),
)
}
}

#[turbo_tasks::value_impl]
impl Asset for HmrEntryModule {
#[turbo_tasks::function]
fn content(self: Vc<Self>) -> Vc<AssetContent> {
todo!("HmrEntryModule doesn't implement content()")
}
}

#[turbo_tasks::value_impl]
impl EcmascriptChunkPlaceable for HmrEntryModule {
#[turbo_tasks::function]
fn get_exports(self: Vc<Self>) -> Vc<EcmascriptExports> {
EcmascriptExports::None.cell()
}

#[turbo_tasks::function]
fn is_marked_as_side_effect_free(self: Vc<Self>, _: Vc<Glob>) -> Vc<bool> {
Vc::cell(false)
}
}

#[turbo_tasks::value_impl]
impl EvaluatableAsset for HmrEntryModule {}

#[turbo_tasks::value]
pub struct HmrEntryModuleReference {
pub module: Vc<Box<dyn Module>>,
}

#[turbo_tasks::value_impl]
impl HmrEntryModuleReference {
#[turbo_tasks::function]
pub fn new(module: Vc<Box<dyn Module>>) -> Vc<Self> {
HmrEntryModuleReference { module }.cell()
}
}

#[turbo_tasks::value_impl]
impl ValueToString for HmrEntryModuleReference {
#[turbo_tasks::function]
fn to_string(&self) -> Vc<RcStr> {
Vc::cell("entry".into())
}
}

#[turbo_tasks::value_impl]
impl ModuleReference for HmrEntryModuleReference {
#[turbo_tasks::function]
async fn resolve_reference(&self) -> Result<Vc<ModuleResolveResult>> {
Ok(ModuleResolveResult::module(self.module).cell())
}
}

#[turbo_tasks::value_impl]
impl ChunkableModuleReference for HmrEntryModuleReference {}

/// The chunk item for [`HmrEntryModule`].
#[turbo_tasks::value]
struct HmrEntryChunkItem {
module: Vc<HmrEntryModule>,
chunking_context: Vc<Box<dyn ChunkingContext>>,
}

#[turbo_tasks::value_impl]
impl ChunkItem for HmrEntryChunkItem {
#[turbo_tasks::function]
fn chunking_context(&self) -> Vc<Box<dyn ChunkingContext>> {
Vc::upcast(self.chunking_context)
}

#[turbo_tasks::function]
fn asset_ident(&self) -> Vc<AssetIdent> {
self.module.ident()
}

#[turbo_tasks::function]
fn references(&self) -> Vc<ModuleReferences> {
self.module.references()
}

#[turbo_tasks::function]
fn ty(&self) -> Vc<Box<dyn ChunkType>> {
Vc::upcast(Vc::<EcmascriptChunkType>::default())
}

#[turbo_tasks::function]
fn module(&self) -> Vc<Box<dyn Module>> {
Vc::upcast(self.module)
}
}

#[turbo_tasks::value_impl]
impl EcmascriptChunkItem for HmrEntryChunkItem {
#[turbo_tasks::function]
fn chunking_context(&self) -> Vc<Box<dyn ChunkingContext>> {
self.chunking_context
}

#[turbo_tasks::function]
async fn content(&self) -> Result<Vc<EcmascriptChunkItemContent>> {
let this = self.module.await?;
let module = this.module;
let chunk_item = module.as_chunk_item(self.chunking_context);
let id = self.chunking_context.chunk_item_id(chunk_item).await?;

let mut code = RopeBuilder::default();
if this.force_reload {
writeln!(
code,
"__turbopack_require__({});\nmodule.hot.dispose(() => window.location.reload());",
StringifyJs(&id)
)?;
} else {
writeln!(
code,
"__turbopack_require__({});\nmodule.hot.accept();",
StringifyJs(&id)
)?;
}
Ok(EcmascriptChunkItemContent {
inner_code: code.build(),
options: EcmascriptChunkItemOptions {
strict: true,
module: true,
..Default::default()
},
..Default::default()
}
.cell())
}
}
1 change: 1 addition & 0 deletions crates/next-core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub mod app_structure;
mod bootstrap;
mod embed_js;
mod emit;
pub mod hmr_entry;
pub mod instrumentation;
mod loader_tree;
pub mod middleware;
Expand Down
2 changes: 2 additions & 0 deletions crates/next-core/src/next_app/app_client_references_chunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ pub async fn get_app_client_references_chunks(
ssr_modules,
);
ssr_chunking_context.chunk_group(
ssr_entry_module.ident(),
Vc::upcast(ssr_entry_module),
Value::new(current_ssr_availability_info),
)
Expand Down Expand Up @@ -241,6 +242,7 @@ pub async fn get_app_client_references_chunks(
client_modules,
);
Some(client_chunking_context.chunk_group(
client_entry_module.ident(),
Vc::upcast(client_entry_module),
Value::new(current_client_availability_info),
))
Expand Down
8 changes: 3 additions & 5 deletions turbopack/crates/turbopack-browser/src/chunking_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,13 +352,11 @@ impl ChunkingContext for BrowserChunkingContext {
#[turbo_tasks::function]
async fn chunk_group(
self: Vc<Self>,
ident: Vc<AssetIdent>,
module: Vc<Box<dyn ChunkableModule>>,
availability_info: Value<AvailabilityInfo>,
) -> Result<Vc<ChunkGroupResult>> {
let span = tracing::info_span!(
"chunking",
module = module.ident().to_string().await?.to_string()
);
let span = tracing::info_span!("chunking", ident = ident.to_string().await?.to_string());
async move {
let this = self.await?;
let input_availability_info = availability_info.into_value();
Expand All @@ -378,7 +376,7 @@ impl ChunkingContext for BrowserChunkingContext {
.collect();

if this.enable_hot_module_replacement {
let mut ident = module.ident();
let mut ident = ident;
match input_availability_info {
AvailabilityInfo::Root => {}
AvailabilityInfo::Untracked => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use turbopack_core::{
asset::{Asset, AssetContent},
chunk::{ChunkingContext, EvaluatableAssets},
ident::AssetIdent,
module::Module,
output::{OutputAsset, OutputAssets},
version::VersionedContent,
};
Expand Down Expand Up @@ -92,10 +91,6 @@ impl OutputAsset for EcmascriptDevChunkList {
#[turbo_tasks::function]
async fn ident(&self) -> Result<Vc<AssetIdent>> {
let mut ident = self.ident.await?.clone_value();
for &evaluatable_asset in self.evaluatable_assets.await?.iter() {
ident.add_asset(Vc::<RcStr>::default(), evaluatable_asset.ident());
}

ident.add_modifier(modifier());

match self.source {
Expand Down
5 changes: 3 additions & 2 deletions turbopack/crates/turbopack-core/src/chunk/chunking_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ pub trait ChunkingContext {

fn chunk_group(
self: Vc<Self>,
ident: Vc<AssetIdent>,
module: Vc<Box<dyn ChunkableModule>>,
availability_info: Value<AvailabilityInfo>,
) -> Vc<ChunkGroupResult>;
Expand Down Expand Up @@ -189,7 +190,7 @@ impl<T: ChunkingContext + Send + Upcast<Box<dyn ChunkingContext>>> ChunkingConte
self: Vc<Self>,
module: Vc<Box<dyn ChunkableModule>>,
) -> Vc<ChunkGroupResult> {
self.chunk_group(module, Value::new(AvailabilityInfo::Root))
self.chunk_group(module.ident(), module, Value::new(AvailabilityInfo::Root))
}

fn root_chunk_group_assets(
Expand Down Expand Up @@ -309,7 +310,7 @@ async fn chunk_group_assets(
availability_info: Value<AvailabilityInfo>,
) -> Result<Vc<OutputAssets>> {
Ok(chunking_context
.chunk_group(module, availability_info)
.chunk_group(module.ident(), module, availability_info)
.await?
.assets)
}
7 changes: 6 additions & 1 deletion turbopack/crates/turbopack-core/src/ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,12 @@ impl ValueToString for AssetIdent {
}

if let Some(part) = self.part {
write!(s, " <{}>", part.to_string().await?)?;
let part = part.to_string().await?;
// facade is not included in ident as switching between facade and non-facade shouldn't
// change the ident
if part.as_str() != "facade" {
write!(s, " <{}>", part)?;
}
}

Ok(Vc::cell(s.into()))
Expand Down
1 change: 1 addition & 0 deletions turbopack/crates/turbopack-nodejs/src/chunking_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ impl ChunkingContext for NodeJsChunkingContext {
#[turbo_tasks::function]
async fn chunk_group(
self: Vc<Self>,
_ident: Vc<AssetIdent>,
module: Vc<Box<dyn ChunkableModule>>,
availability_info: Value<AvailabilityInfo>,
) -> Result<Vc<ChunkGroupResult>> {
Expand Down
Loading

0 comments on commit fcde8fc

Please sign in to comment.