From c08fe8dc13ae512cf669eb25356edcd22cc36351 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donny/=EA=B0=95=EB=8F=99=EC=9C=A4?= Date: Fri, 28 Feb 2025 10:55:33 +0900 Subject: [PATCH] fix(es/minifier): Skip inlining if the referential identity of a function matters (#10123) **Description:** ```js function a() { if (a.isInit) return; a.isInit = true; console.log('run') } function b() { a(); console.log('after'); } b(); b(); ``` was minified as ```js function b() { (function a() { if (a.isInit) return; a.isInit = true, console.log('run'); })(), console.log('after'); } b(), b(); ``` Which is wrong because the referential identity of `a` is important. **Related issue:** - Closes https://github.com/swc-project/swc/issues/10095 --- .changeset/cuddly-spiders-begin.md | 6 + crates/swc_ecma_minifier/src/program_data.rs | 7 +- .../tests/benches-full/jquery.js | 5 +- .../tests/benches-full/lodash.js | 207 ++++++++++-------- crates/swc_ecma_minifier/tests/exec.rs | 75 +++++++ .../fixture/next/react-pdf-renderer/output.js | 15 +- 6 files changed, 208 insertions(+), 107 deletions(-) create mode 100644 .changeset/cuddly-spiders-begin.md diff --git a/.changeset/cuddly-spiders-begin.md b/.changeset/cuddly-spiders-begin.md new file mode 100644 index 000000000000..36b2a6461100 --- /dev/null +++ b/.changeset/cuddly-spiders-begin.md @@ -0,0 +1,6 @@ +--- +swc_ecma_minifier: patch +swc_core: patch +--- + +fix(es/minifier): Skip inlining if a function has recursive access diff --git a/crates/swc_ecma_minifier/src/program_data.rs b/crates/swc_ecma_minifier/src/program_data.rs index 9d3472b858a0..8e0bd0cb0234 100644 --- a/crates/swc_ecma_minifier/src/program_data.rs +++ b/crates/swc_ecma_minifier/src/program_data.rs @@ -177,8 +177,11 @@ impl VarUsageInfo { } pub(crate) fn can_inline_fn_once(&self) -> bool { - self.callee_count > 0 - || !self.executed_multiple_time && (self.is_fn_local || !self.used_in_non_child_fn) + (self.callee_count > 0 + || !self.executed_multiple_time && (self.is_fn_local || !self.used_in_non_child_fn)) + && !(self.used_recursively + && self.has_property_access + && self.property_mutation_count != 0) } fn initialized(&self) -> bool { diff --git a/crates/swc_ecma_minifier/tests/benches-full/jquery.js b/crates/swc_ecma_minifier/tests/benches-full/jquery.js index e5127edd50ac..11e2227e1843 100644 --- a/crates/swc_ecma_minifier/tests/benches-full/jquery.js +++ b/crates/swc_ecma_minifier/tests/benches-full/jquery.js @@ -382,10 +382,11 @@ * deleting the oldest entry */ function createCache() { var keys = []; - return function cache(key, value) { + function cache(key, value) { return keys.push(key + " ") > Expr.cacheLength && // Only keep the most recent entries delete cache[keys.shift()], cache[key + " "] = value; - }; + } + return cache; } /** * Mark a function for special use by Sizzle diff --git a/crates/swc_ecma_minifier/tests/benches-full/lodash.js b/crates/swc_ecma_minifier/tests/benches-full/lodash.js index 3bea154e12dc..d3220524a849 100644 --- a/crates/swc_ecma_minifier/tests/benches-full/lodash.js +++ b/crates/swc_ecma_minifier/tests/benches-full/lodash.js @@ -2972,7 +2972,7 @@ * @returns {Function} Returns the new wrapped function. */ function createHybrid(func, bitmask, thisArg, partials, holders, partialsRight, holdersRight, argPos, ary, arity) { var isAry = 128 & bitmask, isBind = 1 & bitmask, isBindKey = 2 & bitmask, isCurried = 24 & bitmask, isFlip = 512 & bitmask, Ctor = isBindKey ? undefined : createCtor(func); - return function wrapper() { + function wrapper() { for(var length = arguments.length, args = Array1(length), index = length; index--;)args[index] = arguments[index]; if (isCurried) var placeholder = getHolder(wrapper), holdersCount = /** * Gets the number of `placeholder` occurrences in `array`. @@ -3006,7 +3006,8 @@ } return array; }(args, argPos) : isFlip && length > 1 && args.reverse(), isAry && ary < length && (args.length = ary), this && this !== root && this instanceof wrapper && (fn = Ctor || createCtor(fn)), fn.apply(thisBinding, args); - }; + } + return wrapper; } /** * Creates a function like `_.invertBy`. @@ -3268,17 +3269,29 @@ (value = source[7]) && (data[7] = value), 128 & srcBitmask && (data[8] = null == data[8] ? source[8] : nativeMin(data[8], source[8])), null == data[9] && (data[9] = source[9]), // Use source `func` and merge bitmasks. data[0] = source[0], data[1] = newBitmask; } - }(newData, data), func = newData[0], bitmask = newData[1], thisArg = newData[2], partials = newData[3], holders = newData[4], (arity = newData[9] = newData[9] === undefined ? isBindKey ? 0 : func.length : nativeMax(newData[9] - length, 0)) || !(24 & bitmask) || (bitmask &= -25), bitmask && 1 != bitmask) 8 == bitmask || 16 == bitmask ? (func1 = func, bitmask1 = bitmask, arity1 = arity, Ctor = createCtor(func1), result = function wrapper() { - for(var length = arguments.length, args = Array1(length), index = length, placeholder = getHolder(wrapper); index--;)args[index] = arguments[index]; - var holders = length < 3 && args[0] !== placeholder && args[length - 1] !== placeholder ? [] : replaceHolders(args, placeholder); - return (length -= holders.length) < arity1 ? createRecurry(func1, bitmask1, createHybrid, wrapper.placeholder, undefined, args, holders, undefined, undefined, arity1 - length) : apply(this && this !== root && this instanceof wrapper ? Ctor : func1, this, args); - }) : 32 != bitmask && 33 != bitmask || holders.length ? result = createHybrid.apply(undefined, newData) : (func2 = func, bitmask2 = bitmask, thisArg1 = thisArg, partials1 = partials, isBind = 1 & bitmask2, Ctor1 = createCtor(func2), result = function wrapper() { - for(var argsIndex = -1, argsLength = arguments.length, leftIndex = -1, leftLength = partials1.length, args = Array1(leftLength + argsLength), fn = this && this !== root && this instanceof wrapper ? Ctor1 : func2; ++leftIndex < leftLength;)args[leftIndex] = partials1[leftIndex]; + }(newData, data), func = newData[0], bitmask = newData[1], thisArg = newData[2], partials = newData[3], holders = newData[4], (arity = newData[9] = newData[9] === undefined ? isBindKey ? 0 : func.length : nativeMax(newData[9] - length, 0)) || !(24 & bitmask) || (bitmask &= -25), bitmask && 1 != bitmask) 8 == bitmask || 16 == bitmask ? result = /** + * Creates a function that wraps `func` to enable currying. + * + * @private + * @param {Function} func The function to wrap. + * @param {number} bitmask The bitmask flags. See `createWrap` for more details. + * @param {number} arity The arity of `func`. + * @returns {Function} Returns the new wrapped function. + */ function(func, bitmask, arity) { + var Ctor = createCtor(func); + function wrapper() { + for(var length = arguments.length, args = Array1(length), index = length, placeholder = getHolder(wrapper); index--;)args[index] = arguments[index]; + var holders = length < 3 && args[0] !== placeholder && args[length - 1] !== placeholder ? [] : replaceHolders(args, placeholder); + return (length -= holders.length) < arity ? createRecurry(func, bitmask, createHybrid, wrapper.placeholder, undefined, args, holders, undefined, undefined, arity - length) : apply(this && this !== root && this instanceof wrapper ? Ctor : func, this, args); + } + return wrapper; + }(func, bitmask, arity) : 32 != bitmask && 33 != bitmask || holders.length ? result = createHybrid.apply(undefined, newData) : (func1 = func, bitmask1 = bitmask, thisArg1 = thisArg, partials1 = partials, isBind = 1 & bitmask1, Ctor = createCtor(func1), result = function wrapper() { + for(var argsIndex = -1, argsLength = arguments.length, leftIndex = -1, leftLength = partials1.length, args = Array1(leftLength + argsLength), fn = this && this !== root && this instanceof wrapper ? Ctor : func1; ++leftIndex < leftLength;)args[leftIndex] = partials1[leftIndex]; for(; argsLength--;)args[leftIndex++] = arguments[++argsIndex]; return apply(fn, isBind ? thisArg1 : this, args); }); - else var func1, bitmask1, arity1, Ctor, func2, bitmask2, thisArg1, partials1, isBind, Ctor1, func3, bitmask3, thisArg2, isBind1, Ctor2, result = (func3 = func, bitmask3 = bitmask, thisArg2 = thisArg, isBind1 = 1 & bitmask3, Ctor2 = createCtor(func3), function wrapper() { - return (this && this !== root && this instanceof wrapper ? Ctor2 : func3).apply(isBind1 ? thisArg2 : this, arguments); + else var func1, bitmask1, thisArg1, partials1, isBind, Ctor, func2, bitmask2, thisArg2, isBind1, Ctor1, result = (func2 = func, bitmask2 = bitmask, thisArg2 = thisArg, isBind1 = 1 & bitmask2, Ctor1 = createCtor(func2), function wrapper() { + return (this && this !== root && this instanceof wrapper ? Ctor1 : func2).apply(isBind1 ? thisArg2 : this, arguments); }); return setWrapToString((data ? baseSetData : setData)(result, newData), func, bitmask); } @@ -4563,6 +4576,93 @@ return createWrap(key, bitmask, object, partials, holders); }); /** + * Creates a function that accepts arguments of `func` and either invokes + * `func` returning its result, if at least `arity` number of arguments have + * been provided, or returns a function that accepts the remaining `func` + * arguments, and so on. The arity of `func` may be specified if `func.length` + * is not sufficient. + * + * The `_.curry.placeholder` value, which defaults to `_` in monolithic builds, + * may be used as a placeholder for provided arguments. + * + * **Note:** This method doesn't set the "length" property of curried functions. + * + * @static + * @memberOf _ + * @since 2.0.0 + * @category Function + * @param {Function} func The function to curry. + * @param {number} [arity=func.length] The arity of `func`. + * @param- {Object} [guard] Enables use as an iteratee for methods like `_.map`. + * @returns {Function} Returns the new curried function. + * @example + * + * var abc = function(a, b, c) { + * return [a, b, c]; + * }; + * + * var curried = _.curry(abc); + * + * curried(1)(2)(3); + * // => [1, 2, 3] + * + * curried(1, 2)(3); + * // => [1, 2, 3] + * + * curried(1, 2, 3); + * // => [1, 2, 3] + * + * // Curried with placeholders. + * curried(1)(_, 3)(2); + * // => [1, 2, 3] + */ function curry(func, arity, guard) { + arity = guard ? undefined : arity; + var result = createWrap(func, 8, undefined, undefined, undefined, undefined, undefined, arity); + return result.placeholder = curry.placeholder, result; + } + /** + * This method is like `_.curry` except that arguments are applied to `func` + * in the manner of `_.partialRight` instead of `_.partial`. + * + * The `_.curryRight.placeholder` value, which defaults to `_` in monolithic + * builds, may be used as a placeholder for provided arguments. + * + * **Note:** This method doesn't set the "length" property of curried functions. + * + * @static + * @memberOf _ + * @since 3.0.0 + * @category Function + * @param {Function} func The function to curry. + * @param {number} [arity=func.length] The arity of `func`. + * @param- {Object} [guard] Enables use as an iteratee for methods like `_.map`. + * @returns {Function} Returns the new curried function. + * @example + * + * var abc = function(a, b, c) { + * return [a, b, c]; + * }; + * + * var curried = _.curryRight(abc); + * + * curried(3)(2)(1); + * // => [1, 2, 3] + * + * curried(2, 3)(1); + * // => [1, 2, 3] + * + * curried(1, 2, 3); + * // => [1, 2, 3] + * + * // Curried with placeholders. + * curried(3)(1, _)(2); + * // => [1, 2, 3] + */ function curryRight(func, arity, guard) { + arity = guard ? undefined : arity; + var result = createWrap(func, 16, undefined, undefined, undefined, undefined, undefined, arity); + return result.placeholder = curryRight.placeholder, result; + } + /** * Creates a debounced function that delays invoking `func` until after `wait` * milliseconds have elapsed since the last time the debounced function was * invoked. The debounced function comes with a `cancel` method to cancel @@ -6596,92 +6696,7 @@ */ function(prototype, properties) { var result = baseCreate(prototype); return null == properties ? result : baseAssign(result, properties); - }, lodash.curry = /** - * Creates a function that accepts arguments of `func` and either invokes - * `func` returning its result, if at least `arity` number of arguments have - * been provided, or returns a function that accepts the remaining `func` - * arguments, and so on. The arity of `func` may be specified if `func.length` - * is not sufficient. - * - * The `_.curry.placeholder` value, which defaults to `_` in monolithic builds, - * may be used as a placeholder for provided arguments. - * - * **Note:** This method doesn't set the "length" property of curried functions. - * - * @static - * @memberOf _ - * @since 2.0.0 - * @category Function - * @param {Function} func The function to curry. - * @param {number} [arity=func.length] The arity of `func`. - * @param- {Object} [guard] Enables use as an iteratee for methods like `_.map`. - * @returns {Function} Returns the new curried function. - * @example - * - * var abc = function(a, b, c) { - * return [a, b, c]; - * }; - * - * var curried = _.curry(abc); - * - * curried(1)(2)(3); - * // => [1, 2, 3] - * - * curried(1, 2)(3); - * // => [1, 2, 3] - * - * curried(1, 2, 3); - * // => [1, 2, 3] - * - * // Curried with placeholders. - * curried(1)(_, 3)(2); - * // => [1, 2, 3] - */ function curry(func, arity, guard) { - arity = guard ? undefined : arity; - var result = createWrap(func, 8, undefined, undefined, undefined, undefined, undefined, arity); - return result.placeholder = curry.placeholder, result; - }, lodash.curryRight = /** - * This method is like `_.curry` except that arguments are applied to `func` - * in the manner of `_.partialRight` instead of `_.partial`. - * - * The `_.curryRight.placeholder` value, which defaults to `_` in monolithic - * builds, may be used as a placeholder for provided arguments. - * - * **Note:** This method doesn't set the "length" property of curried functions. - * - * @static - * @memberOf _ - * @since 3.0.0 - * @category Function - * @param {Function} func The function to curry. - * @param {number} [arity=func.length] The arity of `func`. - * @param- {Object} [guard] Enables use as an iteratee for methods like `_.map`. - * @returns {Function} Returns the new curried function. - * @example - * - * var abc = function(a, b, c) { - * return [a, b, c]; - * }; - * - * var curried = _.curryRight(abc); - * - * curried(3)(2)(1); - * // => [1, 2, 3] - * - * curried(2, 3)(1); - * // => [1, 2, 3] - * - * curried(1, 2, 3); - * // => [1, 2, 3] - * - * // Curried with placeholders. - * curried(3)(1, _)(2); - * // => [1, 2, 3] - */ function curryRight(func, arity, guard) { - arity = guard ? undefined : arity; - var result = createWrap(func, 16, undefined, undefined, undefined, undefined, undefined, arity); - return result.placeholder = curryRight.placeholder, result; - }, lodash.debounce = debounce, lodash.defaults = defaults, lodash.defaultsDeep = defaultsDeep, lodash.defer = defer, lodash.delay = delay, lodash.difference = difference, lodash.differenceBy = differenceBy, lodash.differenceWith = differenceWith, lodash.drop = /** + }, lodash.curry = curry, lodash.curryRight = curryRight, lodash.debounce = debounce, lodash.defaults = defaults, lodash.defaultsDeep = defaultsDeep, lodash.defer = defer, lodash.delay = delay, lodash.difference = difference, lodash.differenceBy = differenceBy, lodash.differenceWith = differenceWith, lodash.drop = /** * Creates a slice of `array` with `n` elements dropped from the beginning. * * @static diff --git a/crates/swc_ecma_minifier/tests/exec.rs b/crates/swc_ecma_minifier/tests/exec.rs index f993d64ec9d1..bee1b6ee08ba 100644 --- a/crates/swc_ecma_minifier/tests/exec.rs +++ b/crates/swc_ecma_minifier/tests/exec.rs @@ -11375,3 +11375,78 @@ fn isssue_9498() { ", ) } + +#[test] +fn issue_10095() { + run_exec_test( + " + function module() { + function a() { + if (a.isInit) return; + a.isInit = true; + + console.log('run') + } + + function b() { + a(); + + console.log('after'); + } + + b(); + b(); + } + + module(); + ", + r#"{ + "defaults": true, + "arguments": false, + "arrows": false, + "booleans": false, + "booleans_as_integers": false, + "collapse_vars": false, + "comparisons": false, + "computed_props": false, + "conditionals": false, + "dead_code": false, + "directives": false, + "drop_console": false, + "drop_debugger": false, + "evaluate": false, + "expression": false, + "hoist_funs": false, + "hoist_props": false, + "hoist_vars": false, + "if_return": false, + "join_vars": false, + "keep_classnames": false, + "keep_fargs": false, + "keep_fnames": false, + "keep_infinity": false, + "loops": false, + "negate_iife": false, + "properties": false, + "reduce_funcs": false, + "reduce_vars": false, + "side_effects": false, + "switches": false, + "typeofs": false, + "unsafe": false, + "unsafe_arrows": false, + "unsafe_comps": false, + "unsafe_Function": false, + "unsafe_math": false, + "unsafe_symbols": false, + "unsafe_methods": false, + "unsafe_proto": false, + "unsafe_regexp": false, + "unsafe_undefined": false, + "unused": false, + "const_to_let": false, + "pristine_globals": false + }"#, + false, + ); +} diff --git a/crates/swc_ecma_minifier/tests/fixture/next/react-pdf-renderer/output.js b/crates/swc_ecma_minifier/tests/fixture/next/react-pdf-renderer/output.js index 414cc490b4e5..713f398f41b3 100644 --- a/crates/swc_ecma_minifier/tests/fixture/next/react-pdf-renderer/output.js +++ b/crates/swc_ecma_minifier/tests/fixture/next/react-pdf-renderer/output.js @@ -16123,6 +16123,13 @@ throw t; } } + function I() { + for(var e = arguments.length, t = Array(e), r = 0; r < e; r++)t[r] = arguments[r]; + S.apply(void 0, [ + I, + t.length + ].concat(t)); + } E.throws = function e(t) { for(var r = arguments.length, n = Array(r > 1 ? r - 1 : 0), i = 1; i < r; i++)n[i - 1] = arguments[i]; F.apply(void 0, [ @@ -16176,13 +16183,7 @@ } throw n; } - }, E.strict = b(function e() { - for(var t = arguments.length, r = Array(t), n = 0; n < t; n++)r[n] = arguments[n]; - S.apply(void 0, [ - e, - r.length - ].concat(r)); - }, E, { + }, E.strict = b(I, E, { equal: E.strictEqual, deepEqual: E.deepStrictEqual, notEqual: E.notStrictEqual,