From 225090688fb20a94a740a097f157fe717cfaee56 Mon Sep 17 00:00:00 2001 From: bohan Date: Tue, 28 Nov 2023 17:12:11 +0800 Subject: [PATCH] fix: throw error when there had conflict between `chunkName` and `entryPointChunk` (#4795) fix: throw error when there had confict between `chunkName` and `entryPointChunk` --- .../src/build_chunk_graph/code_splitter.rs | 47 +++++++++++++---- .../rspack_core/src/compiler/compilation.rs | 19 ++++--- crates/rspack_core/src/context_module.rs | 9 ++-- crates/rspack_core/src/dependencies_block.rs | 50 ++++++++++++++++++- crates/rspack_core/src/lib.rs | 2 +- .../mf/container/container_entry_module.rs | 2 +- .../src/mf/sharing/consume_shared_module.rs | 2 +- .../src/mf/sharing/provide_shared_module.rs | 2 +- crates/rspack_error/src/diagnostic.rs | 3 ++ .../src/visitors/dependency/import_scanner.rs | 6 ++- .../src/visitors/dependency/worker_scanner.rs | 6 ++- .../src/split_chunks_plugin.rs | 3 +- .../src/plugin/chunk.rs | 6 ++- .../src/plugin/max_size.rs | 4 +- .../weird-reference-to-entry/test.filter.js | 4 -- 15 files changed, 122 insertions(+), 43 deletions(-) delete mode 100644 webpack-test/cases/chunks/weird-reference-to-entry/test.filter.js diff --git a/crates/rspack_core/src/build_chunk_graph/code_splitter.rs b/crates/rspack_core/src/build_chunk_graph/code_splitter.rs index 333cd0d0954..36d735aeefe 100644 --- a/crates/rspack_core/src/build_chunk_graph/code_splitter.rs +++ b/crates/rspack_core/src/build_chunk_graph/code_splitter.rs @@ -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, @@ -77,11 +78,12 @@ impl<'me> CodeSplitter<'me> { .filter_map(|dep| module_graph.module_identifier_by_dependency_id(dep)) .collect::>(); - 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()); } @@ -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(); @@ -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); @@ -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, @@ -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 { @@ -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()); } @@ -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 { @@ -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)); @@ -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 diff --git a/crates/rspack_core/src/compiler/compilation.rs b/crates/rspack_core/src/compiler/compilation.rs index 957c8cff706..28a27ff550a 100644 --- a/crates/rspack_core/src/compiler/compilation.rs +++ b/crates/rspack_core/src/compiler/compilation.rs @@ -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, - ) -> &'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)] diff --git a/crates/rspack_core/src/context_module.rs b/crates/rspack_core/src/context_module.rs index 5a2d45e15f2..d065b06fd9b 100644 --- a/crates/rspack_core/src/context_module.rs +++ b/crates/rspack_core/src/context_module.rs @@ -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)); @@ -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); diff --git a/crates/rspack_core/src/dependencies_block.rs b/crates/rspack_core/src/dependencies_block.rs index dabc1b6ce9f..72f9048db41 100644 --- a/crates/rspack_core/src/dependencies_block.rs +++ b/crates/rspack_core/src/dependencies_block.rs @@ -1,3 +1,4 @@ +use rspack_error::{Diagnostic, DIAGNOSTIC_POS_DUMMY}; use serde::Serialize; use ustr::Ustr; @@ -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, @@ -44,11 +65,16 @@ pub struct AsyncDependenciesBlock { block_ids: Vec, dependency_ids: Vec, dependencies: Vec, + loc: Option, } impl AsyncDependenciesBlock { /// modifier should be Dependency.span in most of time - pub fn new(from: ModuleIdentifier, modifier: impl AsRef) -> Self { + pub fn new( + from: ModuleIdentifier, + modifier: impl AsRef, + loc: Option, + ) -> Self { Self { id: AsyncDependenciesBlockIdentifier::new(from, modifier.as_ref().into()), group_options: Default::default(), @@ -56,6 +82,7 @@ impl AsyncDependenciesBlock { block_ids: Default::default(), dependency_ids: Default::default(), dependencies: Default::default(), + loc, } } } @@ -104,6 +131,10 @@ impl AsyncDependenciesBlock { pub fn take_blocks(&mut self) -> Vec { std::mem::take(&mut self.blocks) } + + pub fn loc(&self) -> Option<&DependencyLocation> { + self.loc.as_ref() + } } impl DependenciesBlock for AsyncDependenciesBlock { @@ -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> 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) + } +} diff --git a/crates/rspack_core/src/lib.rs b/crates/rspack_core/src/lib.rs index 66da4dd57e8..2291c85b444 100644 --- a/crates/rspack_core/src/lib.rs +++ b/crates/rspack_core/src/lib.rs @@ -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::*; diff --git a/crates/rspack_core/src/mf/container/container_entry_module.rs b/crates/rspack_core/src/mf/container/container_entry_module.rs index 490621b6f3b..d2b7e7b8939 100644 --- a/crates/rspack_core/src/mf/container/container_entry_module.rs +++ b/crates/rspack_core/src/mf/container/container_entry_module.rs @@ -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(), })); diff --git a/crates/rspack_core/src/mf/sharing/consume_shared_module.rs b/crates/rspack_core/src/mf/sharing/consume_shared_module.rs index 0a86ff77f04..e07c28e0b19 100644 --- a/crates/rspack_core/src/mf/sharing/consume_shared_module.rs +++ b/crates/rspack_core/src/mf/sharing/consume_shared_module.rs @@ -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); } diff --git a/crates/rspack_core/src/mf/sharing/provide_shared_module.rs b/crates/rspack_core/src/mf/sharing/provide_shared_module.rs index 2a8c5ae0731..86a5a9f3a51 100644 --- a/crates/rspack_core/src/mf/sharing/provide_shared_module.rs +++ b/crates/rspack_core/src/mf/sharing/provide_shared_module.rs @@ -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); } diff --git a/crates/rspack_error/src/diagnostic.rs b/crates/rspack_error/src/diagnostic.rs index e4cf7c5bfc8..5049f1ce917 100644 --- a/crates/rspack_error/src/diagnostic.rs +++ b/crates/rspack_error/src/diagnostic.rs @@ -158,6 +158,9 @@ impl From for Vec { vec![diagnostic] } } + pub fn errors_to_diagnostics(errs: Vec) -> Vec { errs.into_iter().flat_map(Vec::::from).collect() } + +pub const DIAGNOSTIC_POS_DUMMY: usize = 0; diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/import_scanner.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/import_scanner.rs index 95cdf7bd84c..b4ba0295218 100644 --- a/crates/rspack_plugin_javascript/src/visitors/dependency/import_scanner.rs +++ b/crates/rspack_plugin_javascript/src/visitors/dependency/import_scanner.rs @@ -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}; @@ -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), @@ -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), diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/worker_scanner.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/worker_scanner.rs index b8d1d2831e5..edd0be049e3 100644 --- a/crates/rspack_plugin_javascript/src/visitors/dependency/worker_scanner.rs +++ b/crates/rspack_plugin_javascript/src/visitors/dependency/worker_scanner.rs @@ -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; @@ -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, diff --git a/crates/rspack_plugin_split_chunks/src/split_chunks_plugin.rs b/crates/rspack_plugin_split_chunks/src/split_chunks_plugin.rs index f70d4b60466..d331e05ce75 100644 --- a/crates/rspack_plugin_split_chunks/src/split_chunks_plugin.rs +++ b/crates/rspack_plugin_split_chunks/src/split_chunks_plugin.rs @@ -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) } }; diff --git a/crates/rspack_plugin_split_chunks_new/src/plugin/chunk.rs b/crates/rspack_plugin_split_chunks_new/src/plugin/chunk.rs index 5dc4ed62aac..ce873d1ea38 100644 --- a/crates/rspack_plugin_split_chunks_new/src/plugin/chunk.rs +++ b/crates/rspack_plugin_split_chunks_new/src/plugin/chunk.rs @@ -94,11 +94,12 @@ impl SplitChunksPlugin { *is_reuse_existing_chunk = true; *chunk } else { - let new_chunk = Compilation::add_named_chunk( + let new_chunk_ukey = Compilation::add_named_chunk( chunk_name.clone(), &mut compilation.chunk_by_ukey, &mut compilation.named_chunks, ); + let new_chunk = compilation.chunk_by_ukey.expect_get_mut(&new_chunk_ukey); new_chunk .chunk_reasons .push("Create by split chunks".to_string()); @@ -113,7 +114,8 @@ impl SplitChunksPlugin { *is_reuse_existing_chunk_with_all_modules = true; reusable_chunk } else { - let new_chunk = Compilation::add_chunk(&mut compilation.chunk_by_ukey); + let new_chunk_ukey = Compilation::add_chunk(&mut compilation.chunk_by_ukey); + let new_chunk = compilation.chunk_by_ukey.expect_get_mut(&new_chunk_ukey); new_chunk .chunk_reasons .push("Create by split chunks".to_string()); diff --git a/crates/rspack_plugin_split_chunks_new/src/plugin/max_size.rs b/crates/rspack_plugin_split_chunks_new/src/plugin/max_size.rs index 65e150e191f..900aebf2ab9 100644 --- a/crates/rspack_plugin_split_chunks_new/src/plugin/max_size.rs +++ b/crates/rspack_plugin_split_chunks_new/src/plugin/max_size.rs @@ -274,7 +274,7 @@ impl SplitChunksPlugin { if index != last_index { let old_chunk = chunk.ukey; - let new_chunk = if let Some(name) = name { + let new_chunk_ukey = if let Some(name) = name { Compilation::add_named_chunk( name, &mut compilation.chunk_by_ukey, @@ -284,8 +284,6 @@ impl SplitChunksPlugin { Compilation::add_chunk(&mut compilation.chunk_by_ukey) }; - let new_chunk_ukey = new_chunk.ukey; - let [new_part, chunk] = compilation .chunk_by_ukey ._todo_should_remove_this_method_inner_mut() diff --git a/webpack-test/cases/chunks/weird-reference-to-entry/test.filter.js b/webpack-test/cases/chunks/weird-reference-to-entry/test.filter.js deleted file mode 100644 index 5bda53750ac..00000000000 --- a/webpack-test/cases/chunks/weird-reference-to-entry/test.filter.js +++ /dev/null @@ -1,4 +0,0 @@ - -module.exports = () => {return "https://github.com/web-infra-dev/rspack/issues/4318"} - - \ No newline at end of file