-
-
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
Enable the noImplicitThis
tsc option
#14545
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/51973/ |
noImplicitThis
tsc optionsnoImplicitThis
tsc option
@@ -127,7 +129,7 @@ export default declare((api, options: Options) => { | |||
AssignmentExpression(path) { | |||
const left = path.get("left"); | |||
if (left.isIdentifier()) { | |||
const localName = path.node.name; | |||
const localName = left.node.name; |
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 is a bug fix. Babel does not replace module
in module = {}
to assertion errors because path.node.name
is always undefined
.
throw new Error("The CommonJS '" + "exports" + "' variable is not available in ES6 modules." + "Consider setting setting sourceType:script or sourceType:unambiguous in your " + "Babel config for this file."); | ||
}() + 4, function () { | ||
throw new Error("The CommonJS '" + "exports" + "' variable is not available in ES6 modules." + "Consider setting setting sourceType:script or sourceType:unambiguous in your " + "Babel config for this file."); | ||
}()); |
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 wonder if we can avoid emitting the error twice. I think it's duplicated because exports += 4
is handled like exports = exports + 4
, so we emit one error for the read and one for the write.
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 is super low priority: since the generated code always throw, it's will almost never be included in production code.
ed94e39
to
efc934e
Compare
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 highlighted below the code differences (everything else are only type changes).
if (!arg.isIdentifier()) return; | ||
const localName = arg.node.name; | ||
if (localName !== "module" && localName !== "exports") return; |
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 code change doesn't have any effect on the result, other than making the code "sound". If arg
is not an identifier, arg.node.name
is undefined and thus we returned.
|
||
let type = this._getTypeAnnotation() || anyTypeAnnotation(); | ||
export function getTypeAnnotation(this: NodePath): t.FlowType { | ||
let type = this.getData("typeAnnotation"); |
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.
With this code change we use the internal map storage that NodePath
supports, rather than just adding an extra property on the path
object.
efc934e
to
dca76c4
Compare
dca76c4
to
47a8009
Compare
In this PR we enable the
noImplicitThis
compiler option and fixes all the new typing errors.This PR is based on #14530, whose changes implicitly fix
noImplicitThis
errors.