Side-effects options breaks Lodash's chain method

[the-spyke]

Do you want to request a feature or report a bug?

Bug.

What is the current behavior?

Importing chain from lodash-es@4.17.10 doesn't work with sideEffects: true (production mode).

TypeError: A(...).keys is not a function

If the current behavior is a bug, please provide the steps to reproduce.

import { chain } from "lodash-es";

const object = { y: 3, x: 5 };
const x = chain(object)
    .keys()
    .sort()
    .value();

What is the expected behavior?

Should work as it does with sideEffects: false.

If this is a feature request, what is motivation or use case for changing the behavior?

Please mention other relevant information such as the browser version, Node.js version, webpack version, and Operating System.

Node: 10.0.0
Webpack: 4.6.0
Lodash-es: 4.17.10

[montogeek]

Thanks for your report. It would be great if you reduce your issue to a small reproducible example. Best put this example into a github repository together with instructions how to get to the problem.

[sokra]

That's true. Let me explain why and how this could be fixed in lodash-es @jdalton

chain imports wrapperLodash and creates an object of this wrapper.

The other part is lodash.default.js which also imports wrapperLodash and adds methods to it's prototype. This is a sideeffect in lodash.default.js.

And there is the entrypoint lodash.js which imports chain, lodash.default.js and some other files.

With the default ESM semantics, lodash.default.js is evaluated when lodash.js is used. In this case all the methods are added to the wrapperLodash prototype.

But with lodash-es opt-in into sideEffects: false (for all files) lodash.default.js is not evaulated when lodash.js is used (but not the default export). In this case no methods are added to the wrapperLodash prototype.

So the problem is that lodash.default.js has side-effects, but is flagged as side-effects free in package.json. That's incorrect.

Even without sideEffects: false this can be a problem. I. e. in this code:

import chain from "lodash-es/chain";

chain(...).keys().value(); // keys is not available

How can this be fixed in lodash-es?

• The side effects of adding methods to the wrapperLodash.prototype should be moved into a separate file. Let's call it _registerMethods.js
• This file should be flagged as "has-side-effect" with "sideEffects": [ "./_registerMethods.js" ]
• lodash.default.js should import this file. This kind of import chain hides with side-effect behind the default export and bundle size is not affected when not using these exports.
• chain.js should import this file.
• In general all modules that depend on the lodash prototype should import this file.

But this has a negative side effect: When using chain the whole lodash library will be included in the bundle.

This is a general problem with methods on the prototype. These kind of APIs should probably avoided.

The ES Pipeline operator (https://github.com/tc39/proposal-pipeline-operator) is on it's way to APIs that could look like this:

import { chain, keys, sort, value } from "lodash";

const object = { y: 3, x: 5 };
const x = object
    |> chain(keys)
    |> chain(sort)
    |> value;

[jdalton]

Hi @the-spyke!

Chaining isn't really supported with cherry-picking. There's some WIP transforms for babel-plugin-lodash, but I'm not really looking to carry it forward. As @sokra mentioned chaining would pull in all of Lodash so something I don't want to encourage.

[the-spyke]

Thank you guys for such extensive answers. I haven't got any time yesterday to investigate it by my self, so I've cheated and just posted this issue :)

First of all I want to say about size. I know that .chain() will pull in the entire Lodash. But I also know that in my application (~5MB minified uncompressed) even without .chain() there is a half of Lodash (saw this in bundle analyzer). So, I really doubt that winning 1% of bundle size worth decreasing developer experience and code simplicity. And there's no point in babel-plugin-lodash with Webpack 4.

I'd say that named imports are a little bit different to cherry-picked modules like lodash/keys. If I import a file, I know that entry point will not be executed. But it isn't obvious with import { keys } from. If lodash-es is marked as a side-effect free, then it at least shouldn't provide a named export for .chain().

Another interesting question is how my app is worked before. We have only named and namespaced imports of lodash-es. No default imports. And with disabled concatenation and UglifyJS I see in the bundle:

/* harmony import */
var _lodash_default_js__WEBPACK_IMPORTED_MODULE_316__ = __webpack_require__(1993);
/* harmony reexport (safe) */
__webpack_require__.d(__webpack_exports__, "default", function() {
    return _lodash_default_js__WEBPACK_IMPORTED_MODULE_316__["a"];
});

I don't know yet why it wasn't stripped away.

[webpack-bot]

This issue had no activity for at least half a year.

It's subject to automatic issue closing if there is no activity in the next 15 days.

[webpack-bot]

Issue was closed because of inactivity.

If you think this is still a valid issue, please file a new issue with additional information.