geowarin / friendly-errors-webpack-plugin

Recognizes certain classes of webpack errors and cleans, aggregates and prioritizes them to provide a better Developer Experience.
MIT License
749 stars 77 forks source link

Retrieve errors/warnings from compilation.children if needed #93

Closed Lyrkan closed 5 years ago

Lyrkan commented 5 years ago

This PR should fix #84 by also looking for errors/warnings recursively in compilation.children when none is found in compilation.

Without it the added test gave the following output:

Expected value to equal:
  ["WARNING  Compiled with 1 warnings", "", "warning  in ./test/fixtures/postcss-warnings/index.css", "", "Module Warning (from ./node_modules/postcss-loader/src/index.js):
Warning

(3:2) grid-gap only works if grid-template(-areas) is being used", ""]
Received:
  ["WARNING  Compiled with 0 warnings", ""]
christophehurpeau commented 5 years ago

Thanks for the PR :)

It seems CRA uses stats.toJson({ all: false, warnings: true, errors: true }) to get all warnings and errors recursivly ? Could you try if you can get them like this and the impact it might have on formatters ?

christophehurpeau commented 5 years ago

https://github.com/facebook/create-react-app/blob/e7a2d6168a3a93ea58bafdc851eb2bbda20d5ce6/packages/react-scripts/scripts/build.js#L166

Lyrkan commented 5 years ago

Hey @christophehurpeau,

I'll look into it but I don't think that can be done easily there, at least not without changing how transformers/formatters work.

Calling stats.toJson() does return error/warning messages (as strings) but transformers/formatters expect a Webpack error that contains other data:

https://github.com/geowarin/friendly-errors-webpack-plugin/blob/273ca3b19d4f8191604836ffcf61286596ee9477/src/core/transformErrors.js#L5-L26

Lyrkan commented 5 years ago

@christophehurpeau Do you think the switch to stats.json() is worth the hassle?

It'd require to rewrite all the current transformers/formatters (and not only the default ones because of the API changes) in order for them to only use the error message.

christophehurpeau commented 5 years ago

Sorry for my late response. You're right I'm not sure stats.json() is worth but in your implementation it seems you don't handle when isMultiStats(stats) is true, could you use findErrorsRecursive in this case too and add a test if possible ? Thanks

Lyrkan commented 5 years ago

Hey,

Multi-stats should already be handled by the following code since it also calls extractErrorsFromStats() recursively:

https://github.com/geowarin/friendly-errors-webpack-plugin/blob/fd2526f98bdfd6ece603e9f0f3982a0519750af0/src/friendly-errors-plugin.js#L138-L144

I'll see if I can add an extra test for that though.

christophehurpeau commented 5 years ago

Yes sorry ! I read to quickly. Thanks for the test ! I'll wait for it before merging then :)

Lyrkan commented 5 years ago

There you go!

christophehurpeau commented 5 years ago

Released in 2.0.0-beta.2, thank you !