Skip to content

Commit 4f48bf8

Browse files
weswighamweswigham
authored
Revised emit for computed property names, including with decorators (#19430)
* Revised emit for computed property names * Fix downlevel name generation scopes * Accept slightly more conservative baseline * First feedback pass * Reduce number of nonrequired variable declarations and assignments * Remove side-effect-free identifier references * skip partially emitted expressions * Comments, move starsOnNewLine to emitNode * Put expressions on newlines when inlined in class expressions for consistency * Update new ref * Fix typo in comment
1 parent ddbd654 commit 4f48bf8

29 files changed

+2777
-113
lines changed

‎src/compiler/binder.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2963,6 +2963,7 @@ namespace ts {
29632963
|| hasModifier(node, ModifierFlags.TypeScriptModifier)
29642964
|| node.typeParameters
29652965
|| node.type
2966+
|| (node.name && isComputedPropertyName(node.name)) // While computed method names aren't typescript, the TS transform must visit them to emit property declarations correctly
29662967
|| !node.body) {
29672968
transformFlags |= TransformFlags.AssertTypeScript;
29682969
}
@@ -2993,6 +2994,7 @@ namespace ts {
29932994
if (node.decorators
29942995
|| hasModifier(node, ModifierFlags.TypeScriptModifier)
29952996
|| node.type
2997+
|| (node.name && isComputedPropertyName(node.name)) // While computed accessor names aren't typescript, the TS transform must visit them to emit property declarations correctly
29962998
|| !node.body) {
29972999
transformFlags |= TransformFlags.AssertTypeScript;
29983000
}

‎src/compiler/emitter.ts

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1733,26 +1733,15 @@ namespace ts {
17331733
increaseIndent();
17341734
}
17351735

1736-
if (getEmitFlags(node) & EmitFlags.ReuseTempVariableScope) {
1737-
emitSignatureHead(node);
1738-
if (onEmitNode) {
1739-
onEmitNode(EmitHint.Unspecified, body, emitBlockCallback);
1740-
}
1741-
else {
1742-
emitBlockFunctionBody(body);
1743-
}
1736+
pushNameGenerationScope(node);
1737+
emitSignatureHead(node);
1738+
if (onEmitNode) {
1739+
onEmitNode(EmitHint.Unspecified, body, emitBlockCallback);
17441740
}
17451741
else {
1746-
pushNameGenerationScope();
1747-
emitSignatureHead(node);
1748-
if (onEmitNode) {
1749-
onEmitNode(EmitHint.Unspecified, body, emitBlockCallback);
1750-
}
1751-
else {
1752-
emitBlockFunctionBody(body);
1753-
}
1754-
popNameGenerationScope();
1742+
emitBlockFunctionBody(body);
17551743
}
1744+
popNameGenerationScope(node);
17561745

17571746
if (indentedFlag) {
17581747
decreaseIndent();
@@ -1871,11 +1860,9 @@ namespace ts {
18711860
emitTypeParameters(node, node.typeParameters);
18721861
emitList(node, node.heritageClauses, ListFormat.ClassHeritageClauses);
18731862

1874-
pushNameGenerationScope();
18751863
write(" {");
18761864
emitList(node, node.members, ListFormat.ClassMembers);
18771865
write("}");
1878-
popNameGenerationScope();
18791866

18801867
if (indentedFlag) {
18811868
decreaseIndent();
@@ -1909,11 +1896,9 @@ namespace ts {
19091896
emitModifiers(node, node.modifiers);
19101897
write("enum ");
19111898
emit(node.name);
1912-
pushNameGenerationScope();
19131899
write(" {");
19141900
emitList(node, node.members, ListFormat.EnumMembers);
19151901
write("}");
1916-
popNameGenerationScope();
19171902
}
19181903

19191904
function emitModuleDeclaration(node: ModuleDeclaration) {
@@ -1935,11 +1920,11 @@ namespace ts {
19351920
}
19361921

19371922
function emitModuleBlock(node: ModuleBlock) {
1938-
pushNameGenerationScope();
1923+
pushNameGenerationScope(node);
19391924
write("{");
19401925
emitBlockStatements(node, /*forceSingleLine*/ isEmptyBlock(node));
19411926
write("}");
1942-
popNameGenerationScope();
1927+
popNameGenerationScope(node);
19431928
}
19441929

19451930
function emitCaseBlock(node: CaseBlock) {
@@ -2284,11 +2269,11 @@ namespace ts {
22842269

22852270
function emitSourceFileWorker(node: SourceFile) {
22862271
const statements = node.statements;
2287-
pushNameGenerationScope();
2272+
pushNameGenerationScope(node);
22882273
emitHelpersIndirect(node);
22892274
const index = findIndex(statements, statement => !isPrologueDirective(statement));
22902275
emitList(node, statements, ListFormat.MultiLine, index === -1 ? statements.length : index);
2291-
popNameGenerationScope();
2276+
popNameGenerationScope(node);
22922277
}
22932278

22942279
// Transformation nodes
@@ -2751,7 +2736,7 @@ namespace ts {
27512736
}
27522737
}
27532738
else {
2754-
return nextNode.startsOnNewLine;
2739+
return getStartsOnNewLine(nextNode);
27552740
}
27562741
}
27572742

@@ -2782,7 +2767,7 @@ namespace ts {
27822767

27832768
function synthesizedNodeStartsOnNewLine(node: Node, format?: ListFormat) {
27842769
if (nodeIsSynthesized(node)) {
2785-
const startsOnNewLine = node.startsOnNewLine;
2770+
const startsOnNewLine = getStartsOnNewLine(node);
27862771
if (startsOnNewLine === undefined) {
27872772
return (format & ListFormat.PreferNewLine) !== 0;
27882773
}
@@ -2799,7 +2784,7 @@ namespace ts {
27992784
node2 = skipSynthesizedParentheses(node2);
28002785

28012786
// Always use a newline for synthesized code if the synthesizer desires it.
2802-
if (node2.startsOnNewLine) {
2787+
if (getStartsOnNewLine(node2)) {
28032788
return true;
28042789
}
28052790

@@ -2858,15 +2843,21 @@ namespace ts {
28582843
/**
28592844
* Push a new name generation scope.
28602845
*/
2861-
function pushNameGenerationScope() {
2846+
function pushNameGenerationScope(node: Node | undefined) {
2847+
if (node && getEmitFlags(node) & EmitFlags.ReuseTempVariableScope) {
2848+
return;
2849+
}
28622850
tempFlagsStack.push(tempFlags);
28632851
tempFlags = 0;
28642852
}
28652853

28662854
/**
28672855
* Pop the current name generation scope.
28682856
*/
2869-
function popNameGenerationScope() {
2857+
function popNameGenerationScope(node: Node | undefined) {
2858+
if (node && getEmitFlags(node) & EmitFlags.ReuseTempVariableScope) {
2859+
return;
2860+
}
28702861
tempFlags = tempFlagsStack.pop();
28712862
}
28722863

@@ -2877,8 +2868,17 @@ namespace ts {
28772868
if (name.autoGenerateKind === GeneratedIdentifierKind.Node) {
28782869
// Node names generate unique names based on their original node
28792870
// and are cached based on that node's id.
2880-
const node = getNodeForGeneratedName(name);
2881-
return generateNameCached(node);
2871+
if (name.skipNameGenerationScope) {
2872+
const savedTempFlags = tempFlags;
2873+
popNameGenerationScope(/*node*/ undefined);
2874+
const result = generateNameCached(getNodeForGeneratedName(name));
2875+
pushNameGenerationScope(/*node*/ undefined);
2876+
tempFlags = savedTempFlags;
2877+
return result;
2878+
}
2879+
else {
2880+
return generateNameCached(getNodeForGeneratedName(name));
2881+
}
28822882
}
28832883
else {
28842884
// Auto, Loop, and Unique names are cached based on their unique

‎src/compiler/factory.ts

Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ namespace ts {
1313
if (updated !== original) {
1414
setOriginalNode(updated, original);
1515
setTextRange(updated, original);
16-
if (original.startsOnNewLine) {
17-
updated.startsOnNewLine = true;
18-
}
1916
aggregateTransformFlags(updated);
2017
}
2118
return updated;
@@ -168,11 +165,14 @@ namespace ts {
168165
}
169166

170167
/** Create a unique name generated for a node. */
171-
export function getGeneratedNameForNode(node: Node): Identifier {
168+
export function getGeneratedNameForNode(node: Node): Identifier;
169+
/*@internal*/ export function getGeneratedNameForNode(node: Node, shouldSkipNameGenerationScope?: boolean): Identifier;
170+
export function getGeneratedNameForNode(node: Node, shouldSkipNameGenerationScope?: boolean): Identifier {
172171
const name = createIdentifier("");
173172
name.autoGenerateKind = GeneratedIdentifierKind.Node;
174173
name.autoGenerateId = nextAutoGenerateId;
175174
name.original = node;
175+
name.skipNameGenerationScope = !!shouldSkipNameGenerationScope;
176176
nextAutoGenerateId++;
177177
return name;
178178
}
@@ -2683,6 +2683,24 @@ namespace ts {
26832683
return node;
26842684
}
26852685

2686+
/**
2687+
* Gets a custom text range to use when emitting comments.
2688+
*/
2689+
/*@internal*/
2690+
export function getStartsOnNewLine(node: Node) {
2691+
const emitNode = node.emitNode;
2692+
return emitNode && emitNode.startsOnNewLine;
2693+
}
2694+
2695+
/**
2696+
* Sets a custom text range to use when emitting comments.
2697+
*/
2698+
/*@internal*/
2699+
export function setStartsOnNewLine<T extends Node>(node: T, newLine: boolean) {
2700+
getOrCreateEmitNode(node).startsOnNewLine = newLine;
2701+
return node;
2702+
}
2703+
26862704
/**
26872705
* Gets a custom text range to use when emitting comments.
26882706
*/
@@ -2841,7 +2859,8 @@ namespace ts {
28412859
sourceMapRange,
28422860
tokenSourceMapRanges,
28432861
constantValue,
2844-
helpers
2862+
helpers,
2863+
startsOnNewLine,
28452864
} = sourceEmitNode;
28462865
if (!destEmitNode) destEmitNode = {};
28472866
// We are using `.slice()` here in case `destEmitNode.leadingComments` is pushed to later.
@@ -2853,6 +2872,7 @@ namespace ts {
28532872
if (tokenSourceMapRanges) destEmitNode.tokenSourceMapRanges = mergeTokenSourceMapRanges(tokenSourceMapRanges, destEmitNode.tokenSourceMapRanges);
28542873
if (constantValue !== undefined) destEmitNode.constantValue = constantValue;
28552874
if (helpers) destEmitNode.helpers = addRange(destEmitNode.helpers, helpers);
2875+
if (startsOnNewLine !== undefined) destEmitNode.startsOnNewLine = startsOnNewLine;
28562876
return destEmitNode;
28572877
}
28582878

@@ -3014,7 +3034,7 @@ namespace ts {
30143034

30153035
if (children.length > 1) {
30163036
for (const child of children) {
3017-
child.startsOnNewLine = true;
3037+
startOnNewLine(child);
30183038
argumentsList.push(child);
30193039
}
30203040
}
@@ -3045,7 +3065,7 @@ namespace ts {
30453065
if (children && children.length > 0) {
30463066
if (children.length > 1) {
30473067
for (const child of children) {
3048-
child.startsOnNewLine = true;
3068+
startOnNewLine(child);
30493069
argumentsList.push(child);
30503070
}
30513071
}
@@ -3620,8 +3640,8 @@ namespace ts {
36203640
);
36213641
setOriginalNode(updated, node);
36223642
setTextRange(updated, node);
3623-
if (node.startsOnNewLine) {
3624-
updated.startsOnNewLine = true;
3643+
if (getStartsOnNewLine(node)) {
3644+
setStartsOnNewLine(updated, /*newLine*/ true);
36253645
}
36263646
aggregateTransformFlags(updated);
36273647
return updated;
@@ -4250,8 +4270,7 @@ namespace ts {
42504270
}
42514271

42524272
export function startOnNewLine<T extends Node>(node: T): T {
4253-
node.startsOnNewLine = true;
4254-
return node;
4273+
return setStartsOnNewLine(node, /*newLine*/ true);
42554274
}
42564275

42574276
export function getExternalHelpersModuleName(node: SourceFile) {

‎src/compiler/transformers/es2015.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -787,9 +787,7 @@ namespace ts {
787787
// To preserve the behavior of the old emitter, we explicitly indent
788788
// the body of the function here if it was requested in an earlier
789789
// transformation.
790-
if (getEmitFlags(node) & EmitFlags.Indented) {
791-
setEmitFlags(classFunction, EmitFlags.Indented);
792-
}
790+
setEmitFlags(classFunction, (getEmitFlags(node) & EmitFlags.Indented) | EmitFlags.ReuseTempVariableScope);
793791

794792
// "inner" and "outer" below are added purely to preserve source map locations from
795793
// the old emitter
@@ -1327,7 +1325,8 @@ namespace ts {
13271325
EmitFlags.SingleLine | EmitFlags.NoTrailingSourceMap | EmitFlags.NoTokenSourceMaps
13281326
)
13291327
);
1330-
statement.startsOnNewLine = true;
1328+
1329+
startOnNewLine(statement);
13311330
setTextRange(statement, parameter);
13321331
setEmitFlags(statement, EmitFlags.NoTokenSourceMaps | EmitFlags.NoTrailingSourceMap | EmitFlags.CustomPrologue);
13331332
statements.push(statement);
@@ -1683,7 +1682,7 @@ namespace ts {
16831682
]
16841683
);
16851684
if (startsOnNewLine) {
1686-
call.startsOnNewLine = true;
1685+
startOnNewLine(call);
16871686
}
16881687

16891688
exitSubtree(ancestorFacts, HierarchyFacts.PropagateNewTargetMask, hierarchyFacts & HierarchyFacts.PropagateNewTargetMask ? HierarchyFacts.NewTarget : HierarchyFacts.None);
@@ -2602,7 +2601,7 @@ namespace ts {
26022601
);
26032602

26042603
if (node.multiLine) {
2605-
assignment.startsOnNewLine = true;
2604+
startOnNewLine(assignment);
26062605
}
26072606

26082607
expressions.push(assignment);
@@ -3083,7 +3082,7 @@ namespace ts {
30833082
);
30843083
setTextRange(expression, property);
30853084
if (startsOnNewLine) {
3086-
expression.startsOnNewLine = true;
3085+
startOnNewLine(expression);
30873086
}
30883087
return expression;
30893088
}
@@ -3105,7 +3104,7 @@ namespace ts {
31053104
);
31063105
setTextRange(expression, property);
31073106
if (startsOnNewLine) {
3108-
expression.startsOnNewLine = true;
3107+
startOnNewLine(expression);
31093108
}
31103109
return expression;
31113110
}
@@ -3128,7 +3127,7 @@ namespace ts {
31283127
);
31293128
setTextRange(expression, method);
31303129
if (startsOnNewLine) {
3131-
expression.startsOnNewLine = true;
3130+
startOnNewLine(expression);
31323131
}
31333132
exitSubtree(ancestorFacts, HierarchyFacts.PropagateNewTargetMask, hierarchyFacts & HierarchyFacts.PropagateNewTargetMask ? HierarchyFacts.NewTarget : HierarchyFacts.None);
31343133
return expression;

‎src/compiler/transformers/generators.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,7 +1077,7 @@ namespace ts {
10771077
const visited = visitNode(expression, visitor, isExpression);
10781078
if (visited) {
10791079
if (multiLine) {
1080-
visited.startsOnNewLine = true;
1080+
startOnNewLine(visited);
10811081
}
10821082
expressions.push(visited);
10831083
}
@@ -2683,8 +2683,7 @@ namespace ts {
26832683
if (clauses) {
26842684
const labelExpression = createPropertyAccess(state, "label");
26852685
const switchStatement = createSwitch(labelExpression, createCaseBlock(clauses));
2686-
switchStatement.startsOnNewLine = true;
2687-
return [switchStatement];
2686+
return [startOnNewLine(switchStatement)];
26882687
}
26892688

26902689
if (statements) {

0 commit comments

Comments
 (0)