Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dead-code elimination of unused default parameters #2185

Closed
bitjson opened this issue Apr 15, 2022 · 4 comments
Closed

Dead-code elimination of unused default parameters #2185

bitjson opened this issue Apr 15, 2022 · 4 comments

Comments

@bitjson
Copy link

bitjson commented Apr 15, 2022

During dead-code elimination/tree shaking, default parameters that aren't used by the bundle can be removed.

For this simple example:

module.js:

const internalAdd = (a, b) => (a === '0' ? b : a + b); // should not appear in bundle
export const addAndLog = (a, b, impl = internalAdd) => console.log(impl(a, b));

app.js:

import { addAndLog } from './module.js';
const myAdd = (a, b) => a + b;
addAndLog(1, 2, myAdd); // => 3

The produced bundle should include only:

const addAndLog = (a, b, impl) => console.log(impl(a, b));
const myAdd = (a, b) => a + b;
addAndLog(1, 2, myAdd); // => 3

A detailed example (testing with all known bundlers) is available at this repo: https://github.com/bitjson/shake-default-params

More context

I maintain Libauth, a library that offers WebAssembly crypto implementations (ripemd160, sha1, sha256, sha512, and secp256k1). As a pure ESM library, Libauth can asynchronously instantiate each WASM implementation internally, exporting simple interfaces that behave like collections of JS-only functions (with better performance). Many of Libauth's exported functions also use one of these built-in WASM instances as a default parameter. For example, decodeBase58Address has the definition:

export const decodeBase58Address = (
  address: string,
  sha256: { hash: (input: Uint8Array) => Uint8Array } = internalSha256
) => {
  // ...
};

Most applications can simply call decodeBase58Address(address) to automatically use the default, WASM-based sha256 implementation (internalSha256).

However, applications that already have another sha256 implementation can provide that implementation as the second parameter: decodeBase58Address(address, mySha256Implementation). In this case, the default parameter (internalSha256) is dead code and should be possible to eliminate from the application's bundle, saving hundreds of KBs from those bundles.

@evanw
Copy link
Owner

evanw commented Apr 16, 2022

This is too much deduction for esbuild to do. Producing optimal code in this sense is not a goal of esbuild. It would essentially require a rewrite to deal with complex indirect analysis like this (along with the worklist algorithm this would need), which is not going to happen.

I think the fact that no other bundler does this either should be a good signal that this isn't a good API design for your library if you care about dead-code elimination. By far the simplest thing to do here is to just split your API into separate functions instead of trying to change how the whole ecosystem works to fit your API.

@bitjson
Copy link
Author

bitjson commented May 17, 2022

Thanks for the response @evanw, that's definitely understandable. I'm sure most applications would see <1KB bundle size reductions with this optimization. And I agree, if I urgently needed to optimize this kind of bundle I would just change the API. I just wanted to open the issue in case it was somewhat reasonable to implement in esbuild.

For anyone curious (or if for some reason you urgently need this behavior), Rollup will now support tree-shaking default parameters (and PR is here: rollup/rollup#4498).

@triallax
Copy link

triallax commented Jul 20, 2022

For anyone curious (or if for some reason you urgently need this behavior), Rollup will now support tree-shaking default parameters (and PR is here: rollup/rollup#4498).

Note that the behavior was rolled back, as it proved far too brittle: rollup/rollup#4520

@evanw
Copy link
Owner

evanw commented Jul 25, 2022

Closing as "won't fix."

@evanw evanw closed this as completed Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants