Skip to content

Commit

Permalink
fix: throw error when there had conflict between chunkName and `ent…
Browse files Browse the repository at this point in the history
…ryPointChunk` (#4795)

fix: throw error when there had confict between `chunkName` and `entryPointChunk`
  • Loading branch information
bvanjoi authored Nov 28, 2023
1 parent 6490a91 commit 2250906
Show file tree
Hide file tree
Showing 15 changed files with 122 additions and 43 deletions.
47 changes: 37 additions & 10 deletions crates/rspack_core/src/build_chunk_graph/code_splitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use rspack_error::{internal_error, Result};
use rustc_hash::{FxHashMap as HashMap, FxHashSet as HashSet};

use super::remove_parent_modules::RemoveParentModulesContext;
use crate::dependencies_block::AsyncDependenciesToInitialChunkError;
use crate::{
get_entry_runtime, AsyncDependenciesBlockIdentifier, BoxDependency, ChunkGroup, ChunkGroupInfo,
ChunkGroupKind, ChunkGroupOptions, ChunkGroupUkey, ChunkLoading, ChunkUkey, Compilation,
Expand Down Expand Up @@ -77,11 +78,12 @@ impl<'me> CodeSplitter<'me> {
.filter_map(|dep| module_graph.module_identifier_by_dependency_id(dep))
.collect::<Vec<_>>();

let chunk = Compilation::add_named_chunk(
let chunk_ukey = Compilation::add_named_chunk(
name.to_string(),
&mut compilation.chunk_by_ukey,
&mut compilation.named_chunks,
);
let chunk = compilation.chunk_by_ukey.expect_get_mut(&chunk_ukey);
if let Some(filename) = &entry_data.options.filename {
chunk.filename_template = Some(filename.clone());
}
Expand Down Expand Up @@ -173,6 +175,12 @@ impl<'me> CodeSplitter<'me> {
.entry(entrypoint.ukey)
.or_default()
.extend(included_modules);

if let Some(name) = entrypoint.name() {
self
.named_chunk_groups
.insert(name.to_string(), entrypoint.ukey);
}
}

let mut runtime_chunks = HashSet::default();
Expand Down Expand Up @@ -211,11 +219,12 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
.ok_or_else(|| anyhow!("no chunk found"))?
}
None => {
let chunk = Compilation::add_named_chunk(
let chunk_ukey = Compilation::add_named_chunk(
runtime.to_string(),
&mut compilation.chunk_by_ukey,
&mut compilation.named_chunks,
);
let chunk = compilation.chunk_by_ukey.expect_get_mut(&chunk_ukey);
chunk.prevent_integration = true;
chunk.chunk_reasons.push(format!("RuntimeChunk({name})",));
compilation.chunk_graph.add_chunk(chunk.ukey);
Expand Down Expand Up @@ -524,7 +533,7 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
.chunk_group_by_ukey
.expect_get(&item_chunk_group_ukey);

let chunk = if let Some(chunk_name) = block.get_group_options().and_then(|x| x.name()) {
let chunk_ukey = if let Some(chunk_name) = block.get_group_options().and_then(|x| x.name()) {
Compilation::add_named_chunk(
chunk_name.to_string(),
&mut self.compilation.chunk_by_ukey,
Expand All @@ -535,8 +544,8 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
};
self
.remove_parent_modules_context
.add_chunk_relation(item_chunk_ukey, chunk.ukey);
self.compilation.chunk_graph.add_chunk(chunk.ukey);
.add_chunk_relation(item_chunk_ukey, chunk_ukey);
self.compilation.chunk_graph.add_chunk(chunk_ukey);

let entry_options = block.get_group_options().and_then(|o| o.entry_options());
if let Some(cgi) = cgi {
Expand All @@ -556,6 +565,7 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
.connect_block_and_chunk_group(block_id, *cgi);
*cgi
} else {
let chunk = self.compilation.chunk_by_ukey.expect_get_mut(&chunk_ukey);
if let Some(filename) = &entry_options.filename {
chunk.filename_template = Some(filename.clone());
}
Expand Down Expand Up @@ -617,7 +627,7 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
.push(QueueAction::ProcessEntryBlock(ProcessEntryBlock {
block: block_id,
chunk_group: cgi,
chunk: chunk.ukey,
chunk: chunk_ukey,
}));
cgi
} else if !item_chunk_group.info.async_chunks || !item_chunk_group.info.chunk_loading {
Expand All @@ -628,14 +638,31 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
}));
return;
} else {
let cgi = if let Some(cgi) = chunk_name.and_then(|name| self.named_chunk_groups.get(name)) {
// TODO: AsyncDependencyToInitialChunkError
let cgi = if let Some(chunk_name) = chunk_name
&& let Some(cgi) = self.named_chunk_groups.get(chunk_name)
{
let mut res = *cgi;
if self
.compilation
.chunk_group_by_ukey
.expect_get(cgi)
.is_initial()
{
let error = AsyncDependenciesToInitialChunkError {
chunk_name,
loc: block.loc(),
};
self.compilation.push_diagnostic(error.into());
res = item_chunk_group_ukey;
}

self
.compilation
.chunk_graph
.connect_block_and_chunk_group(block_id, *cgi);
*cgi
res
} else {
let chunk = self.compilation.chunk_by_ukey.expect_get_mut(&chunk_ukey);
chunk
.chunk_reasons
.push(format!("DynamicImport({:?})", block_id));
Expand Down Expand Up @@ -690,7 +717,7 @@ Or do you want to use the entrypoints '{name}' and '{runtime}' independently on
.push(QueueAction::ProcessBlock(ProcessBlock {
block: block_id.into(),
chunk_group: c,
chunk: chunk.ukey,
chunk: chunk_ukey,
}));
} else if let Some(entrypoint) = entrypoint {
let item_chunk_group = self
Expand Down
19 changes: 9 additions & 10 deletions crates/rspack_core/src/compiler/compilation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,30 +346,29 @@ impl Compilation {
Stats::new(self)
}

pub fn add_named_chunk<'chunk>(
pub fn add_named_chunk(
name: String,
chunk_by_ukey: &'chunk mut ChunkByUkey,
chunk_by_ukey: &mut ChunkByUkey,
named_chunks: &mut HashMap<String, ChunkUkey>,
) -> &'chunk mut Chunk {
) -> ChunkUkey {
let existed_chunk_ukey = named_chunks.get(&name);
if let Some(chunk_ukey) = existed_chunk_ukey {
let chunk = chunk_by_ukey
.get_mut(chunk_ukey)
.expect("This should not happen");
chunk
assert!(chunk_by_ukey.contains(chunk_ukey));
*chunk_ukey
} else {
let chunk = Chunk::new(Some(name.clone()), ChunkKind::Normal);
let ukey = chunk.ukey;
named_chunks.insert(name, chunk.ukey);
chunk_by_ukey.entry(ukey).or_insert_with(|| chunk)
chunk_by_ukey.entry(ukey).or_insert_with(|| chunk);
ukey
}
}

pub fn add_chunk(chunk_by_ukey: &mut ChunkByUkey) -> &mut Chunk {
pub fn add_chunk(chunk_by_ukey: &mut ChunkByUkey) -> ChunkUkey {
let chunk = Chunk::new(None, ChunkKind::Normal);
let ukey = chunk.ukey;
chunk_by_ukey.add(chunk);
chunk_by_ukey.get_mut(&ukey).expect("chunk not found")
ukey
}

#[instrument(name = "compilation:make", skip_all)]
Expand Down
9 changes: 6 additions & 3 deletions crates/rspack_core/src/context_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ impl ContextModule {
&& !context_element_dependencies.is_empty()
{
let name = self.options.context_options.chunk_name.clone();
let mut block = AsyncDependenciesBlock::new(self.identifier, "");
let mut block = AsyncDependenciesBlock::new(self.identifier, "", None);
block.set_group_options(GroupOptions::ChunkGroup(ChunkGroupOptions { name }));
for context_element_dependency in context_element_dependencies {
block.add_dependency(Box::new(context_element_dependency));
Expand Down Expand Up @@ -811,8 +811,11 @@ impl ContextModule {
});
name.into_owned()
});
let mut block =
AsyncDependenciesBlock::new(self.identifier, &context_element_dependency.user_request);
let mut block = AsyncDependenciesBlock::new(
self.identifier,
&context_element_dependency.user_request,
None,
);
block.set_group_options(GroupOptions::ChunkGroup(ChunkGroupOptions { name }));
block.add_dependency(Box::new(context_element_dependency));
blocks.push(block);
Expand Down
50 changes: 49 additions & 1 deletion crates/rspack_core/src/dependencies_block.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use rspack_error::{Diagnostic, DIAGNOSTIC_POS_DUMMY};
use serde::Serialize;
use ustr::Ustr;

Expand Down Expand Up @@ -36,6 +37,26 @@ impl AsyncDependenciesBlockIdentifier {
}
}

#[derive(Debug, Clone, Copy)]
pub struct DependencyLocation {
start: u32,
end: u32,
}

impl DependencyLocation {
pub fn new(start: u32, end: u32) -> Self {
Self { start, end }
}

pub fn start(&self) -> u32 {
self.start
}

pub fn end(&self) -> u32 {
self.end
}
}

#[derive(Debug, Clone)]
pub struct AsyncDependenciesBlock {
id: AsyncDependenciesBlockIdentifier,
Expand All @@ -44,18 +65,24 @@ pub struct AsyncDependenciesBlock {
block_ids: Vec<AsyncDependenciesBlockIdentifier>,
dependency_ids: Vec<DependencyId>,
dependencies: Vec<BoxDependency>,
loc: Option<DependencyLocation>,
}

impl AsyncDependenciesBlock {
/// modifier should be Dependency.span in most of time
pub fn new(from: ModuleIdentifier, modifier: impl AsRef<str>) -> Self {
pub fn new(
from: ModuleIdentifier,
modifier: impl AsRef<str>,
loc: Option<DependencyLocation>,
) -> Self {
Self {
id: AsyncDependenciesBlockIdentifier::new(from, modifier.as_ref().into()),
group_options: Default::default(),
blocks: Default::default(),
block_ids: Default::default(),
dependency_ids: Default::default(),
dependencies: Default::default(),
loc,
}
}
}
Expand Down Expand Up @@ -104,6 +131,10 @@ impl AsyncDependenciesBlock {
pub fn take_blocks(&mut self) -> Vec<AsyncDependenciesBlock> {
std::mem::take(&mut self.blocks)
}

pub fn loc(&self) -> Option<&DependencyLocation> {
self.loc.as_ref()
}
}

impl DependenciesBlock for AsyncDependenciesBlock {
Expand All @@ -124,3 +155,20 @@ impl DependenciesBlock for AsyncDependenciesBlock {
&self.dependency_ids
}
}

pub struct AsyncDependenciesToInitialChunkError<'a> {
pub chunk_name: &'a str,
pub loc: Option<&'a DependencyLocation>,
}

impl<'a> From<AsyncDependenciesToInitialChunkError<'a>> for Diagnostic {
fn from(value: AsyncDependenciesToInitialChunkError<'a>) -> Self {
let title = "AsyncDependencyToInitialChunkError".to_string();
let message = format!("It's not allowed to load an initial chunk on demand. The chunk name \"{}\" is already used by an entrypoint.", value.chunk_name);
let (start, end) = value
.loc
.map(|loc| (loc.start as usize, loc.end as usize))
.unwrap_or((DIAGNOSTIC_POS_DUMMY, DIAGNOSTIC_POS_DUMMY));
Diagnostic::error(title, message, start, end)
}
}
2 changes: 1 addition & 1 deletion crates/rspack_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::{fmt, sync::Arc};
mod dependencies_block;
pub mod mf;
pub use dependencies_block::{
AsyncDependenciesBlock, AsyncDependenciesBlockIdentifier, DependenciesBlock,
AsyncDependenciesBlock, AsyncDependenciesBlockIdentifier, DependenciesBlock, DependencyLocation,
};
mod fake_namespace_object;
pub use fake_namespace_object::*;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ impl Module for ContainerEntryModule {

let mut blocks = vec![];
for (name, options) in &self.exposes {
let mut block = AsyncDependenciesBlock::new(self.identifier, name);
let mut block = AsyncDependenciesBlock::new(self.identifier, name, None);
block.set_group_options(GroupOptions::ChunkGroup(ChunkGroupOptions {
name: options.name.clone(),
}));
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/mf/sharing/consume_shared_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl Module for ConsumeSharedModule {
if self.options.eager {
dependencies.push(dep as BoxDependency);
} else {
let mut block = AsyncDependenciesBlock::new(self.identifier, "");
let mut block = AsyncDependenciesBlock::new(self.identifier, "", None);
block.add_dependency(dep);
blocks.push(block);
}
Expand Down
2 changes: 1 addition & 1 deletion crates/rspack_core/src/mf/sharing/provide_shared_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl Module for ProvideSharedModule {
if self.eager {
dependencies.push(dep as BoxDependency);
} else {
let mut block = AsyncDependenciesBlock::new(self.identifier, "");
let mut block = AsyncDependenciesBlock::new(self.identifier, "", None);
block.add_dependency(dep);
blocks.push(block);
}
Expand Down
3 changes: 3 additions & 0 deletions crates/rspack_error/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,9 @@ impl From<Error> for Vec<Diagnostic> {
vec![diagnostic]
}
}

pub fn errors_to_diagnostics(errs: Vec<Error>) -> Vec<Diagnostic> {
errs.into_iter().flat_map(Vec::<Diagnostic>::from).collect()
}

pub const DIAGNOSTIC_POS_DUMMY: usize = 0;
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use once_cell::sync::Lazy;
use rspack_core::{
clean_regexp_in_context_module, context_reg_exp, AsyncDependenciesBlock, DynamicImportMode,
ErrorSpan, GroupOptions, JavascriptParserOptions, ModuleIdentifier,
clean_regexp_in_context_module, context_reg_exp, AsyncDependenciesBlock, DependencyLocation,
DynamicImportMode, ErrorSpan, GroupOptions, JavascriptParserOptions, ModuleIdentifier,
};
use rspack_core::{BoxDependency, BuildMeta, ChunkGroupOptions, ContextMode};
use rspack_core::{ContextNameSpaceObject, ContextOptions, DependencyCategory, SpanExt};
Expand Down Expand Up @@ -211,6 +211,7 @@ impl Visit for ImportScanner<'_> {
let mut block = AsyncDependenciesBlock::new(
self.module_identifier,
format!("{}:{}", span.start, span.end),
Some(DependencyLocation::new(span.start, span.end)),
);
block.set_group_options(GroupOptions::ChunkGroup(
ChunkGroupOptions::default().name_optional(chunk_name),
Expand Down Expand Up @@ -239,6 +240,7 @@ impl Visit for ImportScanner<'_> {
let mut block = AsyncDependenciesBlock::new(
self.module_identifier,
format!("{}:{}", span.start, span.end),
Some(DependencyLocation::new(span.start, span.end)),
);
block.set_group_options(GroupOptions::ChunkGroup(
ChunkGroupOptions::default().name_optional(chunk_name),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use std::hash::Hash;

use rspack_core::{
AsyncDependenciesBlock, BoxDependency, BoxDependencyTemplate, ConstDependency, EntryOptions,
ErrorSpan, GroupOptions, ModuleIdentifier, OutputOptions, SpanExt,
AsyncDependenciesBlock, BoxDependency, BoxDependencyTemplate, ConstDependency,
DependencyLocation, EntryOptions, ErrorSpan, GroupOptions, ModuleIdentifier, OutputOptions,
SpanExt,
};
use rspack_hash::RspackHash;
use swc_core::common::Spanned;
Expand Down Expand Up @@ -69,6 +70,7 @@ impl<'a> WorkerScanner<'a> {
let mut block = AsyncDependenciesBlock::new(
*self.module_identifier,
format!("{}:{}", span.start, span.end),
Some(DependencyLocation::new(span.start, span.end)),
);
block.set_group_options(GroupOptions::Entrypoint(Box::new(EntryOptions {
name,
Expand Down
3 changes: 1 addition & 2 deletions crates/rspack_plugin_split_chunks/src/split_chunks_plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -790,13 +790,12 @@ impl Plugin for SplitChunksPlugin {
&mut compilation.chunk_by_ukey,
&mut compilation.named_chunks,
)
.ukey
} else {
tracing::debug!(
"create a non-named chunk for cache group {}",
item_cache_group.key
);
Compilation::add_chunk(&mut compilation.chunk_by_ukey).ukey
Compilation::add_chunk(&mut compilation.chunk_by_ukey)
}
};

Expand Down
Loading

0 comments on commit 2250906

Please sign in to comment.