-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
this
before super()
is a runtime error, not a static one.
#6467
this
before super()
is a runtime error, not a static one.
#6467
Conversation
14f35c8
to
84569c0
Compare
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/6106/ |
5233320
to
d404b39
Compare
Rebased |
); | ||
if (this.isDerived) { | ||
if (path.parentPath.isMemberExpression({ object: path.node })) { | ||
// In cases like this.foo or this[foo], there is no need to ass |
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.
Super small nit: extraneous ass
:)
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.
Lol I meant "add" 🤣
I will correct it later and then merge the pr.
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.
@nicolo-ribaudo lol sounds good
@@ -500,11 +500,23 @@ helpers.objectWithoutProperties = defineHelper(` | |||
} | |||
`); | |||
|
|||
helpers.assertThisInitialized = defineHelper(` | |||
export default function _assertThisInitialized(self) { | |||
if (!self) { |
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.
I would prefer to be more explicit in the test here, like: typeof self === 'undefined'
. It's faster.
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.
Which is faster? typeof self === "undefined"
or self === void 0
?
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.
I set up a not too much significative jsperf (https://jsperf.com/x-vs-typeof-vs-void-0/1) and they are almost the same. I'll stick with the second one since it is shorter.
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.
Great, thanks!
@@ -36,7 +36,7 @@ const verifyConstructorVisitor = visitors.merge([ | |||
ThisExpression(path) { | |||
if (this.isDerived) { | |||
if (path.parentPath.isMemberExpression({ object: path.node })) { | |||
// In cases like this.foo or this[foo], there is no need to ass |
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.
👍
This errors were all thrown at compile-time, but they should be runtime errors:
I have mixed feelings about 14f35c8: it makes the output faster and smaller, but it generates an inconsistent error message (
this.foo; super()
throws something like "Can't read property foo of undefined" instead of "this is not initialized")