-
Notifications
You must be signed in to change notification settings - Fork 13.2k
Fix emit for leading 'var' declarations for synthesized namespaces #17631
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
Merged
Merged
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
b44ac91
Added failing test for a before-transform that indirectly replaces a …
DanielRosenwasser 5cb5cf1
Accepted baselines.
DanielRosenwasser 9f1b747
Made the first-declaration check conservative in the TypeScript trans…
DanielRosenwasser 22e0d9f
Accepted baselines.
DanielRosenwasser 6ef27a4
Added test for class/namespace merging with an ESNext target.
DanielRosenwasser 66e2a0b
Accepted baselines.
DanielRosenwasser 18cced9
Added test.
DanielRosenwasser 33a036b
Accepted baselines.
DanielRosenwasser a51397e
Just track the local names of identifiers instead of ever using symbols.
DanielRosenwasser 6e60a01
Accepted baselines.
DanielRosenwasser 281d821
Fix lint errors.
DanielRosenwasser b702062
Addressed code review feedback.
DanielRosenwasser File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2639,9 +2639,6 @@ namespace ts { | |
| /** | ||
| * Records that a declaration was emitted in the current scope, if it was the first | ||
| * declaration for the provided symbol. | ||
| * | ||
| * NOTE: if there is ever a transformation above this one, we may not be able to rely | ||
| * on symbol names. | ||
| */ | ||
| function recordEmittedDeclarationInScope(node: Node) { | ||
| const name = node.symbol && node.symbol.escapedName; | ||
|
|
@@ -2657,18 +2654,22 @@ namespace ts { | |
| } | ||
|
|
||
| /** | ||
| * Determines whether a declaration is the first declaration with the same name emitted | ||
| * in the current scope. | ||
| * Determines whether a declaration is *could* be the first declaration with | ||
| * the same name emitted in the current scope. Only returns false if we are absolutely | ||
| * certain a previous declaration has been emitted. | ||
| */ | ||
| function isFirstEmittedDeclarationInScope(node: Node) { | ||
| function isPotentiallyFirstEmittedDeclarationInScope(node: Node) { | ||
| // If the node has a named symbol, then we have enough knowledge to determine | ||
| // whether a prior declaration has been emitted. | ||
|
||
| if (currentScopeFirstDeclarationsOfName) { | ||
| const name = node.symbol && node.symbol.escapedName; | ||
| if (name) { | ||
| return currentScopeFirstDeclarationsOfName.get(name) === node; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| // Otherwise, we can't be sure. For example, this node could be synthetic. | ||
| return true; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -2690,7 +2691,7 @@ namespace ts { | |
| setOriginalNode(statement, node); | ||
|
|
||
| recordEmittedDeclarationInScope(node); | ||
| if (isFirstEmittedDeclarationInScope(node)) { | ||
| if (isPotentiallyFirstEmittedDeclarationInScope(node)) { | ||
| // Adjust the source map emit to match the old emitter. | ||
| if (node.kind === SyntaxKind.EnumDeclaration) { | ||
| setSourceMapRange(statement.declarationList, node); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ enum void { | |
| } | ||
|
|
||
| //// [parserEnumDeclaration4.js] | ||
| var ; | ||
| (function () { | ||
| })( || ( = {})); | ||
| void {}; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ debugger; | |
| if () | ||
| ; | ||
| [1, 2]; | ||
| var ; | ||
| (function () { | ||
| })( || ( = {})); | ||
| void {}; | ||
4 changes: 4 additions & 0 deletions
4
tests/baselines/reference/transformApi/transformsCorrectly.synthesizedNamespace.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| var Reflect; | ||
| (function (Reflect) { | ||
| var x = 1; | ||
| })(Reflect || (Reflect = {})); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.