Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove ChunkingType::Separate #5419

Merged
merged 3 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 2 additions & 25 deletions crates/turbopack-core/src/chunk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,9 @@ pub enum ChunkingType {
/// Referenced asset will not inherit the available modules, but form a
/// new availability root.
IsolatedParallel,
/// Asset is placed in a separate chunk group that is referenced from the
/// referencing chunk group, but not loaded.
/// Note: Separate chunks need to be loaded by something external to current
/// reference.
Separate,
/// An async loader is placed into the referencing chunk and loads the
/// separate chunk group in which the asset is placed.
SeparateAsync,
Async,
}

#[turbo_tasks::value(transparent)]
Expand Down Expand Up @@ -239,7 +234,6 @@ impl ValueToString for ChunkGroupReference {
pub struct ChunkContentResult<I> {
pub chunk_items: Vec<I>,
pub chunks: Vec<ChunkVc>,
pub async_chunk_group_entries: Vec<ChunkVc>,
pub external_asset_references: Vec<AssetReferenceVc>,
pub availability_info: AvailabilityInfo,
}
Expand Down Expand Up @@ -289,9 +283,6 @@ enum ChunkContentGraphNode<I> {
AvailableAsset(AssetVc),
// Chunks that are loaded in parallel to the current chunk
Chunk(ChunkVc),
// Chunk groups that are referenced from the current chunk, but
// not loaded in parallel
AsyncChunkGroup { entry: ChunkVc },
ExternalAssetReference(AssetReferenceVc),
}

Expand Down Expand Up @@ -414,16 +405,7 @@ where
ChunkContentGraphNode::Chunk(chunk),
));
}
ChunkingType::Separate => {
graph_nodes.push((
Some((asset, chunking_type)),
ChunkContentGraphNode::AsyncChunkGroup {
entry: chunkable_asset
.as_chunk(context.chunking_context, context.availability_info),
},
));
}
ChunkingType::SeparateAsync => {
ChunkingType::Async => {
if let Some(manifest_loader_item) = I::from_async_asset(
context.chunking_context,
chunkable_asset,
Expand Down Expand Up @@ -594,7 +576,6 @@ where

let mut chunk_items = Vec::new();
let mut chunks = Vec::new();
let mut async_chunk_group_entries = Vec::new();
let mut external_asset_references = Vec::new();

for graph_node in graph_nodes {
Expand All @@ -606,9 +587,6 @@ where
ChunkContentGraphNode::Chunk(chunk) => {
chunks.push(chunk);
}
ChunkContentGraphNode::AsyncChunkGroup { entry } => {
async_chunk_group_entries.push(entry);
}
ChunkContentGraphNode::ExternalAssetReference(reference) => {
external_asset_references.push(reference);
}
Expand All @@ -618,7 +596,6 @@ where
Ok(Some(ChunkContentResult {
chunk_items,
chunks,
async_chunk_group_entries,
external_asset_references,
availability_info: availability_info.into_value(),
}))
Expand Down
8 changes: 1 addition & 7 deletions crates/turbopack-core/src/introspect/asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,6 @@ fn isolated_parallel_reference_ty() -> StringVc {
StringVc::cell("isolated parallel reference".to_string())
}

#[turbo_tasks::function]
fn separate_reference_ty() -> StringVc {
StringVc::cell("separate reference".to_string())
}

#[turbo_tasks::function]
fn async_reference_ty() -> StringVc {
StringVc::cell("async reference".to_string())
Expand Down Expand Up @@ -121,9 +116,8 @@ pub async fn children_from_asset_references(
Some(ChunkingType::Placed) => key = placed_reference_ty(),
Some(ChunkingType::Parallel) => key = parallel_reference_ty(),
Some(ChunkingType::IsolatedParallel) => key = isolated_parallel_reference_ty(),
Some(ChunkingType::Separate) => key = separate_reference_ty(),
Some(ChunkingType::PlacedOrParallel) => key = placed_or_parallel_reference_ty(),
Some(ChunkingType::SeparateAsync) => key = async_reference_ty(),
Some(ChunkingType::Async) => key = async_reference_ty(),
}
}

Expand Down
16 changes: 3 additions & 13 deletions crates/turbopack-css/src/chunk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,9 @@ use turbopack_core::{
asset::{Asset, AssetContentVc, AssetVc, AssetsVc},
chunk::{
availability_info::AvailabilityInfo, chunk_content, chunk_content_split, Chunk,
ChunkContentResult, ChunkGroupReferenceVc, ChunkItem, ChunkItemVc, ChunkVc,
ChunkableAssetVc, ChunkingContext, ChunkingContextVc, ChunksVc, FromChunkableAsset,
ModuleId, ModuleIdVc, ModuleIdsVc, OutputChunk, OutputChunkRuntimeInfo,
OutputChunkRuntimeInfoVc, OutputChunkVc,
ChunkContentResult, ChunkItem, ChunkItemVc, ChunkVc, ChunkableAssetVc, ChunkingContext,
ChunkingContextVc, ChunksVc, FromChunkableAsset, ModuleId, ModuleIdVc, ModuleIdsVc,
OutputChunk, OutputChunkRuntimeInfo, OutputChunkRuntimeInfoVc, OutputChunkVc,
},
code_builder::{CodeBuilder, CodeVc},
ident::{AssetIdent, AssetIdentVc},
Expand Down Expand Up @@ -202,7 +201,6 @@ impl GenerateSourceMap for CssChunkContent {
pub struct CssChunkContentResult {
pub chunk_items: Vec<CssChunkItemVc>,
pub chunks: Vec<ChunkVc>,
pub async_chunk_group_entries: Vec<ChunkVc>,
pub external_asset_references: Vec<AssetReferenceVc>,
}

Expand All @@ -211,7 +209,6 @@ impl From<ChunkContentResult<CssChunkItemVc>> for CssChunkContentResult {
CssChunkContentResult {
chunk_items: from.chunk_items,
chunks: from.chunks,
async_chunk_group_entries: from.async_chunk_group_entries,
external_asset_references: from.external_asset_references,
}
}
Expand All @@ -236,26 +233,22 @@ async fn css_chunk_content(

let mut all_chunk_items = IndexSet::<CssChunkItemVc>::new();
let mut all_chunks = IndexSet::<ChunkVc>::new();
let mut all_async_chunk_group_entries = IndexSet::<ChunkVc>::new();
let mut all_external_asset_references = IndexSet::<AssetReferenceVc>::new();

for content in contents {
let CssChunkContentResult {
chunk_items,
chunks,
async_chunk_group_entries,
external_asset_references,
} = &*content.await?;
all_chunk_items.extend(chunk_items.iter().copied());
all_chunks.extend(chunks.iter().copied());
all_async_chunk_group_entries.extend(async_chunk_group_entries.iter().copied());
all_external_asset_references.extend(external_asset_references.iter().copied());
}

Ok(CssChunkContentResult {
chunk_items: all_chunk_items.into_iter().collect(),
chunks: all_chunks.into_iter().collect(),
async_chunk_group_entries: all_async_chunk_group_entries.into_iter().collect(),
external_asset_references: all_external_asset_references.into_iter().collect(),
}
.cell())
Expand Down Expand Up @@ -417,9 +410,6 @@ impl Asset for CssChunk {
}
}
}
for entry in content.async_chunk_group_entries.iter() {
references.push(ChunkGroupReferenceVc::new(this.context, *entry).into());
}
for item in content.chunk_items.iter() {
references.push(SingleItemCssChunkReferenceVc::new(this.context, *item).into());
}
Expand Down
13 changes: 1 addition & 12 deletions crates/turbopack-css/src/chunk/single_item_chunk/reference.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use anyhow::Result;
use turbo_tasks::{primitives::StringVc, ValueToString, ValueToStringVc};
use turbopack_core::{
chunk::{
ChunkItem, ChunkableAssetReference, ChunkableAssetReferenceVc, ChunkingContextVc,
ChunkingType, ChunkingTypeOptionVc,
},
chunk::{ChunkItem, ChunkingContextVc},
reference::{AssetReference, AssetReferenceVc},
resolve::{ResolveResult, ResolveResultVc},
};
Expand Down Expand Up @@ -47,11 +44,3 @@ impl ValueToString for SingleItemCssChunkReference {
)))
}
}

#[turbo_tasks::value_impl]
impl ChunkableAssetReference for SingleItemCssChunkReference {
#[turbo_tasks::function]
fn chunking_type(&self) -> Result<ChunkingTypeOptionVc> {
Ok(ChunkingTypeOptionVc::cell(Some(ChunkingType::Separate)))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you verified that this still produces a single CSS chunk for each source CSS asset?

Copy link
Member Author

@sokra sokra Jun 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't, I guess the tests do that.

But the SingleItemCssChunkReference is used in references from CssChunk, which is in the output graph, where ChunkableAssetReference is not used at all. ChunkableAssetReference is only used in the module graph while determining the chunking.

6 changes: 0 additions & 6 deletions crates/turbopack-ecmascript/src/chunk/content.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use super::{
pub struct EcmascriptChunkContent {
pub chunk_items: Vec<EcmascriptChunkItemVc>,
pub chunks: Vec<ChunkVc>,
pub async_chunk_group_entries: Vec<ChunkVc>,
pub external_asset_references: Vec<AssetReferenceVc>,
pub availability_info: AvailabilityInfo,
}
Expand All @@ -29,7 +28,6 @@ impl From<ChunkContentResult<EcmascriptChunkItemVc>> for EcmascriptChunkContent
EcmascriptChunkContent {
chunk_items: from.chunk_items,
chunks: from.chunks,
async_chunk_group_entries: from.async_chunk_group_entries,
external_asset_references: from.external_asset_references,
availability_info: from.availability_info,
}
Expand Down Expand Up @@ -80,27 +78,23 @@ async fn ecmascript_chunk_content_internal(

let mut all_chunk_items = IndexSet::<EcmascriptChunkItemVc>::new();
let mut all_chunks = IndexSet::<ChunkVc>::new();
let mut all_async_chunk_group_entries = IndexSet::<ChunkVc>::new();
let mut all_external_asset_references = IndexSet::<AssetReferenceVc>::new();

for content in contents {
let EcmascriptChunkContent {
chunk_items,
chunks,
async_chunk_group_entries,
external_asset_references,
availability_info: _,
} = &*content.await?;
all_chunk_items.extend(chunk_items.iter().copied());
all_chunks.extend(chunks.iter().copied());
all_async_chunk_group_entries.extend(async_chunk_group_entries.iter().copied());
all_external_asset_references.extend(external_asset_references.iter().copied());
}

Ok(EcmascriptChunkContent {
chunk_items: all_chunk_items.into_iter().collect(),
chunks: all_chunks.into_iter().collect(),
async_chunk_group_entries: all_async_chunk_group_entries.into_iter().collect(),
external_asset_references: all_external_asset_references.into_iter().collect(),
availability_info: availability_info.into_value(),
}
Expand Down
7 changes: 2 additions & 5 deletions crates/turbopack-ecmascript/src/chunk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ use turbo_tasks_fs::FileSystemPathOptionVc;
use turbopack_core::{
asset::{Asset, AssetContentVc, AssetVc},
chunk::{
availability_info::AvailabilityInfo, Chunk, ChunkGroupReferenceVc, ChunkItem, ChunkVc,
ChunkingContextVc, ChunksVc, ModuleIdsVc,
availability_info::AvailabilityInfo, Chunk, ChunkItem, ChunkVc, ChunkingContextVc,
ChunksVc, ModuleIdsVc,
},
ident::{AssetIdent, AssetIdentVc},
introspect::{
Expand Down Expand Up @@ -392,9 +392,6 @@ impl Asset for EcmascriptChunk {
for r in content.external_asset_references.iter() {
references.push(*r);
}
for entry in content.async_chunk_group_entries.iter() {
references.push(ChunkGroupReferenceVc::new(this.context.into(), *entry).into());
}

Ok(AssetReferencesVc::cell(references))
}
Expand Down
5 changes: 2 additions & 3 deletions crates/turbopack-ecmascript/src/references/esm/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ impl ChunkableAssetReference for EsmAssetReference {
Ok(ChunkingTypeOptionVc::cell(
if let Some(chunking_type) = self.annotations.chunking_type() {
match chunking_type {
"separate" => Some(ChunkingType::Separate),
"parallel" => Some(ChunkingType::Parallel),
"isolatedParallel" => Some(ChunkingType::IsolatedParallel),
"none" => None,
Expand Down Expand Up @@ -222,8 +221,8 @@ impl CodeGenerateable for EsmAssetReference {
return Ok(CodeGeneration { visitors }.into());
}

// separate chunks can't be imported as the modules are not available
if !matches!(*chunking_type, None | Some(ChunkingType::Separate)) {
// only chunked references can be imported
if chunking_type.is_some() {
let referenced_asset = self_vc.get_referenced_asset().await?;
if let Some(ident) = referenced_asset.get_ident().await? {
match &*referenced_asset {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ impl ValueToString for EsmAsyncAssetReference {
impl ChunkableAssetReference for EsmAsyncAssetReference {
#[turbo_tasks::function]
fn chunking_type(&self) -> ChunkingTypeOptionVc {
ChunkingTypeOptionVc::cell(Some(ChunkingType::SeparateAsync))
ChunkingTypeOptionVc::cell(Some(ChunkingType::Async))
}
}

Expand Down