newTreeshaking fails at Ramda's all() utility

kaisellgren commented 10 months ago

System Info

Using "ramda": "0.27.1" and "@rspack/cli": "0.4.3"

Details

Ramda library has this all utility:

import _curry2 from "./internal/_curry2.js";
import _dispatchable from "./internal/_dispatchable.js";
import _xall from "./internal/_xall.js";
/**
 * Returns `true` if all elements of the list match the predicate, `false` if
 * there are any that don't. HEH
 *
 * Dispatches to the `all` method of the second argument, if present.
 *
 * Acts as a transducer if a transformer is given in list position.
 *
 * @func
 * @memberOf R
 * @since v0.1.0
 * @category List
 * @sig (a -> Boolean) -> [a] -> Boolean
 * @param {Function} fn The predicate function.
 * @param {Array} list The array to consider.
 * @return {Boolean} `true` if the predicate is satisfied by every element, `false`
 *         otherwise.
 * @see R.any, R.none, R.transduce
 * @example
 *
 *      const equals3 = R.equals(3);
 *      R.all(equals3)([3, 3, 3, 3]); //=> true
 *      R.all(equals3)([3, 3, 1, 3]); //=> false
 */

var all =
/*#__PURE__*/
_curry2(
/*#__PURE__*/
_dispatchable(['all'], _xall, function all(fn, list) {
  var idx = 0;

  while (idx < list.length) {
    if (!fn(list[idx])) {
      return false;
    }

    idx += 1;
  }

  return true;
}));

export default all;

This is compiled down to this using Rspack with newTreeshaking and SWC:

"66364": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
"use strict";
__webpack_require__.r(__webpack_exports__);
/* harmony import */var _internal_curry2_js__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ./internal/_curry2.js */"55495");
/* harmony import */var _internal_dispatchable_js__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(/*! ./internal/_dispatchable.js */"33478");
/* harmony import */var _internal_xall_js__WEBPACK_IMPORTED_MODULE_2__ = __webpack_require__(/*! ./internal/_xall.js */"35805");

/**
 * Returns `true` if all elements of the list match the predicate, `false` if
 * there are any that don't. HEH
 *
 * Dispatches to the `all` method of the second argument, if present.
 *
 * Acts as a transducer if a transformer is given in list position.
 *
 * @func
 * @memberOf R
 * @since v0.1.0
 * @category List
 * @sig (a -> Boolean) -> [a] -> Boolean
 * @param {Function} fn The predicate function.
 * @param {Array} list The array to consider.
 * @return {Boolean} `true` if the predicate is satisfied by every element, `false`
 *         otherwise.
 * @see R.any, R.none, R.transduce
 * @example
 *
 *      const equals3 = R.equals(3);
 *      R.all(equals3)([3, 3, 3, 3]); //=> true
 *      R.all(equals3)([3, 3, 1, 3]); //=> false
 */ var all = /*#__PURE__*/ (/* unused pure expression or super */ null && ((0, _internal_curry2_js__WEBPACK_IMPORTED_MODULE_0__.Z)(/*#__PURE__*/ (0, _internal_dispatchable_js__WEBPACK_IMPORTED_MODULE_1__.Z)([
    "all"
], , function all(fn, list) {
    var idx = 0;
    while(idx < list.length){
        if (!fn(list[idx])) return false;
        idx += 1;
    }
    return true;
}))));
var __WEBPACK_DEFAULT_EXPORT__ = (/* unused pure expression or super */ null && (all));
})

Reproduce link

No response

Reproduce Steps

When using the newTreeshaking: true, it fails with this error:

> rspack --mode production --env target=client

  × Error[javascript]: JavaScript parsing error
       ╭─[vendors-f1b574f2c20aa4ec8e59.js:43275:1]
 43275 │     "all"
 43276 │ ], , function all(fn, list) {
       ·    ┬
       ·    ╰── Expression expected
 43277 │     var idx = 0;
       ╰────

The compilation has stripped the _xall from original source code and left an empty comma expression and now it fails there.

IWANABETHATGUY commented 10 months ago

Can you provide a reproducible demo or repo?

kaisellgren commented 10 months ago

It's quite difficult. The more I strip down the application the less likely the error occurs. The issue happens when I turn on:

{
    optimization: {
      minimize: true,

      splitChunks: {
        cacheGroups: {
          vendor: {
            test: /[\\/]node_modules[\\/]/,
            name: 'vendors',
            chunks: 'all',
          },
        },
      },
    },
    experiments: {
      rspackFuture: {
        newTreeshaking: true,
      },
    },
}

It doesn't happen every time. It's not deterministic. I have to build over and over again for it to happen. The more I strip our codebase the probability of the error goes down and when I try to create a simple repro project it just no longer happens (probability close to 0%).

I need to enable minification, vendor chunking and the new treeshaker.

When the error occurs, it complains about vendors-66bf7fba49a56d2c1df8.js:

That function is Ramda's all function (node_modules/ramda/es/all.js):

kaisellgren commented 10 months ago

Interestingly, I manually patched the Ramda's all function from:

var all =
/*#__PURE__*/
_curry2(
/*#__PURE__*/
_dispatchable(['all'], _xall, function all(fn, list) {
  var idx = 0;

  while (idx < list.length) {
    if (!fn(list[idx])) {
      return false;
    }

    idx += 1;
  }

  return true;
}));

to:

var all =
/*#__PURE__*/
_curry2(
/*#__PURE__*/
_dispatchable(['all'], function kaisTest(f) {
  return function (xf) {
    return new XAll(f, xf);
  };
}, function all(fn, list) {
  var idx = 0;

  while (idx < list.length) {
    if (!fn(list[idx])) {
      return false;
    }

    idx += 1;
  }

  return true;
}));

And now the processing went further and failed at the next Ramda function called allPass:

The original Ramda source code:

I think the treeshaker simply strips code that it shouldn't. The weird thing is that it sometimes works and sometimes doesn't, meaning that there's non-determinism involved. The more I strip our codebase the less likely the issue occurs.

github-actions[bot] commented 10 months ago

Hello @kaisellgren, sorry we can't investigate the problem further without reproduction demo, please provide a repro demo by forking rspack-repro, or provide a minimal GitHub repository by yourself. Issues labeled by need reproduction will be closed if no activities in 14 days.

github-actions[bot] commented 9 months ago

Since the issue was labeled with need reproduction, but no response in 14 days. This issue will be closed. Feel free to comment and reopen it if you have any further questions.

github-actions[bot] commented 9 months ago

Since the issue was labeled with need reproduction, but no response in 14 days. This issue will be closed. Feel free to comment and reopen it if you have any further questions.

bostondv commented 9 months ago

I'm running into a similar error as describe here with various libraries in our app and haven't posted an issue yet because I've been unable to narrow down a good reproduction case.

Example error from reakit@1.3.6 (source).

#39 160.0 [rspack-client] ERROR in × JavaScript parsing error: Expression expected
#39 160.0 [rspack-client]        ╭─[17268:1]
#39 160.0 [rspack-client]  17268 │     name: "Combobox",
#39 160.0 [rspack-client]  17269 │     compose: _Composite_Composite_js__WEBPACK_IMPORTED_MODULE_4__/* .useComposite */.Q,
#39 160.0 [rspack-client]  17270 │     keys: ,
#39 160.0 [rspack-client]        ·           ─
#39 160.0 [rspack-client]  17271 │     useOptions: function useOptions(_ref) {
#39 160.0 [rspack-client]  17272 │         var _ref$menuRole = _ref.menuRole, menuRole = _ref$menuRole === void 0 ? "listbox" : _ref$menuRole, _ref$hideOnEsc = _ref.hideOnEsc, hideOnEsc = _ref$hideOnEsc === void 0 ? true : _ref$hideOnEsc, options = (0, _rollupPluginBabelHelpers_1f0bf8c2_js__WEBPACK_IMPORTED_MODULE_6__._)(_ref, [
#39 160.0 [rspack-client]        ╰────

The missing variable seems very familiar to what kaisellgren has seen.

Also get similar errors out of date-fns@2.30.0 the in same app.

For me, I can't only so far reproduce in our Docker build.

System:
    OS: Linux 5.15 Ubuntu 22.04.3 LTS 22.04.3 LTS (Jammy Jellyfish)
   CPU: (8) x64 Intel(R) Xeon(R) Platinum 8175M CPU @ 2.50GHz
    Memory: 53.53 GB / 62.01 GB
    Container: Yes
    Shell: 5.1.16 - /bin/bash
Binaries:
    Node: 18.17.1 - /usr/bin/node
    Yarn: 1.22.4 - /var~/node_modules/.bin/yarn
    npm: 9.6.7 - /usr/bin/npm
npmPackages:
    @rspack/cli: 0.5.2 => 0.5.2 
    @rspack/core: 0.5.2 => 0.5.2 
    @rspack/plugin-react-refresh: 0.5.2 => 0.5.2

Running the build manually from remote dev box, I can't reproduce.

System:
    OS: Linux 5.15 Ubuntu 20.04.6 LTS (Focal Fossa)
    CPU: (8) x64 Intel(R) Xeon(R) Platinum 8175M CPU @ 2.50GHz
    Memory: 53.78 GB / 62.01 GB
    Container: Yes
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 18.19.0 - ~/.nvm/versions/node/v18.19.0/bin/node
    Yarn: 1.22.4 - ~/carrot/customers/store/node_modules/.bin/yarn
    npm: 10.2.3 - ~/.nvm/versions/node/v18.19.0/bin/npm
    Watchman: 20240121.093135.0 - /usr/local/bin/watchman
  npmPackages:
    @rspack/cli: 0.5.2 => 0.5.2 
    @rspack/core: 0.5.2 => 0.5.2 
    @rspack/plugin-react-refresh: 0.5.2 => 0.5.2

Have not been able to reproduce in a simple rspack-repro example, yet...

bostondv commented 9 months ago

I have also reproduced now on my Mac M2 running rspack build --config path/to/config.js:

System:
    OS: macOS 13.6.2
    CPU: (12) arm64 Apple M2 Pro
    Memory: 835.77 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.19.0 - ~/.nvm/versions/node/v18.19.0/bin/node
    Yarn: 1.22.4 - ~/carrot/customers/store/node_modules/.bin/yarn
    npm: 10.2.3 - ~/.nvm/versions/node/v18.19.0/bin/npm
  Browsers:
    Chrome: 120.0.6099.234
    Firefox: 122.0
    Safari: 17.3
  npmPackages:
    @rspack/cli: 0.5.3 => 0.5.3 
    @rspack/core: 0.5.3 => 0.5.3 
    @rspack/plugin-react-refresh: 0.5.3 => 0.5.3

IWANABETHATGUY commented 9 months ago

I have also reproduced now on my Mac M2 running rspack build --config path/to/config.js:

System:
    OS: macOS 13.6.2
    CPU: (12) arm64 Apple M2 Pro
    Memory: 835.77 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.19.0 - ~/.nvm/versions/node/v18.19.0/bin/node
    Yarn: 1.22.4 - ~/carrot/customers/store/node_modules/.bin/yarn
    npm: 10.2.3 - ~/.nvm/versions/node/v18.19.0/bin/npm
  Browsers:
    Chrome: 120.0.6099.234
    Firefox: 122.0
    Safari: 17.3
  npmPackages:
    @rspack/cli: 0.5.3 => 0.5.3 
    @rspack/core: 0.5.3 => 0.5.3 
    @rspack/plugin-react-refresh: 0.5.3 => 0.5.3

Can you provide your reproducing repo link?

bostondv commented 9 months ago

I’ve only been able to reproduce in our app so far which I can’t share the source.

In a simpler example with nearly the same config it’s not reproducible. I’ll keep trying though.

FWIW for me the failure is deterministic but it seems that I can get different results depending on where it’s run. On my Mac one version of the codebase always fails whereas the build succeeds in Docker. Earlier, with 0.5.2, i had the opposite case.

In a somewhat stripped down version of our app I’ve narrowed down a particular module in our source which if removed, rspack build succeeds on my Mac. But I can also even just rename the module and build will succeed. It’s very unusual.

Other than ‘cache’ config option is there any caching in rspack that could be impacting this behaviour? Because it really doesn’t make sense why renaming a module would have any effect on this tree shaking failure.

bostondv commented 9 months ago

I'm still unable to reproduce with a simple case but if its helpful it only occurs with newTreeshaking and optimization.splitChunks enabled in production mode.

{
    optimization: {
      splitChunks: {
        chunks: 'all',
      },
    },
    experiments: {
      rspackFuture: {
        newTreeshaking: true,
      },
    },
}

It does not occur with other splitChunks.chunks options such as async.

bostondv commented 9 months ago

Still don't have a simple repro but have found that setting optimization.usedExports = false will fix the issue. Obviously not ideal though since that helps with dead code elimination.

bostondv commented 9 months ago

Just to share another example. Here's a snippet of the chunk output from out build.

This locale isn't used at all in the app, its object is retained but all property values have been removed and left with invalid syntax.

"8449": (function (__unused_webpack_module, __unused_webpack___webpack_exports__, __webpack_require__) {
"use strict";
/* harmony import */var _lib_formatDistance_index_js__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ./_lib/formatDistance/index.js */"95885");
/* harmony import */var _lib_formatLong_index_js__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(/*! ./_lib/formatLong/index.js */"88");
/* harmony import */var _lib_formatRelative_index_js__WEBPACK_IMPORTED_MODULE_2__ = __webpack_require__(/*! ./_lib/formatRelative/index.js */"50855");
/* harmony import */var _lib_localize_index_js__WEBPACK_IMPORTED_MODULE_3__ = __webpack_require__(/*! ./_lib/localize/index.js */"85270");
/* harmony import */var _lib_match_index_js__WEBPACK_IMPORTED_MODULE_4__ = __webpack_require__(/*! ./_lib/match/index.js */"58");

/**
 * @type {Locale}
 * @category Locales
 * @summary Afrikaans locale.
 * @language Afrikaans
 * @iso-639-2 afr
 * @author Marnus Weststrate [@marnusw]{@link https://github.com/marnusw}
 */ var locale = (/* unused pure expression or super */ null && ({
    code: 'af',
    formatDistance: ,
    formatLong: ,
    formatRelative: ,
    localize: ,
    match: ,
    options: {
        weekStartsOn: 0 /* Sunday */ ,
        firstWeekContainsDate: 1
    }
}));
/* unused harmony default export */ var __WEBPACK_DEFAULT_EXPORT__ = ((/* unused pure expression or super */ null && (locale)));
}),

This locale is used in the app, so some of its object properties are retained but other property values which aren't used have been removed and are left with invalid syntax.

"69576": (function (__unused_webpack_module, __unused_webpack___webpack_exports__, __webpack_require__) {
"use strict";
/* harmony import */var _en_US_lib_formatRelative_index_js__WEBPACK_IMPORTED_MODULE_2__ = __webpack_require__(/*! ../en-US/_lib/formatRelative/index.js */"21865");
/* harmony import */var _en_US_lib_localize_index_js__WEBPACK_IMPORTED_MODULE_3__ = __webpack_require__(/*! ../en-US/_lib/localize/index.js */"11950");
/* harmony import */var _en_US_lib_match_index_js__WEBPACK_IMPORTED_MODULE_4__ = __webpack_require__(/*! ../en-US/_lib/match/index.js */"7219");
/* harmony import */var _lib_formatDistance_index_js__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ./_lib/formatDistance/index.js */"12329");
/* harmony import */var _lib_formatLong_index_js__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(/*! ./_lib/formatLong/index.js */"34422");

/**
 * @type {Locale}
 * @category Locales
 * @summary English locale (Canada).
 * @language English
 * @iso-639-2 eng
 * @author Mark Owsiak [@markowsiak]{@link https://github.com/markowsiak}
 * @author Marco Imperatore [@mimperatore]{@link https://github.com/mimperatore}
 */ var locale = (/* unused pure expression or super */ null && ({
    code: 'en-CA',
    formatDistance: ,
    formatLong: ,
    formatRelative: _en_US_lib_formatRelative_index_js__WEBPACK_IMPORTED_MODULE_2__/* ["default"] */.Z,
    localize: _en_US_lib_localize_index_js__WEBPACK_IMPORTED_MODULE_3__/* ["default"] */.Z,
    match: _en_US_lib_match_index_js__WEBPACK_IMPORTED_MODULE_4__/* ["default"] */.Z,
    options: {
        weekStartsOn: 0 /* Sunday */ ,
        firstWeekContainsDate: 1
    }
}));
/* unused harmony default export */ var __WEBPACK_DEFAULT_EXPORT__ = ((/* unused pure expression or super */ null && (locale)));
})

In a in my attempts to repro here (https://github.com/bostondv/rspack-treeshaking-test/tree/d8911d4a3c1eb2a7337dfe11d2bfafca899219b7) the chunks output correctly. Only the used locale modules are included and of the two that are included all properties in the locale object have values.

"493": (function (__unused_webpack_module, __webpack_exports__, __webpack_require__) {
"use strict";
/* harmony import */var _lib_formatDistance_index_js__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(/*! ./_lib/formatDistance/index.js */"991");
/* harmony import */var _lib_formatLong_index_js__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(/*! ./_lib/formatLong/index.js */"763");
/* harmony import */var _lib_formatRelative_index_js__WEBPACK_IMPORTED_MODULE_2__ = __webpack_require__(/*! ./_lib/formatRelative/index.js */"865");
/* harmony import */var _lib_localize_index_js__WEBPACK_IMPORTED_MODULE_3__ = __webpack_require__(/*! ./_lib/localize/index.js */"950");
/* harmony import */var _lib_match_index_js__WEBPACK_IMPORTED_MODULE_4__ = __webpack_require__(/*! ./_lib/match/index.js */"219");

/**
 * @type {Locale}
 * @category Locales
 * @summary English locale (United States).
 * @language English
 * @iso-639-2 eng
 * @author Sasha Koss [@kossnocorp]{@link https://github.com/kossnocorp}
 * @author Lesha Koss [@leshakoss]{@link https://github.com/leshakoss}
 */ var locale = {
    code: 'en-US',
    formatDistance: _lib_formatDistance_index_js__WEBPACK_IMPORTED_MODULE_0__/* ["default"] */.Z,
    formatLong: _lib_formatLong_index_js__WEBPACK_IMPORTED_MODULE_1__/* ["default"] */.Z,
    formatRelative: _lib_formatRelative_index_js__WEBPACK_IMPORTED_MODULE_2__/* ["default"] */.Z,
    localize: _lib_localize_index_js__WEBPACK_IMPORTED_MODULE_3__/* ["default"] */.Z,
    match: _lib_match_index_js__WEBPACK_IMPORTED_MODULE_4__/* ["default"] */.Z,
    options: {
        weekStartsOn: 0 /* Sunday */ ,
        firstWeekContainsDate: 1
    }
};
/* harmony default export */ __webpack_exports__.Z = (locale);
})

IWANABETHATGUY commented 8 months ago

Can you try to disable the optimization.splitChunks and check if it is still reproducible?

IWANABETHATGUY commented 8 months ago

I'm still unable to reproduce with a simple case but if its helpful it only occurs with newTreeshaking and optimization.splitChunks enabled in production mode.
{
    optimization: {
      splitChunks: {
        chunks: 'all',
      },
    },
    experiments: {
      rspackFuture: {
        newTreeshaking: true,
      },
    },
}
It does not occur with other splitChunks.chunks options such as async.

I noticed this comment, and I could replicate in https://github.com/web-infra-dev/rspack/issues/5595, we will fix this issue ASAP

IWANABETHATGUY commented 8 months ago

Hello, can you help with testing this version? 0.5.3-canary-431379d-20240206044033

bostondv commented 8 months ago

Hello, can you help with testing this version? 0.5.3-canary-431379d-20240206044033

Canary appears to fix the issue in our app 🙏

web-infra-dev / rspack