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

Webpack Tree-shaking fails with static properties but not static getters #8308

Closed
simonsarris opened this issue Oct 30, 2018 · 5 comments
Closed

Comments

@simonsarris
Copy link

simonsarris commented Oct 30, 2018

Bug report

What is the current behavior?

If an ES6 class contains a static getter property but the class itself is un-used, it will correctly be removed from the bundle. So this is tree-shake compatible:

// works, tree shaking will remove this class
class someClass {
  ...
  static get staticProperty1() { return 'prop1'; }
}

However, if an ES6 class contains a (attach style) static property, and the class is un-used, it will not be removed from the bundle. Tree shaking fails on this case, in other words:

class someClass {
  ...
}
// does not work, if present, tree shaking will not remove someClass
someClass.staticProperty2 = 'prop2';

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

Make a file, app.js with two classes:

class h1 {
  constructor() {
    console.log('hello!');
  }
}

class g1 {
  constructor() {
    console.log('goodbye!');
  }
  // to test, comment out EITHER this line:
  static get staticProperty1() { return 'prop1'; }
}
// OR this line, then reverse them to test again:
// g1.staticProperty2 = 'prop2';

export { g1 as goodbye, h1 as hello }

and an init.js:

import { hello } from './app.js';

window.init = function() {
var h = new hello();
window.h = h; // just in case it gets minifed away

note how we only import hello, and NOT goodbye. So we expect goodbye to be absent from the resulting bundle.

Then build with webpack defaults. If this is uncommented:

static get staticProperty1() { return 'prop1'; }

The bundle.js will not contain the goodbye class.

However if that one is commented out, and this one is uncommented:

g1.staticProperty2 = 'prop2';

The bundle will include the goodbye class, even though it is never used.

What is the expected behavior?

In both cases, different ways of putting static stuff on a class should not prevent the class from being removed from the bundle. Both times the goodbye class should be removed from the bundle, since it is not used or referenced anywhere.

Other relevant information:
webpack version: 4.23.1
Node.js version: v9.10.1
Operating System: Windows 10
Additional tools:

@sokra
Copy link
Member

sokra commented Oct 30, 2018

The minimizer doesn't detect this as dead code.

You can try the terser-webpack-plugin with the pure_getters: true option. This seem to be able to remove this code.


Code like this g1.staticProperty2 = 'prop2'; can actually case side-effects, if setters are registered.

@simonsarris
Copy link
Author

Thanks, I will give the plugin a look. This is from isolating tree-shaking issues in a larger library as you might expect.

Good point on the side-effects, but the "if" seems deterministic enough.

Does this mean webpack has no intention of changing this behavior in the future?

@sokra
Copy link
Member

sokra commented Oct 30, 2018

Does this mean webpack has no intention of changing this behavior in the future?

webpack is not doing the Dead-Code-Elimination. It's done by uglify-js2 (webpack 4) or terser (webpack 5). If you want this behavior changed, that's where to look.

@kzc
Copy link

kzc commented Oct 31, 2018

Rollup can detect and drop such unused static properties on classes and functions.

Terser and Uglify, on the other hand, will never get rid of classes and functions with static properties set in the event there's a side effect. pure_getters will not help in this scenario.

$ cat ex1.js
foo();
class A {}
A.x = 1;
A.y = 2;
bar();
$ cat ex1.js | terser --toplevel -mc pure_getters
foo();class a{}a.x=1,a.y=2,bar();

In order to achieve the result you're looking for you'd have to wrap such classes and functions within a pure IIFE as follows to instruct terser/uglify there are no side effects if not referenced:

$ cat ex2.js
foo();
var A = /*@__PURE__*/(function() {
    class A {}
    A.x = 1;
    A.y = 2;
    return A;
})();
bar();
$ cat ex2.js | terser --toplevel -mc
foo();bar();

Babel 7 emits its ES5 generated class code within pure IIFEs for this reason.

Related Babel discussion to generate ES static class properties within a pure IIFE:

@simonsarris
Copy link
Author

Thanks @sokra and @kzc. Since webpack per se isn't doing code elimination anyway I'm gonna close this.

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