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

Errors and Warnings Should Be Processed Individually #133

Closed
shellscape opened this issue Feb 20, 2018 · 4 comments
Closed

Errors and Warnings Should Be Processed Individually #133

shellscape opened this issue Feb 20, 2018 · 4 comments

Comments

@shellscape
Copy link
Contributor

Hey folks! Webpack member and webpack-stylish author here. We recently had a user post an interesting issue about warnings being received from this plugin on their compilation. We're working through that issue to account for the vastly different-than-expected error message format. But that raised a few issues about this plugin:

  • webpack plugins shouldn't blindly pass along tool-formatted errors, because the context is a webpack plugin, and not the same as if that tool had been run separately.
  • each individual error and warning should be sent as a separate error here: https://github.com/JaKXz/stylelint-webpack-plugin/blob/master/lib/run-compilation.js#L38. that would provide some consistency with webpack's error reporting (funky as it is), and provide a reasonable ability to parse it after the fact (for example, in webpack-stylish).

Addressing both would be a great improvement for this module.

@JaKXz
Copy link
Contributor

JaKXz commented Feb 21, 2018

Thanks for the issue @shellscape - would you be willing to put together a draft PR that addresses these issues? These changes have been made over time to adjust to things like webpack friendly errors plugin (#112, #104) and so on, so I just don't want to recreate those conflicts.

I hope that makes sense.

webpack plugins shouldn't blindly pass along tool-formatted errors

One of this plugins' features is being allowed to specify a formatter, and it just so happens that we use the stylelint string formatter by default. Perhaps this is the wrong way to do this. What do you think would be better but still allow for this functionality?

@shellscape
Copy link
Contributor Author

@JaKXz no problem, I can PR this. It should be a relatively simple change. From looking at the changes in both of those PRs, I don't see a conflict. But it would be prudent to loop in @nickgcattaneo on this change at the very least. I believe that we should be able to maintain the same formatter patterns as well.

The big difference in this change would be: for a file that had 10 stylelint errors, the plugin would push 10 individual Errors to the compiler, rather than 1 Error formatted that contained 10 errors in one string.

It also looks like this RFC (webpack/webpack#6534) will be approved and pushed forward in the near term. That RFC proposes enforcing a maximum length for an error message - something that this plugin would be impacted by without changes proposed in this issue. That'll likely result in a minor bump of the [email protected] branch, and will make it into 4.x shortly after. That proposal will create a standard around pushing errors to the compiler and enforce that standard, to degree. So getting on board with that early will be a good thing 👍

@ricardogobbosouza
Copy link
Collaborator

Should this be applied?

@alexander-akait
Copy link
Member

@ricardogobbosouza feel free to close

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

No branches or pull requests

4 participants