-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
fix: prevent empty JS file generation for CSS-only entry points #20454
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
Changes from all commits
fab42bd
eb183af
4f60461
926bab1
b5a4b04
132d31f
ec55539
cd1b51b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "webpack": patch | ||
| --- | ||
|
|
||
| Fixed an issue where empty JavaScript files were generated for CSS-only entry points. The code now correctly checks if entry modules have JavaScript source types before determining whether to generate a JS file. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,19 +6,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}` | ||
| ]; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. now we don't generate extra runtime code for such cases because here we have css entries here, right?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah — in both of the following entry cases, we no longer generate runtime.js or main.js. entry: {
app: ["../_images/file.png", "./entry.css"]
} |
||
| return ["test.js", `${i}/entry2.${ext}`, `${i}/runtime~entry2.${ext}`]; | ||
| default: | ||
| break; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| body { | ||
| color: red; | ||
| } | ||
| --- | ||
| body { | ||
| color: blue; | ||
| } | ||
| --- | ||
| body { | ||
| color: green; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to use
ChunkGraphas a key here (i.e. two level cache)? I thinkChunkGraphis not changing due compilations... in case of multi compiler I think we can movechunkHasJsCacheinside class, i.e.this._chunkHasJsCache = new WeakMapIdeally in future we should reduce count of
new WeakMap/new WeakSet/new Map/new Setand etc at the top of file, it takes time for initial loading, yeah, it is not critical, but we should refactor it in futureThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The two-level WeakMap cache is necessary since each compilation (including HMR) creates a new
ChunkGraphinstance, so theChunkGraphkey correctly isolates results per compilation.We cannot move the cache into the class because
chunkHasJsis a static method.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it, we will think how to refactor this to avoid extra initial maps/sets in future