Skip to content

Commit 02b85f8

Browse files
rbucktonrbuckton
committed
PR feedback
1 parent de9866f commit 02b85f8

5 files changed

Lines changed: 50 additions & 26 deletions

File tree

‎src/compiler/sourcemap.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ namespace ts {
6767
// Source map data
6868
let sourceMapData: SourceMapData;
6969

70+
// This keeps track of the number of times `disable` has been called without a
71+
// corresponding call to `enable`. As long as this value is non-zero, mappings will not
72+
// be recorded.
73+
// This is primarily used to provide a better experience when debugging binding
74+
// patterns and destructuring assignments for simple expressions.
7075
let disableDepth: number;
7176

7277
return {
@@ -160,12 +165,18 @@ namespace ts {
160165
disableDepth = 0;
161166
}
162167

168+
/**
169+
* Re-enables the recording of mappings.
170+
*/
163171
function enable() {
164172
if (disableDepth > 0) {
165173
disableDepth--;
166174
}
167175
}
168176

177+
/**
178+
* Disables the recording of mappings.
179+
*/
169180
function disable() {
170181
disableDepth++;
171182
}

‎src/compiler/transformers/destructuring.ts‎

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,29 @@ namespace ts {
1818
recordTempVariable: (node: Identifier) => void,
1919
visitor?: (node: Node) => Node) {
2020

21-
let location: TextRange = node;
22-
let value = node.right;
2321
if (isEmptyObjectLiteralOrArrayLiteral(node.left)) {
24-
return value;
22+
return node.right;
2523
}
2624

25+
let location: TextRange = node;
26+
let value = node.right;
2727
const expressions: Expression[] = [];
2828
if (needsValue) {
29-
// Temporary assignment needed to emit root should highlight whole binary expression
30-
value = ensureIdentifier(node.right, /*reuseIdentifierExpressions*/ true, node, emitTempVariableAssignment);
29+
// If the right-hand value of the destructuring assignment needs to be preserved (as
30+
// is the case when the destructuring assignmen) is part of a larger expression),
31+
// then we need to cache the right-hand value.
32+
//
33+
// The source map location for the assignment should point to the entire binary
34+
// expression.
35+
value = ensureIdentifier(node.right, /*reuseIdentifierExpressions*/ true, location, emitTempVariableAssignment);
3136
}
3237
else if (nodeIsSynthesized(node)) {
33-
// Source map node for root.left = root.right is root
34-
// but if root is synthetic, which could be in below case, use the target which is { a }
35-
// for ({a} of {a: string}) {
36-
// }
38+
// Generally, the source map location for a destructuring assignment is the root
39+
// expression.
40+
//
41+
// However, if the root expression is synthesized (as in the case
42+
// of the initializer when transforming a ForOfStatement), then the source map
43+
// location should point to the right-hand value of the expression.
3744
location = node.right;
3845
}
3946

@@ -255,21 +262,22 @@ namespace ts {
255262

256263
function emitArrayLiteralAssignment(target: ArrayLiteralExpression, value: Expression, location: TextRange) {
257264
const elements = target.elements;
258-
if (elements.length !== 1) {
265+
const numElements = elements.length;
266+
if (numElements !== 1) {
259267
// For anything but a single element destructuring we need to generate a temporary
260268
// to ensure value is evaluated exactly once.
261269
// When doing so we want to hightlight the passed in source map node since thats the one needing this temp assignment
262270
value = ensureIdentifier(value, /*reuseIdentifierExpressions*/ true, location, emitTempVariableAssignment);
263271
}
264272

265-
for (let i = 0; i < elements.length; i++) {
273+
for (let i = 0; i < numElements; i++) {
266274
const e = elements[i];
267275
if (e.kind !== SyntaxKind.OmittedExpression) {
268276
// Assignment for target = value.propName should highligh whole property, hence use e as source map node
269277
if (e.kind !== SyntaxKind.SpreadElementExpression) {
270278
emitDestructuringAssignment(e, createElementAccess(value, createLiteral(i)), e);
271279
}
272-
else if (i === elements.length - 1) {
280+
else if (i === numElements - 1) {
273281
emitDestructuringAssignment((<SpreadElementExpression>e).expression, createArraySlice(value, i), e);
274282
}
275283
}
@@ -299,19 +307,19 @@ namespace ts {
299307
// so in that case, we'll intentionally create that temporary.
300308
value = ensureIdentifier(value, /*reuseIdentifierExpressions*/ numElements !== 0, target, emitTempVariableAssignment);
301309
}
302-
for (let i = 0; i < elements.length; i++) {
303-
let element = elements[i];
310+
for (let i = 0; i < numElements; i++) {
311+
const element = elements[i];
304312
if (name.kind === SyntaxKind.ObjectBindingPattern) {
305313
// Rewrite element to a declaration with an initializer that fetches property
306-
let propName = element.propertyName || <Identifier>element.name;
314+
const propName = element.propertyName || <Identifier>element.name;
307315
emitBindingElement(element, createDestructuringPropertyAccess(value, propName));
308316
}
309317
else if (element.kind !== SyntaxKind.OmittedExpression) {
310318
if (!element.dotDotDotToken) {
311319
// Rewrite element to a declaration that accesses array element at index i
312320
emitBindingElement(element, createElementAccess(value, i));
313321
}
314-
else if (i === elements.length - 1) {
322+
else if (i === numElements - 1) {
315323
emitBindingElement(element, createArraySlice(value, i));
316324
}
317325
}
@@ -332,24 +340,31 @@ namespace ts {
332340
);
333341
}
334342

335-
function createDestructuringPropertyAccess(object: Expression, propertyName: PropertyName): LeftHandSideExpression {
343+
/**
344+
* Creates either a PropertyAccessExpression or an ElementAccessExpression for the
345+
* right-hand side of a transformed destructuring assignment.
346+
*
347+
* @param expression The right-hand expression that is the source of the property.
348+
* @param propertyName The destructuring property name.
349+
*/
350+
function createDestructuringPropertyAccess(expression: Expression, propertyName: PropertyName): LeftHandSideExpression {
336351
if (isComputedPropertyName(propertyName)) {
337352
return createElementAccess(
338-
object,
339-
ensureIdentifier(propertyName.expression, /*reuseIdentifierExpressions*/ false, propertyName, emitTempVariableAssignment)
353+
expression,
354+
ensureIdentifier(propertyName.expression, /*reuseIdentifierExpressions*/ false, /*location*/ propertyName, emitTempVariableAssignment)
340355
);
341356
}
342357
else if (isIdentifier(propertyName)) {
343358
return createPropertyAccess(
344-
object,
359+
expression,
345360
propertyName.text
346361
);
347362
}
348363
else {
349364
// We create a synthetic copy of the identifier in order to avoid the rewriting that might
350365
// otherwise occur when the identifier is emitted.
351366
return createElementAccess(
352-
object,
367+
expression,
353368
cloneNode(propertyName)
354369
);
355370
}

‎src/compiler/transformers/es7.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55
namespace ts {
66
// TODO(rbuckton): ES7->ES6 transformer
77
export function transformES7(context: TransformationContext) {
8-
const { hoistVariableDeclaration } = context;
9-
108
return transformSourceFile;
119

1210
function transformSourceFile(node: SourceFile) {

‎src/compiler/transformers/ts.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ namespace ts {
202202

203203
case SyntaxKind.Constructor:
204204
// TypeScript constructors are elided. The constructor of a class will be
205-
// reordered to the start of the member list in `transformClassDeclaration`.
205+
// transformed as part of `transformClassDeclaration`.
206206
return undefined;
207207

208208
case SyntaxKind.ClassDeclaration:
@@ -1964,7 +1964,7 @@ namespace ts {
19641964
)
19651965
);
19661966

1967-
const block = createBlock(statements);
1967+
const block = createBlock(statements, /*location*/ node.body);
19681968

19691969
// Minor optimization, emit `_super` helper to capture `super` access in an arrow.
19701970
// This step isn't needed if we eventually transform this to ES5.

‎src/compiler/utilities.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2792,7 +2792,7 @@ namespace ts {
27922792
else if (kind === SyntaxKind.ConditionalExpression) {
27932793
return isSimpleExpressionWorker((<ConditionalExpression>node).condition, depth + 1)
27942794
&& isSimpleExpressionWorker((<ConditionalExpression>node).whenTrue, depth + 1)
2795-
&& isSimpleExpressionWorker((<ConditionalExpression>node).whenFalse, depth + 1)
2795+
&& isSimpleExpressionWorker((<ConditionalExpression>node).whenFalse, depth + 1);
27962796
}
27972797
else if (kind === SyntaxKind.VoidExpression
27982798
|| kind === SyntaxKind.TypeOfExpression

0 commit comments

Comments
 (0)