Skip to content

Comments

fix: multi compiler progress output#3683

Closed
alexander-akait wants to merge 4 commits intomasterfrom
fix-progress-for-multi-compiler-mode
Closed

fix: multi compiler progress output#3683
alexander-akait wants to merge 4 commits intomasterfrom
fix-progress-for-multi-compiler-mode

Conversation

@alexander-akait
Copy link
Member

What kind of change does this PR introduce?

bug fix

Did you add tests for your changes?

No, but we need after we will fix a problem in webpack

If relevant, did you update the documentation?

No need

Summary

fixes #3604

Does this PR introduce a breaking change?

No

Other information

There is a bug inside webpack:

TypeError: Cannot read properties of undefined (reading 'then')
    at /home/akait/IdeaProjects/webpack-cli/node_modules/webpack/lib/ProgressPlugin.js:358:27
    at Hook.eval [as callAsync] (eval at create (/home/akait/IdeaProjects/webpack-cli/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:8:17)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (/home/akait/IdeaProjects/webpack-cli/node_modules/tapable/lib/Hook.js:18:14)
    at /home/akait/IdeaProjects/webpack-cli/node_modules/webpack/lib/Compiler.js:1192:33
    at finalCallback (/home/akait/IdeaProjects/webpack-cli/node_modules/webpack/lib/Compilation.js:2787:11)
    at /home/akait/IdeaProjects/webpack-cli/node_modules/webpack/lib/Compilation.js:3092:11
    at Hook.eval [as callAsync] (eval at create (/home/akait/IdeaProjects/webpack-cli/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:6:1)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (/home/akait/IdeaProjects/webpack-cli/node_modules/tapable/lib/Hook.js:18:14)
    at /home/akait/IdeaProjects/webpack-cli/node_modules/webpack/lib/Compilation.js:3085:38
    at eval (eval at create (/home/akait/IdeaProjects/webpack-cli/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:17:1)

Node.js v18.12.0

looks like cache was not design for multi compiler mode, we need to fix it

@alexander-akait alexander-akait requested a review from a team as a code owner March 18, 2023 18:26
@snitin315
Copy link
Member

snitin315 commented Apr 3, 2023

Yeah, it looks not straightforward as we assumed.

@alexander-akait alexander-akait force-pushed the fix-progress-for-multi-compiler-mode branch from 7f6693b to 46a25e1 Compare June 4, 2023 18:28
@alexander-akait
Copy link
Member Author

Don't have ideas how to fix it properly... We need multi compiler and apply plugin, but don't have a hook for it, we can use handler, but it will loose ability to set options, it was not design for this, very bad

@alexander-akait
Copy link
Member Author

alexander-akait commented Jun 4, 2023

And we can't just create compiler, because it requires callback for watch, I told Tobias what it was bad design for multi compiler plugins

@alexander-akait
Copy link
Member Author

Okay, I found solution, but it still need improve some things on webpack side

@alexander-akait alexander-akait force-pushed the fix-progress-for-multi-compiler-mode branch from 7ede1d2 to 7e0018c Compare June 4, 2023 23:47
@codecov
Copy link

codecov bot commented Jun 4, 2023

Codecov Report

Merging #3683 (7e0018c) into master (cf1796f) will decrease coverage by 0.52%.
The diff coverage is 41.17%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3683      +/-   ##
==========================================
- Coverage   91.39%   90.87%   -0.52%     
==========================================
  Files          22       22              
  Lines        1673     1688      +15     
  Branches      484      486       +2     
==========================================
+ Hits         1529     1534       +5     
- Misses        144      154      +10     
Impacted Files Coverage Δ
packages/webpack-cli/src/webpack-cli.ts 92.60% <ø> (ø)
packages/webpack-cli/src/plugins/cli-plugin.ts 86.11% <41.17%> (-13.89%) ⬇️

Continue to review full report in Codecov by Sentry.

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

@alexander-akait
Copy link
Member Author

alexander-akait commented Jun 4, 2023

After we shipped some new things to webpack (I will send a PR soon), we will need to update webpack here, run CI again and we can merge

@snitin315
Copy link
Member

@alexander-akait so does that mean this feature will not work with older version of webpack v5? We would need to mention it in docs.

@alexander-akait
Copy link
Member Author

@snitin315 Yeah, It will put this in the changelog

@alexander-akait
Copy link
Member Author

I don't undestand why github doesn't show my sent commits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ProgressPlugin support for MultiCompiler

3 participants