refactor(ModuleWarning): upgrade to ES6 - #3853
Conversation
| } | ||
| this.name = "ModuleWarning"; | ||
| this.module = module; | ||
| this.message = warning; |
There was a problem hiding this comment.
Give this as the argument to super instead (super(warning))
There was a problem hiding this comment.
I'm just keeping how it was before. I can't change it because I can't track all impacts. That's the reason why I kept all the current behaviors and attributes.
| super(); | ||
|
|
||
| if(Error.hasOwnProperty("captureStackTrace")) { | ||
| Error.captureStackTrace(this, ModuleWarning); |
There was a problem hiding this comment.
Not needed in native subclasses, unless you specifically want to exclude the constructor from the stack trace.
CustomErrorClass: without captureStackTrace
at CustomErrorClass (.../test.js:5:10)
at throwMe (.../test.js:32:11)
CustomErrorClass2: with captureStackTrace
at throwMe (.../test.js:32:11)
ES5-style errors would just have no .stack at all, so it was necessary in those.
There was a problem hiding this comment.
@Kovensky in that case I can just pass this.constructor as same argument used in other error classes
There was a problem hiding this comment.
Same comment as here. If you guys are comfortable with these changes I can update the PR without problems.
|
Thanks |
What kind of change does this PR introduce?
ES5 => ES6 refactor
Did you add tests for your changes?
refactor, all existing tests pass
If relevant, link to documentation update:
N/A
Summary
Helping in part of the ES6 refactor
Does this PR introduce a breaking change?
Nope