Remove empty modules #380

irudoy · 2019-04-18T16:13:54Z

This PR contains a:

Motivation / Use-Case

fixes #357

Breaking Changes

Additional Info

jsf-clabot · 2019-04-18T16:13:56Z

All committers have signed the CLA.

alexander-akait · 2019-04-18T16:52:45Z

Please fix CI problems, also accept CLA, thanks

src/index.js

irudoy · 2019-04-18T17:08:50Z

@evilebottnawi do you suggest updating vulnerable dependencies in this PR?
CLA has already been signed.

src/index.js

alexander-akait

Great job 👍

irudoy · 2019-04-29T11:57:06Z

Friendly ping

alexander-akait · 2019-04-29T12:11:12Z

All looks very good, wait @sokra review (we do this in near future), thanks fro waiting

aberezkin · 2019-05-29T06:18:18Z

@sokra, @evilebottnawi, any news about that?

codecov · 2019-05-29T11:33:45Z

Codecov Report

Merging #380 into master will increase coverage by 0.78%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #380      +/-   ##
==========================================
+ Coverage   88.49%   89.27%   +0.78%     
==========================================
  Files           5        5              
  Lines         426      457      +31     
  Branches       94      104      +10     
==========================================
+ Hits          377      408      +31     
  Misses         47       47              
  Partials        2        2

Impacted Files	Coverage Δ
src/index.js	`89.30% <100.00%> (+1.44%)`	⬆️
src/loader.js	`89.47% <100.00%> (+0.22%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ffc393...396cba7. Read the comment docs.

src/index.js

irudoy · 2019-06-19T13:30:19Z

Any news?

test/cases/simple-require/expected/main.js

src/index.js

irudoy · 2019-08-04T15:41:33Z

@sokra @evilebottnawi is it ok now?

irudoy · 2019-09-23T06:14:12Z

Any news?

sokra

Improved a lot since last review 👍

src/index.js

sokra · 2019-09-24T06:56:58Z

src/index.js

@@ -390,6 +399,47 @@ class MiniCssExtractPlugin {
          return source;
        }
      );
+
+      compilation.hooks.optimizeChunks.tap(pluginName, (chunks) => {


This should be done earlier, maybe in finishModules. Otherwise unneeded things are done for these removed modules, i. e. added to chunks, assigned ids, etc.

src/index.js

sokra · 2019-09-24T07:05:54Z

src/index.js

+                  cssModuleReason.module &&
+                  cssModuleReason.module.buildMeta.extracted
+                ) {
+                  chunk.removeModule(cssModuleReason.module);


Should be unneeded when using finishModules hook.

alexander-akait · 2019-10-03T11:12:14Z

/cc @irudoy you do great job, big thanks, ping me when you need review or help from me or sokra

We do release after merge this

igoradamenko · 2019-10-31T09:58:18Z

Hey guys!

What is the status of the PR? Is it going to be merged? I see that @irudoy has already pushed some changes. Are they enough to merge or not? May I help to speed up the development here in any way?

irudoy · 2019-10-31T11:05:42Z

@igoradamenko as @sokra noted we should implement it using finishModules hook. Unfortunately, I haven't much time at the moment to do this. I will try to finish it soon.

therealshark · 2020-02-12T08:14:10Z

@irudoy Did you have any time to work on this? The Project i'm working on currently has ~640 empty modules because of this behaviour, so this pr would help us quite a bit :)

kherock · 2020-03-27T03:47:15Z

For those following this issue that still can't use webpack 5 for one reason or another - I've created a gist that contains the most complete implementation of a plugin for removing empty JS files that I'm aware of:
Remove empty CSS modules workaround

The implementation is mostly inspired by the work in this PR plus some of the suggested changes. Some of the issues I ran into with other workarounds posted is that they break with split chunks or have other quirks when optimization is enabled.

irudoy · 2020-03-27T09:31:08Z

@kherock thanks for the contribution!

@sokra @evilebottnawi
As of webpack-5-beta.13, the problem still exists. Dead code with empty modules remains in the production bundle. Maybe it is possible to remove it at the webpack level? If there is no code in the module (excluding comments), remove the module automatically.

If this option is not suitable, maybe we have a new and more proper way to remove unnecessary modules in webpack-5? Or maybe we need the new API as of webpack-5, which will allow us to easily remove unneeded modules?

The current implementation looks fragile and brings an overhead that seems to be avoided by doing so in the webpack core.

ScriptedAlchemy · 2020-04-06T05:21:14Z

if someone has something stable, ill be willing to accept a PR to extract-css-chunks-webpack-plugin. Its got a decent amount of users and I regularly try to PR changes from extract-css back to mini-css. Serves as a good testing ground as well. So if anyone wants something installable on NPM, open a PR there and as long as its stable ill release it till this PR gets merged.

irudoy · 2020-08-10T18:51:53Z

Seems like it was fixed by #546 (using esModule: true option)

Can we close this PR and related issue?

alexander-akait · 2020-08-11T10:43:54Z

Yes, let's add more tests, WIP

irudoy · 2020-08-11T13:56:52Z

@evilebottnawi with another PR, maybe?

alexander-akait · 2020-08-11T13:58:18Z

@irudoy yes, we can do it in the separate PR

irudoy · 2020-08-13T11:19:43Z

@evilebottnawi done #566

aleen42 · 2022-04-08T02:26:47Z

I have tested with {esModule: true}, which is true by default, and found that there were still so many empty modules:

(self["webpackJsonp"] = self["webpackJsonp"] || []).push([["X"],{
    // ...

/***/ "./a.less":
/*!***************************************************************************************************************!*\
  !*** ./a.less ***!
  \***************************************************************************************************************/
/***/ (function() {

eval("// extracted by mini-css-extract-plugin\n\n//# sourceURL=webpack://cmxt.wmweb/./a.less?");

/***/ }),

/***/ "./b.less":
/*!***************************************************************************************************!*\
  !*** ./b.less ***!
  \***************************************************************************************************/
/***/ (function() {

eval("// extracted by mini-css-extract-plugin\n\n//# sourceURL=webpack://cmxt.wmweb/./b.less?");

/***/ }),

    // ...
}]);

alexander-akait · 2022-04-08T03:23:35Z

Because you import them in JS, we can't fully remove them, otherwise if will get wrong behavior

aleen42 · 2022-04-08T07:14:16Z

Because you import them in JS, we can't fully remove them, otherwise if will get wrong behavior

The hardest thing to remove is to eliminate all import points in JavaScript, but it should be a feature to do so.

alexander-akait reviewed Apr 18, 2019

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

alexander-akait reviewed Apr 19, 2019

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

alexander-akait previously approved these changes Apr 19, 2019

View reviewed changes

sokra requested changes May 29, 2019

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

src/index.js Outdated Show resolved Hide resolved

src/index.js Outdated Show resolved Hide resolved

src/index.js Outdated Show resolved Hide resolved

irudoy dismissed alexander-akait’s stale review via e3ec454 May 31, 2019 16:47

sokra requested changes Jun 24, 2019

View reviewed changes

test/cases/simple-require/expected/main.js Outdated Show resolved Hide resolved

sokra requested changes Jul 30, 2019

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

src/index.js Outdated Show resolved Hide resolved

irudoy requested a review from sokra September 18, 2019 11:26

sokra requested changes Sep 24, 2019

View reviewed changes

irudoy added 7 commits February 20, 2020 03:40

fix: remove empty modules

84782d4

fix: use for of instead of forEach for modulesIterable

36c1456

fix: add cjs test case

55c959a

test: add manual stress test

218a135

fix: avoid using issuer

bec4a2b

fix: skip cjs when removing empty modules, refactor

1a26dca

fix: add test/stress to .prettierignore

ba37daa

irudoy added 4 commits February 20, 2020 03:40

fix: css formatting

634784e

fix: improve SetMap

ee6be12

fix: check CommonJS

c70f4e3

fix: add allowlist for supported dependencies

396cba7

irudoy closed this Aug 11, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove empty modules #380

Remove empty modules #380

irudoy commented Apr 18, 2019

jsf-clabot commented Apr 18, 2019 •

edited

Loading

alexander-akait commented Apr 18, 2019

irudoy commented Apr 18, 2019

alexander-akait left a comment

irudoy commented Apr 29, 2019

alexander-akait commented Apr 29, 2019

aberezkin commented May 29, 2019

codecov bot commented May 29, 2019 •

edited

Loading

irudoy commented Jun 19, 2019

irudoy commented Aug 4, 2019

irudoy commented Sep 23, 2019

sokra left a comment

sokra Sep 24, 2019

sokra Sep 24, 2019

alexander-akait commented Oct 3, 2019

igoradamenko commented Oct 31, 2019

irudoy commented Oct 31, 2019

therealshark commented Feb 12, 2020

kherock commented Mar 27, 2020 •

edited

Loading

irudoy commented Mar 27, 2020

ScriptedAlchemy commented Apr 6, 2020

irudoy commented Aug 10, 2020 •

edited

Loading

alexander-akait commented Aug 11, 2020

irudoy commented Aug 11, 2020

alexander-akait commented Aug 11, 2020

irudoy commented Aug 13, 2020

aleen42 commented Apr 8, 2022

alexander-akait commented Apr 8, 2022

aleen42 commented Apr 8, 2022 •

edited

Loading

Remove empty modules #380

Remove empty modules #380

Conversation

irudoy commented Apr 18, 2019

Motivation / Use-Case

Breaking Changes

Additional Info

jsf-clabot commented Apr 18, 2019 • edited Loading

alexander-akait commented Apr 18, 2019

irudoy commented Apr 18, 2019

alexander-akait left a comment

Choose a reason for hiding this comment

irudoy commented Apr 29, 2019

alexander-akait commented Apr 29, 2019

aberezkin commented May 29, 2019

codecov bot commented May 29, 2019 • edited Loading

Codecov Report

irudoy commented Jun 19, 2019

irudoy commented Aug 4, 2019

irudoy commented Sep 23, 2019

sokra left a comment

Choose a reason for hiding this comment

sokra Sep 24, 2019

Choose a reason for hiding this comment

sokra Sep 24, 2019

Choose a reason for hiding this comment

alexander-akait commented Oct 3, 2019

igoradamenko commented Oct 31, 2019

irudoy commented Oct 31, 2019

therealshark commented Feb 12, 2020

kherock commented Mar 27, 2020 • edited Loading

irudoy commented Mar 27, 2020

ScriptedAlchemy commented Apr 6, 2020

irudoy commented Aug 10, 2020 • edited Loading

alexander-akait commented Aug 11, 2020

irudoy commented Aug 11, 2020

alexander-akait commented Aug 11, 2020

irudoy commented Aug 13, 2020

aleen42 commented Apr 8, 2022

alexander-akait commented Apr 8, 2022

aleen42 commented Apr 8, 2022 • edited Loading

jsf-clabot commented Apr 18, 2019 •

edited

Loading

codecov bot commented May 29, 2019 •

edited

Loading

kherock commented Mar 27, 2020 •

edited

Loading

irudoy commented Aug 10, 2020 •

edited

Loading

aleen42 commented Apr 8, 2022 •

edited

Loading