Skip to content

Commit 4873841

Browse files
authored
fix(minify): preserve destructuring assignment targets (#22839)
1 parent 0602dfe commit 4873841

4 files changed

Lines changed: 180 additions & 33 deletions

File tree

‎lib/javascript/syntax-printer.js‎

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3987,6 +3987,31 @@ const lambdaModifiers = (func) =>
39873987
*/
39883988
const staticSize = (isStatic) => (isStatic ? 7 : 0);
39893989

3990+
/**
3991+
* Whether a destructuring pattern writes an expression: its element,
3992+
* property value, rest or defaulted target.
3993+
* @param {Node} self the expression
3994+
* @param {Compressor} compressor the compressor, `self`'s parent first
3995+
* @param {TerserAst} ast the node kinds
3996+
* @returns {boolean} true when a pattern writes it
3997+
*/
3998+
const isPatternTarget = (self, compressor, ast) => {
3999+
/** @type {Node | NodeShape} */
4000+
let node = self;
4001+
let level = 0;
4002+
let parent = compressor.parent(level);
4003+
while (
4004+
parent &&
4005+
((ast.isObjectPropertyNode(parent) && parent.value === node) ||
4006+
ast.isExpansionNode(parent) ||
4007+
(ast.isDefaultAssignNode(parent) && parent.left === node))
4008+
) {
4009+
node = parent;
4010+
parent = compressor.parent(++level);
4011+
}
4012+
return Boolean(parent) && ast.isDestructuringNode(parent);
4013+
};
4014+
39904015
/**
39914016
* terser's `first_in_statement`: whether the node a walk is at is the first
39924017
* thing its statement prints, so an object or function there needs parentheses.
@@ -24090,7 +24115,8 @@ const installImprove = (modules) => {
2409024115
node.optional ||
2409124116
node.property.name !== "source" ||
2409224117
!A.isRegExpNode(node.object) ||
24093-
isLhs(node, compressor.parent()) ||
24118+
isLhs(compressor.self(), compressor.parent()) ||
24119+
isPatternTarget(compressor.self(), compressor, A) ||
2409424120
!compressor.option("evaluate")
2409524121
) {
2409624122
return undefined;
@@ -31349,16 +31375,12 @@ const installImprove = (modules) => {
3134931375
!Number.isInteger(index) ||
3135031376
index < 0 ||
3135131377
index >= object.elements.length ||
31352-
isLhs(self, parent) ||
31378+
isLhs(compressor.self(), parent) ||
31379+
isPatternTarget(compressor.self(), compressor, A) ||
3135331380
(A.isUnaryPrefixNode(parent) && parent.operator === "delete") ||
3135431381
// Called, it reads the array as its `this`.
3135531382
(A.isCallNode(parent) && parent.callee === self) ||
31356-
A.isPrefixedTemplateStringNode(parent) ||
31357-
(A.isForInNode(parent) && parent.left === self) ||
31358-
A.isDestructuringNode(parent) ||
31359-
A.isDefaultAssignNode(parent) ||
31360-
(A.isObjectKeyValNode(parent) &&
31361-
A.isDestructuringNode(compressor.parent(1)))
31383+
A.isPrefixedTemplateStringNode(parent)
3136231384
) {
3136331385
return undefined;
3136431386
}
@@ -43547,30 +43569,6 @@ const installValueOptimizers = (modules, helpers) => {
4354743569
);
4354843570
};
4354943571

43550-
/**
43551-
* Whether a destructuring pattern writes a name: its element, property
43552-
* value, rest or defaulted target, which `void 0` cannot stand for.
43553-
* @param {Node} self the name
43554-
* @param {Compressor} compressor the compressor, `self`'s parent first
43555-
* @returns {boolean} true when a pattern writes it
43556-
*/
43557-
const isPatternTarget = (self, compressor) => {
43558-
/** @type {Node | NodeShape} */
43559-
let node = self;
43560-
let level = 0;
43561-
let parent = compressor.parent(level);
43562-
while (
43563-
parent &&
43564-
((A.isObjectPropertyNode(parent) && parent.value === node) ||
43565-
A.isExpansionNode(parent) ||
43566-
(A.isDefaultAssignNode(parent) && parent.left === node))
43567-
) {
43568-
node = parent;
43569-
parent = compressor.parent(++level);
43570-
}
43571-
return Boolean(parent) && A.isDestructuringNode(parent);
43572-
};
43573-
4357443572
defineOptimizer("SymbolRef", (self, compressor) => {
4357543573
if (
4357643574
!compressor.option("ie8") &&
@@ -43579,7 +43577,7 @@ const installValueOptimizers = (modules, helpers) => {
4357943577
!(
4358043578
modules.corrections !== undefined &&
4358143579
modules.corrections.enabled &&
43582-
isPatternTarget(self, compressor)
43580+
isPatternTarget(self, compressor, A)
4358343581
)
4358443582
) {
4358543583
switch (self.name) {
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
"use strict";
2+
3+
it("should preserve an array rest assignment target", () => {
4+
let value = 1;
5+
[...[value][0]] = [2, 3];
6+
expect(value).toBe(1);
7+
});
8+
9+
it("should preserve an object rest assignment target", () => {
10+
let value = 1;
11+
({ ...[value][0] } = { answer: 2 });
12+
expect(value).toBe(1);
13+
});
14+
15+
it("should preserve nested rest assignment targets", () => {
16+
let value = 1;
17+
({ answer: [...[value][0]] } = { answer: [2, 3] });
18+
expect(value).toBe(1);
19+
for ([...[value][0]] of [[2, 3]]) {
20+
expect(value).toBe(1);
21+
}
22+
});
23+
24+
it("should preserve ordinary and defaulted assignment targets", () => {
25+
let value = 1;
26+
[[value][0]] = [2];
27+
({ answer: [value][0] } = { answer: 2 });
28+
[[value][0] = 2] = [];
29+
({ answer: [value][0] = 2 } = {});
30+
expect(value).toBe(1);
31+
});
32+
33+
it("should preserve direct writes and loop targets", () => {
34+
let value = 1;
35+
[value][0] = 2;
36+
[value][0]++;
37+
delete [value][0];
38+
for ([value][0] of [2]) {
39+
expect(value).toBe(1);
40+
}
41+
});
42+
43+
it("should still read values in computed keys and defaults", () => {
44+
let value;
45+
({ [["answer"][0]]: value = [2][0] } = {});
46+
expect(value).toBe(2);
47+
({ [["answer"][0]]: value } = { answer: 3 });
48+
expect(value).toBe(3);
49+
});
50+
51+
it("should still read an array element inside a property assignment target", () => {
52+
const value = {};
53+
[value][0].answer = 2;
54+
expect(value.answer).toBe(2);
55+
value[["answer"][0]] = 3;
56+
expect(value.answer).toBe(3);
57+
});
58+
59+
it("should still read array elements in spread expressions", () => {
60+
expect([...[[2, 3]][0]]).toEqual([2, 3]);
61+
expect({ ...[{ answer: 2 }][0] }).toEqual({ answer: 2 });
62+
});
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
"use strict";
2+
3+
it("should preserve direct and computed regular expression property writes", () => {
4+
expect(() => {
5+
/a/.source = "b";
6+
}).toThrow(TypeError);
7+
expect(() => {
8+
/a/["source"] = "b";
9+
}).toThrow(TypeError);
10+
expect(() => {
11+
for (/a/.source of ["b"]) {
12+
throw new Error("unreachable");
13+
}
14+
}).toThrow(TypeError);
15+
});
16+
17+
it("should preserve regular expression properties in assignment patterns", () => {
18+
expect(() => {
19+
({ answer: /a/.source } = { answer: "b" });
20+
}).toThrow(TypeError);
21+
expect(() => {
22+
[/a/.source] = ["b"];
23+
}).toThrow(TypeError);
24+
});
25+
26+
it("should preserve computed regular expression properties in patterns", () => {
27+
expect(() => {
28+
({ answer: /a/["source"] } = { answer: "b" });
29+
}).toThrow(TypeError);
30+
});
31+
32+
it("should preserve regular expression properties in rest targets", () => {
33+
expect(() => {
34+
[.../a/.source] = ["b"];
35+
}).toThrow(TypeError);
36+
expect(() => {
37+
({ .../a/.source } = { answer: "b" });
38+
}).toThrow(TypeError);
39+
});
40+
41+
it("should preserve regular expression properties in defaulted targets", () => {
42+
expect(() => {
43+
[/a/.source = "b"] = [];
44+
}).toThrow(TypeError);
45+
expect(() => {
46+
({ answer: /a/.source = "b" } = {});
47+
}).toThrow(TypeError);
48+
});
49+
50+
it("should preserve regular expression properties in nested loop targets", () => {
51+
expect(() => {
52+
for ({ answer: [.../a/.source] } of [{ answer: ["b"] }]) {
53+
throw new Error("unreachable");
54+
}
55+
}).toThrow(TypeError);
56+
});
57+
58+
it("should still read regular expression properties in keys and defaults", () => {
59+
let value;
60+
({ [/a/.source]: value = /b/.source } = {});
61+
expect(value).toBe("b");
62+
({ [/a/.source]: value } = { a: "c" });
63+
expect(value).toBe("c");
64+
});
65+
66+
it("should still read regular expression properties in spread expressions", () => {
67+
expect([.../ab/.source]).toEqual(["a", "b"]);
68+
expect({ .../ab/.source }).toEqual({ 0: "a", 1: "b" });
69+
});
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
"use strict";
2+
3+
/** @type {import("../../../../").Configuration[]} */
4+
module.exports = ["array", "regexp"].map((name) => ({
5+
mode: "production",
6+
target: ["web", "es2018"],
7+
entry: `./${name}.js`,
8+
experiments: { futureDefaults: true },
9+
output: { module: false },
10+
optimization: {
11+
minimizer: ["..."],
12+
minimize: {
13+
javascript: {
14+
compress: { pure_getters: false, pure_conversions: false }
15+
}
16+
}
17+
}
18+
}));

0 commit comments

Comments
 (0)