Repository navigation
Fix #22866: Condition checking for name collision of generated variable names on emit happening - #22886
Conversation
…le names on emit happening
|
Looks like there is lint :( |
Nathan Shively-Sanders (sandersn)
left a comment
Nathan Shively-Sanders (sandersn)
left a comment
There was a problem hiding this comment.
Looks good, just one question about the tests.
|
|
||
|
|
||
| // shadowed require | ||
| var require = 0; |
There was a problem hiding this comment.
are these actually shadowed? I thought they came from @types/node. In tests I've written, I've had trouble getting them to be declared instead of the other way round.
There was a problem hiding this comment.
it is just that the emitter expects them to exist and to mean something specific when --module is AMD or CommonJS. this ensures that users do not accidentally define a local variable with the same name.
Wesley Wigham (weswigham)
left a comment
Wesley Wigham (weswigham)
left a comment
There was a problem hiding this comment.
This looks like an improvement; but shouldn't we just start using (optimistic) unique names for these temporaries in our emit (so we just use a different variable if there would be a conflict)?
3 participants
Fixes #22866