New options flag to output ES2015 modules

[joseprio]

This PR contains a:

• bugfix
• new feature
• code refactor
• test update
• typo fix
• metadata update

Motivation / Use-Case

Using the old CommonJS module syntax breaks some optimizations that Webpack does in newer versions, like tree shaking and module concatenation. This flags allows the output to be ES2015 modules if enabled.

Breaking Changes

Default is false, so it shouldn't break anything unless used.

Additional Info

[joseprio]

PR for issue #338

[alexander-akait]

Good job, we should improve supporting esmodules for css-loader, maybe for html-loader, just try to add tests for css-loader and you can see problem, anyway feel free to send a PR to css-loader

[joseprio]

Good job, we should improve supporting esmodules for css-loader, maybe for html-loader, just try to add tests for css-loader and you can see problem, anyway feel free to send a PR to css-loader

html-loader already has support for it; the option is exportAsEs6Default. Will check css-loader.

[joseprio]

Just checked css-loader; it would be a more complex process than mini-css-extract-plugin, html-loader and file-loader, but definitely doable.
However, I don't think it's a valuable feature to add. My understanding is that css-loader output is used as the input for plugins like postcss, so the final output won't be a JS module, but a CSS text file; so, the advantages of using ES6 modules in Webpack are not relevant.

[alexander-akait]

@joseprio no, look here https://github.com/webpack-contrib/css-loader/blob/master/src/runtime/getUrl.js#L1, now it is not string, it is object with default property

[jpreynat]

Hi @evilebottnawi,
Any chance we could have this PR merged soon?

[alexander-akait]

@jpreynat Why do you need this? In webpack@5 we have asset built-in modules (it is like file-loader and url-loader), so package will be deprecated after stable release webpack@5

[jpreynat]

@evilebottnawi Thanks for the info, I didn't see this new experiment on webpack 5.
However, it seems that the asset plugin in webpack 5 doesn't generate ES modules either.

Also, it seems to lack configuration. For instance, with file-loader we were able to omit the generation of the file when it wasn't needed, and specify a specific directory for different types of modules.
For instance, we could test .(png|jpg) and emit them as images/[hash].[ext], while testing for .css and emit them as styles/[hash].[ext].

That's a lot of things missing for us with the current state of the asset experiments flag.

[alexander-akait]

@jpreynat

it seems that the asset plugin in webpack 5 doesn't generate ES modules either.

// TODO: (hiroppy) use ESM will be done in near future

For instance, with file-loader we were able to omit the generation of the file when it wasn't needed

Can you clarify?

specify a specific directory for different types of modules
For instance, we could test .(png|jpg) and emit them as images/[hash].[ext], while testing for .css and emit them as styles/[hash].[ext].

You can use test/include/exclude for filtering.

Look in assetModuleFilename option. It can be function so you can create any custom logic.

That's a lot of things missing for us with the current state of the asset experiments flag.

Which?

The file-loader will be deprecated after webpack@5 was released.

[jpreynat]

@evilebottnawi
Thanks for the info.

Great if switching to ESM can be done quickly.
I will also have a look at the assetModuleFilename option. I thought that it could only be a string, but looks like a function should do the job.

Finally, we were using the emitFile: false option for file-loader when bundling for Node.
Can we easily replicate this behavior with the asset type in modules rules?

[alexander-akait]

@jpreynat

Great if switching to ESM can be done quickly.

Yes, we will do it, but i can't understand why you need modules here? It is always will be interpreted as default (you can't use named import for assets 😄 )

Finally, we were using the emitFile: false option for file-loader when bundling for Node.
Can we easily replicate this behavior with the asset type in modules rules?

In Todo, but why don't use null-loader for this when you build node project?

[jpreynat]

@evilebottnawi
For insights on our use-case, we are currently working on reducing the initial load time of our application, and since our app has many dependencies, assets, etc... it turns out that lots of initial time is spent in webpack_requires.

Using ES modules should allow webpack to further concatenate our bundle by using module concatenation for our assets too.

[alexander-akait]

@jpreynat

Using ES modules should allow webpack to further concatenate our bundle by using module concatenation for our assets too.

Please clarify? Do you mean include url to asset without extra module, right?

[jpreynat]

@evilebottnawi
Yes, basically we want to prevent having to require a new module for each of our assets.

[joseprio]

@jpreynat
This is the exact purpose for this pull request, as described in the 'motivation' part; we currently patch webpack in order to take advantage of that optimization, and it definitely helps. I'm sure other projects could take advantage of it, and migrating to version 5 (once it actually can output ESM) may not be an option for everybody

[jpreynat]

@joseprio
Thanks for confirming that. This is why I asked for an update on the PR in the first place 👍

[jpreynat]

@joseprio
Could you share any insights on how you managed to tweak webpack in order to achieve this on your side? Was it simply to work with a fork of file-loader or did you make other/further optimizations?

[alexander-akait]

@jpreynat okay, let's merge it and released, also i will add tests for this cases for asset module to ensure all works fine in webpack@5

Can you answer about emitFile?

[joseprio]

We're just using `patch-package`; not ideal from a maintenance point of view, but does the trick.

…

On Wed, 20 Nov 2019 at 14:24, Johan Preynat ***@***.***> wrote: @joseprio <https://github.com/joseprio> Could you share any insights on how you managed to tweak webpack in order to achieve this on your side? Was it simply to work with a fork of file-loader or did you make other/further optimizations? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#340?email_source=notifications&email_token=AAMTJU2I6MWJN2H3T3P6KQ3QUVCBPA5CNFSM4IFSYDSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEESEJWY#issuecomment-556025051>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMTJU76NNS64V27QVPKK4DQUVCBPANCNFSM4IFSYDSA> .

[jpreynat]

@evilebottnawi
Great news, thanks.
It's ok for us to use the new asset experiment, but we should make sure that this is working as expected with module concatenation.

Regarding the emitFile, we simply turn it to false when compiling for Node to prevent exporting our assets in our serverless functions bundle.
If the same could be achieved with the asset type, it would be awesome.
Something like if null is returned from the assetModuleFilename function, then the file is not emitted, would do the job.

@joseprio
I didn't know this package. This seems indeed a bit hacky, but sometimes we've got no choice 😉
Thanks for sharing the tip 👍

[alexander-akait]

Something like if null is returned from the assetModuleFilename function, then the file is not emitted, would do the job.

Maybe not bad idea

[jpreynat]

Just a suggestion, but that would allow to fine-grain process the assets.

[codecov]

Codecov Report

Merging #340 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #340   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           2      2           
  Lines          28     29    +1     
  Branches       11     13    +2     
=====================================
+ Hits           28     29    +1

Impacted Files	Coverage Δ
src/index.js	100% <100%> (ø)	⬆️

---

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 ba0fd4c...16519c9. Read the comment docs.

[alexander-akait]

just testing, work fine with ModuleConcatenationPlugin

[jpreynat]

Thanks for the release @evilebottnawi