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

fix: throw error when there had conflict between chunkName and entryPointChunk #4795

Merged
merged 1 commit into from
Nov 28, 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
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
Loading