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

feat: improve logic for handling multicomplier stats result to accomodate webpack5 #429

Merged
merged 3 commits into from Nov 7, 2022
Merged

feat: improve logic for handling multicomplier stats result to accomodate webpack5 #429

merged 3 commits into from Nov 7, 2022

Conversation

eokoneyo
Copy link
Contributor

This PR contains a:

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

Motivation / Use-Case

This PR aims to address an issue I think is related to #390,

Details of my system setup;

  • Operating System: Mac
  • Node Version: v16.18.0
  • NPM Version: 8.19.2
  • Yarn Version: 1.22.17
  • webpack Version: ^5.7.3
  • ${package} Version: 2.25.2

I had a similar issue on further investigation, I realized that the problem happens because the shape of stats object returned in Webpack 5 differs from that of Webpack 4. See here.

This PR checks the returned stats object for the stats property on the returned statsResult which also present on multicomplier stats results generated from Webpack, the returned stats is handled as such if this is the case, from there on everything works like it should.

Breaking Changes

Additional Info

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 28, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add tests case and accept CLA? thank you

@@ -46,4 +46,4 @@ jobs:
working_directory: /mnt/ramdisk
steps:
- setup
- run: npm test
- run: NODE_OPTIONS=--openssl-legacy-provider npm test
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes an issue running the test, an example of such failing test can be found here the adopted fix was suggested here.

@eokoneyo
Copy link
Contributor Author

eokoneyo commented Oct 30, 2022

Can you add tests case and accept CLA? thank you

@alexander-akait I added tests cases and accepted the CLA.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@glenjamin Hellom can I take care about webpack-hot-middleware, becauwe we have a lot of PRs and issues, I want to start to fix them and in future union webpack-dev-server/webpack-dev-middleware and webpack-hot-middleware in the one repo, and reuse the same code for better maintanance

@glenjamin
Copy link
Collaborator

@glenjamin Hellom can I take care about webpack-hot-middleware, becauwe we have a lot of PRs and issues, I want to start to fix them and in future union webpack-dev-server/webpack-dev-middleware and webpack-hot-middleware in the one repo, and reuse the same code for better maintanance

Yep, please go ahead - I've not been actively planning anything

@alexander-akait alexander-akait merged commit 3eb1ef0 into webpack-contrib:master Nov 7, 2022
@alexander-akait
Copy link
Member

@eokoneyo Thank you

@eokoneyo eokoneyo deleted the feature/support-webpack5-multistats-object branch November 10, 2022 09:53
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

3 participants