Skip to content

Conversation

maxleaver
Copy link

@maxleaver maxleaver commented Jun 27, 2018

Warnings are currently sent to webpack as strings. However, Webpack's emitWarning function expects an error, as described in their API. This results in the errors described in #350. This PR simply changes the warnings emitted by postcss-loader from strings to an error instance.

Type

  • Fix

SemVer

  • Bug (:label: Patch)

Issues

Checklist

  • Lint and unit tests pass with my changes
  • I have added tests that prove my fix is effective/works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes are merged and published in downstream modules

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.

Thanks for PR! Can you add test?

@alexander-akait
Copy link
Member

/cc @Kovensky can you test this too, thanks

@michael-ciniawsky michael-ciniawsky changed the title Fix #350 - Warnings are emitted as errors instead of strings fix(index): emit warnings as an instance of {Error} instead of {String} Jul 9, 2018
@michael-ciniawsky michael-ciniawsky added this to the 2.1.6 milestone Jul 9, 2018
Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

Please add a test for this. Feel free to ask any question in the PR thread in case you're stuck with the test setup. test/Errors.test.js(Syntax Error) is a good starting point

.process(css, options)
.then((result) => {
result.warnings().forEach((msg) => this.emitWarning(msg.toString()))
result.warnings().forEach((msg) => this.emitWarning(new Error(msg.toString())))
Copy link
Member

@michael-ciniawsky michael-ciniawsky Jul 9, 2018

Choose a reason for hiding this comment

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

I need to check the Postcss API docs, but I think warnings can contain location info and a code frame if the plugin author used the appropiated PostCSS Plugin API. If that's the case it would be better to reuse the code located in lib/Error.js and create a new class LoaderWarning (lib/Warning.js) based upon that to be consistent in terms of formatting

@michael-ciniawsky
Copy link
Member

Closing in favor of 6eb82be Thx

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.

postcss warnings print "(Emitted value instead of an instance of Error)"

3 participants