-
Notifications
You must be signed in to change notification settings - Fork 116
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
chore(deps): Upgrade to Babel 7 #1047
Conversation
Verified that @jstoffan has signed the CLA. Thanks for the pull request! |
// Pass remaining arguments to parent constructor | ||
super(...params); | ||
super(message, ...params); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the parent constructor wants [message, fileName, lineNumber], you can probably get rid of ...params since latter two are optional and not passed in for PreviewErrors usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I didn't see them passed in anywhere, but I wanted to stay as close to the original implementation as possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also PreviewError's 2nd arg is the message, aka displayMessage
, so shouldn't it be super(displayMessage)? And if so, maybe you wont need line 45
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope. If both are passed in, this.message
should still be the original message, not the display message.
I had to modify the PreviewError class slightly because
this.message
didn't seem to be getting set properly after being transpiled. It looks like modifying built-ins the way we have can be a little tricky.