Performance improvement

[sokra]

This PR contains a:

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

Motivation / Use-Case

Sets don't allow duplicates, so checking for has is not necessary

webpack 5 makes these Sets lazy, which makes reading more expensive.
So only the watcher should read from these Sets, which is called after the compilation has finished,
so that doesn't contribute to build performance.

LazySets also have a addAll method, which allows to add an Iterable lazily (only iterated when LazySet is read)

Breaking Changes

No, webpack 4 compatibility is kept

Additional Info

[codecov]

Codecov Report

Merging #406 into master will decrease coverage by 0.03%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master     #406      +/-   ##
==========================================
- Coverage   94.88%   94.84%   -0.04%     
==========================================
  Files           9        9              
  Lines         293      291       -2     
  Branches       78       78              
==========================================
- Hits          278      276       -2     
  Misses         14       14              
  Partials        1        1

Impacted Files	Coverage Δ
src/index.js	96.15% <66.66%> (-0.15%)	⬇️

---

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 5db376b...cdb3cbd. Read the comment docs.

[alexander-akait]

Thanks!

[jquense]

This is breaking webpack4 for us. I think it's a bad interaction with some library using corejs, but interestingly if i log out addAll it's returning function addAll() { [native code] } which is a bit alarming. I cannot reproduce it in the repl tho!

FYI addAll is a proposed method for Set's so likely this test will fail if they are implemented https://github.com/tc39/proposal-collection-methods

[alexander-akait]

@jquense it is custom webpack class and not related to corejs

[jquense]

@evilebottnawi I realize that, the problem is that it checks if addAll is in the class, in webpack4 it is a Set, and that webpack 4 Set also has addAll that works differently. because some other library (I think) uses corejs which is polyfilling Set.addAll

[jquense]

In other words checking 'addAll' in compilation.fileDependencies is not a safe check, because there is a proposal, and polyfill for a native addAll that works differently than LazySet.addAll. The code should probably duck type this differently

[alexander-akait]

@jquense what idea do you have to fix it? Maybe instance of solve this?

[jquense]

either instanceof or maybe add a static symbol to LazySet so you can typecheck like foo[Symbol.for('@@LazySet')] == true or maybe just check the constructor.name === 'LazySet'?

[alexander-akait]

@jquense Can you send a PR?

[jquense]

That would require i larger ecosystem change and i'm not sure what the desired approach would be. Generally happy to send PR's tho. @sokra should really be aware of this issue as well, b/c this test is likely in other places