Skip to content
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

Support webpack 5 #98

Merged
merged 41 commits into from May 12, 2021
Merged

Support webpack 5 #98

merged 41 commits into from May 12, 2021

Conversation

sodatea
Copy link
Contributor

@sodatea sodatea commented May 11, 2021

What's the problem this PR addresses?

Continues the work on #83

How did you fix it?

By fixing the isBuilt function.


Other thoughts:

  1. Haven't checked webpack 4 compatibility. Shall I work on that or will this PR be released in a new major?
  2. The 2 failing tests are caused by the usage of memory-fs in mochapack. It stops the devServer.writeToDisk option from writing actual files to the disk. In the past, it's handled by write-file-webpack-plugin, but the plugin is not compatible with webpack 5. Shall I add an additional option to the CLI just to fix the tests?

jayaddison and others added 30 commits October 11, 2020 14:28
TODO: Determine how to retrieve failure count
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
Signed-off-by: John Molakvoæ (skjnldsv) <skjnldsv@protonmail.com>
fix exit code build problem for watch return type
The `built` flag in webpack 5 is moved:
webpack/webpack@eb63cf8
The `esModule` option is turned on by default in v3
@sodatea sodatea mentioned this pull request May 11, 2021
4 tasks
@larixer
Copy link
Member

larixer commented May 11, 2021

  1. Haven't checked webpack 4 compatibility. Shall I work on that or will this PR be released in a new major?

I would be grateful if webpack 4 compatibility were retained in this PR, otherwise, it will be a maintenance nightmare to keep up to date both new major and old major to support users of webpack 4 and webpack 5 at the same time

  1. The 2 failing tests are caused by the usage of memory-fs in mochapack. It stops the devServer.writeToDisk option from writing actual files to the disk. In the past, it's handled by write-file-webpack-plugin, but the plugin is not compatible with webpack 5. Shall I add an additional option to the CLI just to fix the tests?

I think env variable would be better, instead, no need to pollute CLI and its enough to have just short notice in the source code why it is needed.

Btw, why have you decided to continue work on Webpack 5 support here, what about instant-mocha, didn't it cover your needs?

@sodatea
Copy link
Contributor Author

sodatea commented May 12, 2021

Btw, why have you decided to continue work on Webpack 5 support here, what about instant-mocha, didn't it cover your needs?

It failed my tests with very obscure error messages. I have no idea how to fix or even reproduce the issue.

On the other hand, I've had a rough idea on how to fix the previous PR for a while, so I give it a try.

@sodatea
Copy link
Contributor Author

sodatea commented May 12, 2021

Done! All tests are passing.

In order to reduce the mental burden, though the implementations of the two versions are similar, I put them into different files:

  1. getAffectedModuleIds.ts & getBuildStats.ts for webpack 5
  2. webpack4GetAffectedModuleIds.ts & webpack4GetBuildStats.ts for webpack 4

I've also added two scripts in the TravisCI config to skip TypeScript/ESLint checking for unrelated files.

@codecov-commenter
Copy link

codecov-commenter commented May 12, 2021

Codecov Report

Merging #98 (b30cc73) into master (6240eeb) will decrease coverage by 4.94%.
The diff coverage is 14.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #98      +/-   ##
==========================================
- Coverage   65.87%   60.93%   -4.95%     
==========================================
  Files          42       44       +2     
  Lines        1225     1349     +124     
  Branches      137      158      +21     
==========================================
+ Hits          807      822      +15     
- Misses        400      509     +109     
  Partials       18       18              
Impacted Files Coverage Δ
...rc/runner/runnerUtils/createWebpackConfig/index.ts 100.00% <ø> (ø)
src/webpack/compiler/registerInMemoryCompiler.ts 18.42% <0.00%> (-0.50%) ⬇️
src/webpack/compiler/registerReadyCallback.ts 22.22% <0.00%> (ø)
src/webpack/loader/entryLoader.ts 54.16% <0.00%> (ø)
src/webpack/util/createEntry.ts 33.33% <ø> (ø)
src/webpack/util/createStatsFormatter.ts 30.00% <ø> (ø)
src/webpack/util/getBuildStats.ts 18.18% <0.00%> (ø)
src/webpack/util/webpack4GetAffectedModuleIds.ts 12.24% <12.24%> (ø)
src/webpack/util/getAffectedModuleIds.ts 12.50% <15.38%> (+0.25%) ⬆️
src/webpack/util/webpack4GetBuildStats.ts 19.44% <19.44%> (ø)
... and 5 more

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 6240eeb...b30cc73. Read the comment docs.

@larixer
Copy link
Member

larixer commented May 12, 2021

@sodatea Awesome, thank you so much! I'm going to review the PR shortly

@larixer larixer merged commit c4806e8 into sysgears:master May 12, 2021
@larixer
Copy link
Member

larixer commented May 12, 2021

@sodatea Merged your PR and published mochapack@2.1.0. Thanks again for your awesome work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants