Webpack optimization bailout due to side effects in rambda.mjs

[fatawesome]

Hi and thanks for the lib!

While replacing lodash with rambda I found that webpack optimizations cannot be performed due to top-level variables. Basically, webpack treats variable definitions, such as const adjust = curry(adjustFn) as a side-effect. The actual error/warning from webpack is

Statement (VariableDeclaration) with side effects in source code at 28:0-31

I've also made a reproduction repo - https://github.com/fatawesome/rambda-side-effects-example What do you think on this issue?

[selfrefactor]

I will check. Thanks for report and the example repo.

[selfrefactor]

I can see that there is issue with Rambda and your setup. I see that Ramda behaves better. I found that Rambda 5.0.0 is one of the latest versions that works well. I will investigate further.

[selfrefactor]

I think I found a solution. I will make a release and ask you to test it out.

[fatawesome]

Fastest reaction i've ever seen <3 Will you make a dev version of the package or I can build from sources?

[selfrefactor]

I will release a patch release, but I will do that during the weekend. I will write here once it is released.

[selfrefactor]

There will be some delay as I want to improve tree shaking testing as part of this ticket. Otherwise the solution seems simple, but I want to be sure that it will work to other bundlers as well.

[fatawesome]

Sure thing, take your time Would be interesting to see how you test the bundle :)

[selfrefactor]

I will use your config as base for webpack check and run it against local build of the library. Then, I will output the size of the bundle.

BTW, with your exact configuration, even the simplest library version(one export of simple function as R.add), still triggers warning, so I am unsure how reliable these warnings are. What I see is that the final output size through minification is influenced by declarations in package.json.

[fatawesome]

Yep, I can see that tree-shaking does not work even if I leave rambda.mjs file as the following

function F() { return false; }
function T() { return true; }
function add(a, b) {
  if (arguments.length === 1) return _b => add(a, _b);
  return Number(a) + Number(b);
}
export { add, F, T }

But with such ^ content I don't get any warnings about VariableDeclaration from webpack, only about my ExpressionStatement in src/index.js.

And as I add smth like

...
function curry(fn, args = []) {
  return (..._args) => (rest => rest.length >= fn.length ? fn(...rest) : curry(fn, rest))([...args, ..._args]);
}
const curriedAdd = curry(add);
export { ..., curriedAdd }

VariableDeclaration warnings are back

[selfrefactor]

It will be an error to develop around tree-shaking of particular tool, especially in the context of Vite and Parcel. I use your Webpack config as it proves that something is wrong. From what I saw, the warning are not fully correct for me, but I don't intend to dig too deep there. I mostly worry about the oblivious(as wanting too much is often a bad direction), i.e. the size of the bundle. Using minification warnings is worth only if the other tools also provide similar tools. If it is only for a webpack plugin, then I will ignore it as it can be issue with the plugin or webpack itself.

[fatawesome]

Yep, you're right here, I totally agree. If the problem with bailout warnings still has place after this issue resolution (with whatever outcome), I will try other webpack optimization techniques to see if there is a problem with Terser plugin.

But if it defines "side-effect" as it is defined in the scientific field, then he is kinda right in his conclusions.

[selfrefactor]

I think I found the solution. I will keep the issue open until the release is made and you confirm that it is fixed.

[fatawesome]

Hello! Sorry for delayed response, github decided not notify me with an email for some reason :/

Yep, tree-shaking fix definitely works. I've commited an updated stats.json to the reproduction repo. It now shows only 1 rambda side-effect, but it does not affect tree-shaking.

[selfrefactor]

thanks for the feedback @fatawesome

it helps to confirm that it works. I still have a work to do to create a process out of this, so I will close the issue as it solves the problem.