Skip to content

Commit 7393dba

Browse files
committed
Fix destructuring evaluation order for initializers
1 parent 83d02a5 commit 7393dba

File tree

40 files changed

+599
-376
lines changed

40 files changed

+599
-376
lines changed

src/compiler/transformers/destructuring.ts

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ namespace ts {
55
level: FlattenLevel;
66
downlevelIteration: boolean;
77
hoistTempVariables: boolean;
8+
hasTransformedPriorElement?: boolean; // indicates whether we've transformed a prior declaration
89
emitExpression: (value: Expression) => void;
910
emitBindingOrAssignment: (target: BindingOrAssignmentElementTarget, value: Expression, location: TextRange, original: Node | undefined) => void;
1011
createArrayBindingOrAssignmentPattern: (elements: BindingOrAssignmentElement[]) => ArrayBindingOrAssignmentPattern;
@@ -265,18 +266,27 @@ namespace ts {
265266
value: Expression | undefined,
266267
location: TextRange,
267268
skipInitializer?: boolean) {
269+
const bindingTarget = getTargetOfBindingOrAssignmentElement(element)!; // TODO: GH#18217
268270
if (!skipInitializer) {
269271
const initializer = visitNode(getInitializerOfBindingOrAssignmentElement(element), flattenContext.visitor, isExpression);
270272
if (initializer) {
271273
// Combine value and initializer
272-
value = value ? createDefaultValueCheck(flattenContext, value, initializer, location) : initializer;
274+
if (value) {
275+
value = createDefaultValueCheck(flattenContext, value, initializer, location);
276+
// If 'value' is not a simple expression, it could contain side-effecting code that should evaluate before an object or array binding pattern.
277+
if (!isSimpleInlineableExpression(initializer) && isBindingOrAssignmentPattern(bindingTarget)) {
278+
value = ensureIdentifier(flattenContext, value, /*reuseIdentifierExpressions*/ true, location);
279+
}
280+
}
281+
else {
282+
value = initializer;
283+
}
273284
}
274285
else if (!value) {
275286
// Use 'void 0' in absence of value and initializer
276287
value = flattenContext.context.factory.createVoidZero();
277288
}
278289
}
279-
const bindingTarget = getTargetOfBindingOrAssignmentElement(element)!; // TODO: GH#18217
280290
if (isObjectBindingOrAssignmentPattern(bindingTarget)) {
281291
flattenObjectBindingOrAssignmentPattern(flattenContext, element, bindingTarget, value!, location);
282292
}
@@ -393,7 +403,8 @@ namespace ts {
393403
if (flattenContext.level >= FlattenLevel.ObjectRest) {
394404
// If an array pattern contains an ObjectRest, we must cache the result so that we
395405
// can perform the ObjectRest destructuring in a different declaration
396-
if (element.transformFlags & TransformFlags.ContainsObjectRestOrSpread) {
406+
if (element.transformFlags & TransformFlags.ContainsObjectRestOrSpread || flattenContext.hasTransformedPriorElement && !isSimpleBindingOrAssignmentElement(element)) {
407+
flattenContext.hasTransformedPriorElement = true;
397408
const temp = flattenContext.context.factory.createTempVariable(/*recordTempVariable*/ undefined);
398409
if (flattenContext.hoistTempVariables) {
399410
flattenContext.context.hoistVariableDeclaration(temp);
@@ -428,6 +439,17 @@ namespace ts {
428439
}
429440
}
430441

442+
function isSimpleBindingOrAssignmentElement(element: BindingOrAssignmentElement): boolean {
443+
const target = getTargetOfBindingOrAssignmentElement(element);
444+
if (!target || isOmittedExpression(target)) return true;
445+
const propertyName = tryGetPropertyNameOfBindingOrAssignmentElement(element);
446+
if (propertyName && !isPropertyNameLiteral(propertyName)) return false;
447+
const initializer = getInitializerOfBindingOrAssignmentElement(element);
448+
if (initializer && !isSimpleInlineableExpression(initializer)) return false;
449+
if (isBindingOrAssignmentPattern(target)) return every(getElementsOfBindingOrAssignmentPattern(target), isSimpleBindingOrAssignmentElement);
450+
return isIdentifier(target);
451+
}
452+
431453
/**
432454
* Creates an expression used to provide a default value if a value is `undefined` at runtime.
433455
*

src/testRunner/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@
8989
"unittests/evaluation/asyncArrow.ts",
9090
"unittests/evaluation/asyncGenerator.ts",
9191
"unittests/evaluation/awaiter.ts",
92+
"unittests/evaluation/destructuring.ts",
9293
"unittests/evaluation/forAwaitOf.ts",
9394
"unittests/evaluation/forOf.ts",
9495
"unittests/evaluation/optionalCall.ts",
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
describe("unittests:: evaluation:: destructuring", () => {
2+
// https://github.com/microsoft/TypeScript/issues/39205
3+
describe("correct order for array destructuring evaluation and initializers", () => {
4+
it("when element is undefined", () => {
5+
const result = evaluator.evaluateTypeScript(`
6+
export const output: any[] = [];
7+
const order = (n: any): any => output.push(n);
8+
let [{ [order(1)]: x } = order(0)] = [];
9+
`, { target: ts.ScriptTarget.ES5 });
10+
assert.deepEqual(result.output, [0, 1]);
11+
});
12+
it("when element is defined", async () => {
13+
const result = evaluator.evaluateTypeScript(`
14+
export const output: any[] = [];
15+
const order = (n: any): any => output.push(n);
16+
let [{ [order(1)]: x } = order(0)] = [{}];
17+
`, { target: ts.ScriptTarget.ES5 });
18+
assert.deepEqual(result.output, [1]);
19+
});
20+
});
21+
describe("correct order for array destructuring evaluation and initializers with spread", () => {
22+
it("ES5", () => {
23+
const result = evaluator.evaluateTypeScript(`
24+
export const output: any[] = [];
25+
const order = (n: any): any => output.push(n);
26+
let { [order(0)]: { [order(2)]: z } = order(1), ...w } = {} as any;
27+
`, { target: ts.ScriptTarget.ES5 });
28+
assert.deepEqual(result.output, [0, 1, 2]);
29+
});
30+
it("ES2015", () => {
31+
const result = evaluator.evaluateTypeScript(`
32+
export const output: any[] = [];
33+
const order = (n: any): any => output.push(n);
34+
let { [order(0)]: { [order(2)]: z } = order(1), ...w } = {} as any;
35+
`, { target: ts.ScriptTarget.ES2015 });
36+
assert.deepEqual(result.output, [0, 1, 2]);
37+
});
38+
});
39+
});

tests/baselines/reference/arrowFunctionExpressions.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ var p8 = function (_a) {
137137
var _b = _a.a, a = _b === void 0 ? 1 : _b;
138138
};
139139
var p9 = function (_a) {
140-
var _b = _a.a, _c = (_b === void 0 ? { b: 1 } : _b).b, b = _c === void 0 ? 1 : _c;
140+
var _b = _a.a, _c = _b === void 0 ? { b: 1 } : _b, _d = _c.b, b = _d === void 0 ? 1 : _d;
141141
};
142142
var p10 = function (_a) {
143143
var _b = _a[0], value = _b.value, done = _b.done;

tests/baselines/reference/contextualTypeForInitalizedVariablesFiltersUndefined.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,15 @@ function fst({ s } = t) { }
1212
//// [contextualTypeForInitalizedVariablesFiltersUndefined.js]
1313
"use strict";
1414
var fInferred = function (_a) {
15-
var _b = (_a === void 0 ? {} : _a).a, a = _b === void 0 ? 0 : _b;
15+
var _b = _a === void 0 ? {} : _a, _c = _b.a, a = _c === void 0 ? 0 : _c;
1616
return a;
1717
};
1818
// const fInferred: ({ a }?: { a?: number; }) => number
1919
var fAnnotated = function (_a) {
20-
var _b = (_a === void 0 ? {} : _a).a, a = _b === void 0 ? 0 : _b;
20+
var _b = _a === void 0 ? {} : _a, _c = _b.a, a = _c === void 0 ? 0 : _c;
2121
return a;
2222
};
2323
var s = t.s;
2424
function fst(_a) {
25-
var s = (_a === void 0 ? t : _a).s;
25+
var _b = _a === void 0 ? t : _a, s = _b.s;
2626
}

tests/baselines/reference/contextuallyTypedIife.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,11 @@ let eleven = (o => o.a(11))({ a: function(n) { return n; } });
9595
return p;
9696
})({ p: 15 });
9797
(function (_a) {
98-
var _b = (_a === void 0 ? { r: 18 } : _a).r, r = _b === void 0 ? 17 : _b;
98+
var _b = _a === void 0 ? { r: 18 } : _a, _c = _b.r, r = _c === void 0 ? 17 : _c;
9999
return r;
100100
})({ r: 19 });
101101
(function (_a) {
102-
var _b = (_a === void 0 ? { u: 23 } : _a).u, u = _b === void 0 ? 22 : _b;
102+
var _b = _a === void 0 ? { u: 23 } : _a, _c = _b.u, u = _c === void 0 ? 22 : _c;
103103
return u;
104104
})();
105105
// contextually typed parameters.

tests/baselines/reference/contextuallyTypedIifeStrict.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,11 @@ let eleven = (o => o.a(11))({ a: function(n) { return n; } });
9595
return p;
9696
})({ p: 15 });
9797
(function (_a) {
98-
var _b = (_a === void 0 ? { r: 18 } : _a).r, r = _b === void 0 ? 17 : _b;
98+
var _b = _a === void 0 ? { r: 18 } : _a, _c = _b.r, r = _c === void 0 ? 17 : _c;
9999
return r;
100100
})({ r: 19 });
101101
(function (_a) {
102-
var _b = (_a === void 0 ? { u: 23 } : _a).u, u = _b === void 0 ? 22 : _b;
102+
var _b = _a === void 0 ? { u: 23 } : _a, _c = _b.u, u = _c === void 0 ? 22 : _c;
103103
return u;
104104
})();
105105
// contextually typed parameters.

tests/baselines/reference/crashInGetTextOfComputedPropertyName.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ typeof itemOk1; // pass
4141
var objWithItems = { items: {} };
4242
var itemOk2 = objWithItems.items[itemId];
4343
typeof itemOk2; // pass
44-
var _c = objWithItems, _d = _c.items /*happens when default value is provided*/, _e = itemId, itemWithTSError = (_d === void 0 ? {} /*happens when default value is provided*/ : _d)[_e];
44+
var _c = objWithItems, _d = _c.items /*happens when default value is provided*/, _e = _d === void 0 ? {} : _d /*happens when default value is provided*/, _f = itemId, itemWithTSError = _e[_f];
4545
// in order to re-produce the error, uncomment next line:
4646
typeof itemWithTSError; // :(
4747
// will result in:

tests/baselines/reference/declarationEmitBindingPatterns.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ var a;
1313
function f(_a, _b, _c) {
1414
_a = a;
1515
_b = a;
16-
var _d = (_c === void 0 ? a : _c).p, _e = _d === void 0 ? a : _d;
16+
var _d = _c === void 0 ? a : _c, _e = _d.p, _f = _e === void 0 ? a : _e, _g = _f;
1717
}
1818

1919

tests/baselines/reference/declarationEmitDestructuring4.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ function baz1(_a) {
2020
_a = [1, 2, 3];
2121
}
2222
function baz2(_a) {
23-
var _b = (_a === void 0 ? [[1, 2, 3]] : _a)[0];
23+
var _b = _a === void 0 ? [[1, 2, 3]] : _a, _c = _b[0];
2424
}
2525
function baz3(_a) { }
2626
function baz4(_a) {

0 commit comments

Comments
 (0)