Skip to content
This repository has been archived by the owner on Sep 28, 2020. It is now read-only.

Remove duplicate output. #169

Merged
merged 1 commit into from
Mar 31, 2017
Merged

Conversation

jaridmargolin
Copy link
Contributor

Minimizes duplicate output in the following situations.

  1. Duplicate ESLint messages are logged due to Webpack throwing NonErrorEmittedError: (Emitted value instead of an instance of Error). This is done by emitting an new Error type ESLintError.

  2. Duplicate ESLint messages caused by both emitting, and throwing an error. This was fixed fairly easy by moving the emit to below the throws.

Note: Because the formatted message acts as a stack trace, our ESLINTError sets the stack to the message. This keeps output clean when logged.


A few before and after screenshots:

Build Before Patch
Build Before Patch

Build After Patch
Build After Patch

Start Before Patch
Start Before Patch

Start After Patch
Start After Patch

@MoOx
Copy link
Contributor

MoOx commented Mar 29, 2017

Looks good. I guess @gaearon will be happy about this?

Can you fix the tests? Looks like an eslint issue is here :) (Will add prettier soon to avoid this)

@jaridmargolin
Copy link
Contributor Author

Not sure how that squeaked by. Made a few modifications. All tests now passing.

@jaridmargolin
Copy link
Contributor Author

jaridmargolin commented Mar 29, 2017

I take that back. A few older versions of node/webpack appear to have issues. I'll look into it.

@jaridmargolin
Copy link
Contributor Author

jaridmargolin commented Mar 29, 2017

Is webpack@1 support important? Is there any possibility of a new major version that supports webpack@2 only? I'll be honest, I'm not certain how much time I want to invest int getting webpack@1 tests passing.

* I will definitely get webpack@2 tests passing in node 4

@jaridmargolin
Copy link
Contributor Author

Update: All webpack@2 tests passing

@MoOx
Copy link
Contributor

MoOx commented Mar 29, 2017

webpack 1 support is important yes, and probably not hard to do (I recently tweaked the tests but your rollbacked this)

@jaridmargolin
Copy link
Contributor Author

Honestly not a huge fan of the solution but I think it is the best we are going to do.

Internally webpack@2 has changed and requires errors to be passed through emit. When it writes it out to a file, it appears to grab the message prop off the error. webpack@1 on the other hand, writes whatever is passed to the file, without any sort of transformation step. As a result the xml output was wrapped in a javascript object with a message property.

So the solution, conditionally send as an ESLintError: emitter(webpack.version === 2 ? new ESLintError(messages) : messages)

All tests now passing.

@MoOx MoOx merged commit a6f4f0b into webpack-contrib:master Mar 31, 2017
@jaridmargolin
Copy link
Contributor Author

Awesome. Thanks for the quick publish 👍

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

Successfully merging this pull request may close these issues.

2 participants