Skip to content

Commit abb4fa7

Browse files
feat(minifier): merge calls through a member path and read null != a && a.b as a?.b for truth
Two calls differing in one argument merge where reading the callee first cannot be seen by the test. The merged conditional is no longer optimized in the context of the one it replaces, which read a value as a truth, and a `const` exported by name stays `const`. Claude-Session: https://claude.ai/code/session_01ViD41i2KdwwGxrDk7yrjeD
1 parent 58cf99d commit abb4fa7

2 files changed

Lines changed: 192 additions & 14 deletions

File tree

‎lib/javascript/syntax-printer.js‎

Lines changed: 156 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21722,21 +21722,143 @@ const installImprove = (modules) => {
2172221722
}
2172321723
}
2172421724
if (index === -1) return;
21725-
const merged = A.optimize(
21726-
makeNode(A.ConditionalNode, self, {
21727-
test: self.test,
21728-
consequent: first[index].argument,
21729-
alternate: second[index].argument
21730-
}),
21731-
compressor
21732-
);
21725+
// Optimized here, it would read the context of the conditional it replaces.
21726+
const merged = makeNode(A.ConditionalNode, self, {
21727+
test: self.test,
21728+
consequent: first[index].argument,
21729+
alternate: second[index].argument
21730+
});
2173321731
const call = A.cloneNode(/** @type {Node} */ (consequent));
2173421732
const spread = A.cloneNode(first[index]);
2173521733
spread.argument = merged;
2173621734
call.arguments[index] = spread;
2173721735
return call;
2173821736
};
2173921737

21738+
/**
21739+
* Whether a name's binding is one no code run elsewhere can rebind or see:
21740+
* its own function's, every reference in that function, and no `eval`.
21741+
* @param {Node} node a name read
21742+
* @param {Compressor} compressor the compressor
21743+
* @returns {boolean} true when it is
21744+
*/
21745+
const readsOwnLocal = (node, compressor) => {
21746+
const definition = node.definition;
21747+
if (
21748+
!definition ||
21749+
definition.global ||
21750+
definition.scope.isPinned() ||
21751+
!readsAlike(node, compressor)
21752+
) {
21753+
return false;
21754+
}
21755+
const functionScope = definition.scope.getDefunScope();
21756+
return definition.references.every(
21757+
(/** @type {Node} */ reference) =>
21758+
reference.scope.getDefunScope() === functionScope
21759+
);
21760+
};
21761+
21762+
/**
21763+
* Whether reading a callee before a conditional's test, not after, runs
21764+
* alike: a function or `var` nothing writes, or, where the test reads only
21765+
* its own function's names and runs nothing, any name or member path.
21766+
* @param {Node} callee the callee both calls share
21767+
* @param {Node} test the conditional's test
21768+
* @param {Compressor} compressor the compressor
21769+
* @returns {boolean} true when it does
21770+
*/
21771+
const readsCalleeAside = (callee, test, compressor) => {
21772+
if (A.isSymbolRefNode(callee) && callee.definition) {
21773+
const definition = callee.definition;
21774+
const declared = definition.identifiers[0];
21775+
if (
21776+
compressor.option("reduce_vars") &&
21777+
readsAlike(callee, compressor) &&
21778+
(A.isSymbolDefunNode(declared) ||
21779+
A.isSymbolVarNode(declared) ||
21780+
A.isSymbolFunargNode(declared)) &&
21781+
!definition.assignments &&
21782+
!definition.patternWrites &&
21783+
definition.fixed
21784+
) {
21785+
return true;
21786+
}
21787+
}
21788+
let path = callee;
21789+
while (A.isPropAccessNode(path) && !path.optional) {
21790+
if (A.isSubNode(path) && !A.isConstantNode(path.property)) return false;
21791+
path = path.object;
21792+
}
21793+
if (!A.isSymbolRefNode(path) && !A.isThisNode(path)) return false;
21794+
if (A.hasSideEffects(test, compressor)) return false;
21795+
let ownLocals = true;
21796+
walkNode(
21797+
test,
21798+
A.createWalker((/** @type {Node} */ node) => {
21799+
if (!ownLocals) return true;
21800+
if (A.isLambdaNode(node)) return (ownLocals = false);
21801+
if (A.isSymbolRefNode(node) && !readsOwnLocal(node, compressor)) {
21802+
ownLocals = false;
21803+
}
21804+
return undefined;
21805+
})
21806+
);
21807+
return ownLocals;
21808+
};
21809+
21810+
/**
21811+
* `a ? o.f(b) : o.f(c)` as `o.f(a ? b : c)`, which terser keeps where the
21812+
* callee may run code, that here neither sees nor changes what the test reads.
21813+
* @param {Node} self a conditional
21814+
* @param {Compressor} compressor the compressor
21815+
* @returns {Node | undefined} the call, none where reading it first may differ
21816+
*/
21817+
const mergedCall = (self, compressor) => {
21818+
const { consequent, alternate } = self;
21819+
if (
21820+
!A.isCallNode(consequent) ||
21821+
A.kindOf(alternate) !== A.kindOf(/** @type {Node} */ (consequent)) ||
21822+
/** @type {Node} */ (consequent).optional ||
21823+
alternate.optional ||
21824+
/** @type {Node} */ (consequent).arguments.length !==
21825+
alternate.arguments.length ||
21826+
!A.isEquivalent(/** @type {Node} */ (consequent).callee, alternate.callee)
21827+
) {
21828+
return;
21829+
}
21830+
const first = /** @type {Node} */ (consequent).arguments;
21831+
const second = alternate.arguments;
21832+
let index = -1;
21833+
for (let i = 0; i < first.length; i++) {
21834+
if (A.isExpansionNode(first[i]) || A.isExpansionNode(second[i])) return;
21835+
if (!A.isEquivalent(first[i], second[i])) {
21836+
if (index !== -1) return;
21837+
index = i;
21838+
} else if (index === -1 && !A.isConstantNode(first[i])) {
21839+
// It ran after the test, and now runs before it.
21840+
return;
21841+
}
21842+
}
21843+
if (
21844+
index === -1 ||
21845+
!readsCalleeAside(
21846+
/** @type {Node} */ (consequent).callee,
21847+
self.test,
21848+
compressor
21849+
)
21850+
) {
21851+
return;
21852+
}
21853+
const call = A.cloneNode(/** @type {Node} */ (consequent));
21854+
call.arguments[index] = makeNode(A.ConditionalNode, self, {
21855+
test: self.test,
21856+
consequent: first[index],
21857+
alternate: second[index]
21858+
});
21859+
return call;
21860+
};
21861+
2174021862
A.wrapOptimizer(
2174121863
"Conditional",
2174221864
(optimizeConditional) => (self, compressor) => {
@@ -21745,7 +21867,8 @@ const installImprove = (modules) => {
2174521867
(improvements.enabled &&
2174621868
A.isConditionalNode(optimized) &&
2174721869
compressor.option("conditionals") &&
21748-
mergedSpreadCall(optimized, compressor)) ||
21870+
(mergedSpreadCall(optimized, compressor) ||
21871+
mergedCall(optimized, compressor))) ||
2174921872
optimized
2175021873
);
2175121874
}
@@ -26502,12 +26625,14 @@ const installImprove = (modules) => {
2650226625

2650326626
/**
2650426627
* `null != a && a.b()` and `null == a || a.b()` as `a?.b()` where nothing
26505-
* reads the value, which the chain gives as `undefined` in place of a boolean.
26628+
* reads the value, which the chain gives as `undefined` in place of a boolean,
26629+
* and `null != a && a.b` where only its truth is read.
2650626630
* @param {Node} node a logical expression
26631+
* @param {Node} self the node visited, whose context the compressor knows
2650726632
* @param {Compressor} compressor the compressor
2650826633
* @returns {Node | undefined} the optional chain, or undefined
2650926634
*/
26510-
const unreadOptionalChain = (node, compressor) => {
26635+
const unreadOptionalChain = (node, self, compressor) => {
2651126636
const { operator, left, right } = node;
2651226637
const parent = compressor.parent();
2651326638
if (
@@ -26518,7 +26643,11 @@ const installImprove = (modules) => {
2651826643
) ||
2651926644
!(
2652026645
(A.isSimpleStatementNode(parent) && parent.expression === node) ||
26521-
(A.isSequenceNode(parent) && A.tailNode(parent) !== node)
26646+
(A.isSequenceNode(parent) && A.tailNode(parent) !== node) ||
26647+
// `false` and `undefined` are alike falsy, while `||` would read `true`.
26648+
(operator === "&&" &&
26649+
compressor.self() === self &&
26650+
compressor.in_boolean_context())
2652226651
)
2652326652
) {
2652426653
return undefined;
@@ -28638,7 +28767,7 @@ const installImprove = (modules) => {
2863828767
booleanEquality(optimized, compressor) ||
2863928768
commutedProduct(optimized, compressor) ||
2864028769
logicalAssignment(optimized, compressor) ||
28641-
unreadOptionalChain(optimized, compressor) ||
28770+
unreadOptionalChain(optimized, self, compressor) ||
2864228771
coerceBinary(optimized, self, compressor) ||
2864328772
shortenBinary(optimized, compressor)
2864428773
);
@@ -28774,6 +28903,9 @@ const installImprove = (modules) => {
2877428903
// A name the compressor wrote without a definition counts by its name.
2877528904
/** @type {Set<Definition | string>} */
2877628905
const written = new Set();
28906+
// `export { x }` names a binding away from its declaration.
28907+
/** @type {Set<Definition>} */
28908+
const exported = new Set();
2877728909
/**
2877828910
* @param {Node | null} target what an assignment, update or loop writes
2877928911
* @returns {void}
@@ -28840,6 +28972,16 @@ const installImprove = (modules) => {
2884028972
case "ForOfStatement":
2884128973
if (!A.isDefinitionsNode(node.left)) markWritten(node.left);
2884228974
break;
28975+
case "ExportNamedDeclaration":
28976+
case "ExportDefaultDeclaration":
28977+
if (node.source) break;
28978+
for (const name of node.exported_names || []) {
28979+
if (name.local.definition) exported.add(name.local.definition);
28980+
}
28981+
if (node.exported_value && node.exported_value.definition) {
28982+
exported.add(node.exported_value.definition);
28983+
}
28984+
break;
2884328985
default:
2884428986
if (join && (node[KIND_BITS] & KIND_BLOCK) !== 0) {
2884528987
blocks.push(node);
@@ -28860,6 +29002,7 @@ const installImprove = (modules) => {
2886029002
symbol !== undefined &&
2886129003
symbol !== null &&
2886229004
(toplevelPrivate || !A.isToplevelNode(symbol.scope.node)) &&
29005+
!exported.has(symbol) &&
2886329006
!symbol.scope.isPinned() &&
2886429007
!written.has(symbol) &&
2886529008
!written.has(node.name);

‎test/unitCases/syntax-printer.unittest.js‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,30 @@ const IMPROVED_CASES = [
245245
{ compress: {}, mangle: false },
246246
'function y(){console.log("y")}var z=2;console.log((y(),1),z,(y(),y(),3));'
247247
],
248+
[
249+
"`null != a && a.b` where only its truth is read, as `a?.b`, which is `undefined` where `a` is nullish",
250+
'function f(n) { if (null != n && n.length) return "x"; return !(null != n && n.a) ? "y" : "z"; } console.log(f(), f(null), f([1]), f({ a: 1 }), f({}));',
251+
{ compress: { ecma: 2020 }, mangle: false },
252+
'function f(n){return n?.length?"x":n?.a?"z":"y"}console.log(f(),f(null),f([1]),f({a:1}),f({}));'
253+
],
254+
[
255+
"two calls through one member path differing in one argument, where the test reads only names nothing else can see",
256+
"function f(i, n) { return 0 !== i ? Math.abs(i) : Math.abs(n); } function g(t, x) { var set = function () { t = 0; }; return t ? x.m(1, set) : x.m(2, set); } var o = { m: function (v) { return v; } }; console.log(f(-1, -2), f(0, -3), g(1, o), g(0, o));",
257+
{ compress: {}, mangle: false },
258+
"function f(i,n){return Math.abs(0!==i?i:n)}function g(t,x){var set=function(){t=0};return t?x.m(1,set):x.m(2,set)}var o={m:function(v){return v}};console.log(f(-1,-2),f(0,-3),g(1,o),g(0,o));"
259+
],
260+
[
261+
"two calls spreading a string or another iterable, merged in a statement that reads no truth",
262+
'function f(c, b) { c ? g(..."ab") : g(...b); } function g(v, w) { console.log(v, w); } f(1, ["y"]); f(0, ["y"]);',
263+
{ compress: {}, mangle: false },
264+
'function f(c,b){g(...c?"ab":b)}function g(v,w){console.log(v,w)}f(1,["y"]),f(0,["y"]);'
265+
],
266+
[
267+
"two calls through one member path, merged in a statement that reads no truth",
268+
'function g(n) { if (!(n < 0)) console.log(typeof []); else console.log("y"); } g(-1); g(1);',
269+
{ compress: {}, mangle: false },
270+
'function g(n){console.log(n<0?"y":"object")}g(-1),g(1);'
271+
],
248272
[
249273
"a new object compared with a primitive, another new object or `null`, which it never equals (terser#1335)",
250274
'console.log({ a: 7 } === null, [] !== 1, (() => 1) == null, {} != null, {} == "[object Object]");',
@@ -303,7 +327,7 @@ const IMPROVED_CASES = [
303327
"a `switch` every branch of which runs one expression and leaves",
304328
'var log = []; function f(a) { switch (a) { case 1: log.push("one"); break; case 2: log.push("two"); break; default: log.push("x"); } } f(1); f(2); f(3); console.log(log.join());',
305329
{ compress: {}, mangle: false },
306-
'var log=[];function f(a){1===a?log.push("one"):2===a?log.push("two"):log.push("x")}f(1),f(2),f(3),console.log(log.join());'
330+
'var log=[];function f(a){log.push(1===a?"one":2===a?"two":"x")}f(1),f(2),f(3),console.log(log.join());'
307331
],
308332
[
309333
"a `switch` running one expression per branch, with no default",
@@ -8645,6 +8669,17 @@ describe("syntax-printer", () => {
86458669
});
86468670
}
86478671

8672+
it("should keep a module's exported `const` while writing its private ones as `let`", async () => {
8673+
const { minify } = await load();
8674+
const { code } = await minify(
8675+
"const x = { a: 1 }; console.log(x); const z = { b: 2 }; console.log(z, z); export { x };",
8676+
{ module: true, mangle: false }
8677+
);
8678+
expect(code).toBe(
8679+
"const x={a:1};console.log(x);let z={b:2};console.log(z,z);export{x};"
8680+
);
8681+
});
8682+
86488683
it("should name a module's imports and exports by the names they spell, where that writes less", async () => {
86498684
const { minify, improvements } = await load();
86508685
/** @type {[string, string][]} */

0 commit comments

Comments
 (0)