Enable strictNullChecks for @babel/types#17609
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/60699 |
| expect(cloned.declarations[0].id.leadingComments[0].loc).toBe(undefined); | ||
| expect(cloned.declarations[0].id.innerComments[0].loc).toBe(undefined); | ||
| expect(cloned.declarations[0].id.trailingComments[0].loc).toBe(undefined); |
There was a problem hiding this comment.
This is a behavioral change, but it should be fine.
|
commit: |
be1e8d4 to
9db6682
Compare
|
Any chance you could rebase? :) |
9db6682 to
d27f8e9
Compare
| if (!IS_STANDALONE) { | ||
| if (!new Error().stack.includes("regenerator")) { | ||
| if (!new Error().stack!.includes("regenerator")) { | ||
| throw error; | ||
| } | ||
| } |
There was a problem hiding this comment.
regenerator has been archived, and we've forked it, so perhaps it's time to remove this check.
There was a problem hiding this comment.
Yeah, agree. Separate PR though.
| defineType("ExportAllDeclaration", { | ||
| builder: ["source"], | ||
| visitor: ["source", "attributes", "assertions"], | ||
| visitor: ["source", "assertions"], |
There was a problem hiding this comment.
Mmm, shouldn't it be just attributes?
There was a problem hiding this comment.
Oops, I got it backwards.
| params, | ||
| body, | ||
| async, | ||
| //@ts-expect-error FIXME in Babel 8 |
There was a problem hiding this comment.
Should we add them?
There was a problem hiding this comment.
Yeah we should probably add them to the arguments if they are required. Or give defaults where it makes sense (probably default to false for the booleans).
At least for arrowFunctionExpression, it'd be nice to have some custom logic that checks the type of the body and defines it.
| expression: { | ||
| // https://github.com/babel/babylon/issues/505 | ||
| // | ||
| // NOTE: In the generated builder we compute the value of this field based | ||
| // on the body. | ||
| validate: assertValueType("boolean"), | ||
| }, |
There was a problem hiding this comment.
Can we remove this attribute?
There was a problem hiding this comment.
It unfortunately is the simplest way to check if we are dealing with a shorthand arrow function or one with the full body, so I feat that at least direct users of the parser might be relying on it.
There was a problem hiding this comment.
I checked the parser, and it's only used in the estree plugin.
Also, it seems to simply check if body.type is BlockStatement.
There was a problem hiding this comment.
Mh ok, let's do it in a new PR to get the breaking change changelog entry and document it in the migratio nguide.
ad3f9db to
77ae69f
Compare
Fixes #1, Fixes #2