fix(index): skip not changed chunk files in watch mode (fix #133)

[akosyakov]

This PR contains a:

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

Motivation / Use-Case

See #133 (comment)

Breaking Changes

Additional Info

I have not looked at tests yet, but would like to know whether it is right approach first? cc @evilebottnawi

I've followed this guidelines to check whether a chunk is changed: https://webpack.js.org/contribute/plugin-patterns/#changed-chunks

[jsf-clabot]

All committers have signed the CLA.

[codecov]

Codecov Report

Merging #134 into master will decrease coverage by 2.24%.
The diff coverage is 96.72%.

@@             Coverage Diff             @@
##            master     #134      +/-   ##
===========================================
- Coverage   100.00%   97.75%   -2.25%     
===========================================
  Files            2        2              
  Lines           74       89      +15     
  Branches        25       29       +4     
===========================================
+ Hits            74       87      +13     
- Misses           0        2       +2     

Impacted Files	Coverage Δ
src/index.js	97.72% <96.72%> (-2.28%)	⬇️

---

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 af883d8...f9bd29d. Read the comment docs.

[akosyakov]

I've fixed tests.

[akosyakov]

Published it as @theia/compression-webpack-plugin@3.0.0 for downstream testing.

[akosyakov]

I've refactored a bit to include files from modules as well and added test in watch + development modes.

[akosyakov]

here you can see that only gz file for async module is regenerated, since it was changed

[alexander-akait]

It is not good solution because our output is not deterministic and potential break other plugins, we have cache option, why don't use this?

[akosyakov]

we have cache option, why don't use this?

New gzip files with very the same content but different timestamp are emitted with the cache option and breaks express.js resource caching. Is there a way to preserve timestamps with the cache option?

[akosyakov]

It is not good solution because our output is not deterministic and potential break other plugins

I don't think it changes anything without watch mode. For single compilation it works as before. Also it's implemented with the approach suggested by webpack docs https://webpack.js.org/contribute/plugin-patterns/#changed-chunks.

[alexander-akait]

New gzip files with very the same content but different timestamp are emitted with the cache option and breaks express.js resource caching. Is there a way to preserve timestamps with the cache option?

Can you create reproducible test repo?

[akosyakov]

Can you create reproducible test repo?

@evilebottnawi there is a reference to repo on the issue: #133 (comment) It's configured for watching with cache enabled.

[alexander-akait]

Thanks, ping me tomorrow, i will look at this

[akosyakov]

@evilebottnawi ping

[ernestostifano]

@evilebottnawi Hi guys! I got here because I was about to open an issue for the same reason @akosyakov did.

I've not fully got through all the changes in this PR, but it seems a little over-complicated. Especially because Webpack already has its own cache/emission logic which could be re-used by simply changing current used hook.

The solution:

Currently, "compression-webpack-plugin" is taping into compiler.hooks.emit hook. Which is called before the emission process has even started. Just after that, Webpack decides which assets should be emitted, using its internal cache logic.

Finally, check-passing assets are effectively written and compiler.hooks.assetEmitted hook is called for each one of them. Obviously, this way, compressed assets are generated for every existent asset, even if they will not be effectively emitted at the end. (And worse, during development, all assets are compressed again, even those that has not been changed and already exist in memory filesystem).

Initially I though that compiler.hooks.emit hook was being used because at some point Webpack removes the source from emitted assets. However, when compiler.hooks.assetEmitted hook is called, source (will be of Buffer type) is passed as second argument.

Additionally, this is already done asynchronously using "neo-async" library, so no additional looping would be needed, thus increasing performance.

I think that solution should be clear from here and it should be way more simple than expected. I would be glad to try fixing this issue if you like. Please let me know.

Note: Hope I'm not missing something.

[akosyakov]

@ernestostifano if there will be official better solution i'm all in for using it :) So far we were using @theia/compression-webpack-plugin for Theia and all downstream projects in dev mode and it did not cause any apparent issues.

[alexander-akait]

@akosyakov sorry for delay, WIP on the problem

[alexander-akait]

Sorry again for big delay, fixed in master

[akosyakov]

@evilebottnawi thank you, that's fine :)