Skip to content

fix: avoid emitting JavaScript for CSS-only entries #14879

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.

Sign up for GitHub

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

Draft
intellild wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: avoid emitting JavaScript for CSS-only entries #14879

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 26 additions & 10 deletions crates/rspack_plugin_css/src/parser_and_generator/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ pub(crate) static CSS_MODULE_AND_JS_SOURCE_TYPE_LIST: &[SourceType; 2] =
pub(crate) static CSS_MODULE_EXPORTS_ONLY_SOURCE_TYPE_LIST: &[SourceType; 1] =
&[SourceType::JavaScript];

pub(crate) static CSS_MODULE_NO_SOURCE_TYPE_LIST: &[SourceType; 0] = &[];

pub type CssExportsRef<'a> = FxIndexMap<&'a str, &'a FxIndexSet<CssExport>>;

#[cacheable]
Expand Down Expand Up @@ -204,19 +206,33 @@ impl ParserAndGenerator for CssParserAndGenerator {
return CSS_MODULE_EXPORTS_ONLY_SOURCE_TYPE_LIST;
}

let incoming_connections = module_graph
.get_incoming_connections(&module.identifier())
.collect::<Vec<_>>();

if self.exports_only {
return CSS_MODULE_EXPORTS_ONLY_SOURCE_TYPE_LIST;
let is_root_only = !incoming_connections.is_empty()
&& incoming_connections
.iter()
.all(|conn| conn.original_module_identifier.is_none());
return if is_root_only {
CSS_MODULE_NO_SOURCE_TYPE_LIST
} else {
CSS_MODULE_EXPORTS_ONLY_SOURCE_TYPE_LIST
};
}

let no_need_js = module_graph
.get_incoming_connections(&module.identifier())
.all(|conn| {
let dep = module_graph.dependency_by_id(&conn.dependency_id);
matches!(
dep.dependency_type(),
DependencyType::CssImport | DependencyType::EsmImport
)
});
let no_need_js = incoming_connections.iter().all(|conn| {
if conn.original_module_identifier.is_none() {
return true;
}

let dep = module_graph.dependency_by_id(&conn.dependency_id);
matches!(
dep.dependency_type(),
DependencyType::CssImport | DependencyType::EsmImport
)
});

if no_need_js {
CSS_MODULE_SOURCE_TYPE_LIST
Expand Down
72 changes: 31 additions & 41 deletions crates/rspack_plugin_javascript/src/plugin/impl_plugin_for_js_plugin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ use rspack_core::{
AssetInfo, CachedConstDependencyTemplate, ChunkGraph, ChunkKind, ChunkUkey, Compilation,
CompilationAdditionalTreeRuntimeRequirements, CompilationChunkHash, CompilationContentHash,
CompilationId, CompilationParams, CompilationRenderManifest, CompilerCompilation,
ConstDependencyTemplate, DependencyType, IgnoreErrorModuleFactory, ManifestAssetType,
ModuleGraph, ModuleType, ParserAndGenerator, PathData, Plugin, RenderManifestEntry,
RuntimeGlobals, RuntimeModule, RuntimeRequirementsDependencyTemplate, SelfModuleFactory,
SourceType, get_js_chunk_filename_template,
ConstDependencyTemplate, DependencyType, IgnoreErrorModuleFactory, ManifestAssetType, ModuleType,
ParserAndGenerator, PathData, Plugin, RenderManifestEntry, RuntimeGlobals, RuntimeModule,
RuntimeRequirementsDependencyTemplate, SelfModuleFactory, SourceType,
get_js_chunk_filename_template,
rspack_sources::{BoxSource, CachedSource, SourceExt},
};
use rspack_error::{Diagnostic, Result};
Expand Down Expand Up @@ -559,29 +559,13 @@ async fn render_manifest(
.expect_get(chunk_ukey);
let runtime_template = compilation.runtime_template.create_chunk_code_template();
let is_hot_update = matches!(chunk.kind(), ChunkKind::HotUpdate);
let is_main_chunk = chunk.groups().iter().any(|group_ukey| {
let group = compilation
.build_chunk_graph_artifact
.chunk_group_by_ukey
.expect_get(group_ukey);

group.is_initial() && group.kind.is_entrypoint() && &group.get_entrypoint_chunk() == chunk_ukey
});
let is_runtime_chunk =
chunk.has_runtime(&compilation.build_chunk_graph_artifact.chunk_group_by_ukey);

if !is_hot_update
&& is_runtime_chunk
&& !chunk_has_runtime_or_js(
chunk_ukey,
&compilation.build_chunk_graph_artifact.chunk_graph,
compilation.get_module_graph(),
)
{
if !is_hot_update && is_runtime_chunk && !chunk_has_runtime_or_js(chunk_ukey, compilation) {
return Ok(());
}
if !is_hot_update && !is_main_chunk && !is_runtime_chunk && !chunk_has_js(chunk_ukey, compilation)
{
if !is_hot_update && !is_runtime_chunk && !chunk_has_js(chunk_ukey, compilation) {
return Ok(());
}
let filename_template = get_js_chunk_filename_template(
Expand Down Expand Up @@ -705,15 +689,6 @@ pub struct ExtractedCommentsInfo {
}

pub fn chunk_has_js(chunk_ukey: &ChunkUkey, compilation: &Compilation) -> bool {
if compilation
.build_chunk_graph_artifact
.chunk_graph
.get_number_of_entry_modules(chunk_ukey)
> 0
{
return true;
}

compilation
.build_chunk_graph_artifact
.chunk_graph
Expand All @@ -724,20 +699,35 @@ pub fn chunk_has_js(chunk_ukey: &ChunkUkey, compilation: &Compilation) -> bool {
)
}

fn chunk_has_runtime_or_js(
chunk: &ChunkUkey,
chunk_graph: &ChunkGraph,
module_graph: &ModuleGraph,
) -> bool {
fn chunk_has_runtime_or_js(chunk_ukey: &ChunkUkey, compilation: &Compilation) -> bool {
if chunk_has_js(chunk_ukey, compilation) {
return true;
}

let chunk_graph = &compilation.build_chunk_graph_artifact.chunk_graph;
if chunk_graph
.get_chunk_runtime_modules_iterable(chunk)
.get_chunk_runtime_modules_iterable(chunk_ukey)
.next()
.is_some()
.is_none()
{
return true;
return false;
}
if chunk_graph.has_chunk_module_by_source_type(chunk, SourceType::JavaScript, module_graph) {
return true;

let chunk = compilation
.build_chunk_graph_artifact
.chunk_by_ukey
.expect_get(chunk_ukey);
for group_ukey in chunk.groups() {
let chunk_group = compilation
.build_chunk_graph_artifact
.chunk_group_by_ukey
.expect_get(group_ukey);
for chunk_ukey in &chunk_group.chunks {
if chunk_has_js(chunk_ukey, compilation) {
return true;
}
}
}

false
}
12 changes: 3 additions & 9 deletions tests/rspack-test/configCases/asset-modules/entry-with-runtimeChunk/test.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,13 @@ module.exports = {

switch (i % 4) {
case 0:
return ["test.js", `${i}/runtime~app.${ext}`];
return ["test.js"];
case 1:
return ["test.js", `${i}/app.${ext}`, `${i}/runtime~app.${ext}`];
case 2:
return ["test.js", `${i}/app.${ext}`, `${i}/runtime~app.${ext}`];
return ["test.js"];
case 3:
return [
"test.js",
`${i}/entry1.${ext}`,
`${i}/entry2.${ext}`,
`${i}/runtime~entry1.${ext}`,
`${i}/runtime~entry2.${ext}`
];
return ["test.js", `${i}/entry2.${ext}`, `${i}/runtime~entry2.${ext}`];
default:
break;
}
Expand Down
12 changes: 6 additions & 6 deletions tests/rspack-test/configCases/asset-modules/only-entry/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@ it("should work", () => {
break;
}
case 2: {
expect(stats.assets.length).toBe(4);
expect(stats.assets.length).toBe(3);

const cssEntryInJs = stats.assets.find(
a => a.name.endsWith("css-entry.js")
);
expect(Boolean(cssEntryInJs)).toBe(true);
expect(Boolean(cssEntryInJs)).toBe(false);

const cssEntry = stats.assets.find(
a => a.name.endsWith("css-entry.css")
Expand All @@ -40,7 +40,7 @@ it("should work", () => {
break;
}
case 3: {
expect(stats.assets.length).toBe(5);
expect(stats.assets.length).toBe(4);

const jsEntry = stats.assets.find(
a => a.name.endsWith("js-entry.js")
Expand All @@ -50,7 +50,7 @@ it("should work", () => {
const cssEntryInJs = stats.assets.find(
a => a.name.endsWith("css-entry.js")
);
expect(Boolean(cssEntryInJs)).toBe(true);
expect(Boolean(cssEntryInJs)).toBe(false);

const cssEntry = stats.assets.find(
a => a.name.endsWith("css-entry.css")
Expand All @@ -59,7 +59,7 @@ it("should work", () => {
break;
}
case 4: {
expect(stats.assets.length).toBe(4);
expect(stats.assets.length).toBe(3);

const jsEntry = stats.assets.find(
a => a.name.endsWith("js-entry.js")
Expand All @@ -69,7 +69,7 @@ it("should work", () => {
const cssEntryInJs = stats.assets.find(
a => a.name.endsWith("css-entry.js")
);
expect(Boolean(cssEntryInJs)).toBe(true);
expect(Boolean(cssEntryInJs)).toBe(false);
break;
}
case 5: {
Expand Down
Loading