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

Add new treeshaking options #1760

Merged
merged 4 commits into from
Nov 25, 2017
Merged

Conversation

lukastaegert
Copy link
Member

@lukastaegert lukastaegert commented Nov 24, 2017

This PR will move options.pureExternalModules to options.treeshake.pureExternalModules and make it configurable via config file and also the CLI via --treeshake.pureExternalModules/--no-treeshake.pureExternalModules.

It also adds the new options.treeshake.propertyReadSideEffects which defaults to true but can be disabled by setting it to false. If it is disabled, the following side-effects will not be detected and result will be removed if it is not used elsewhere in the code:

  • accessing a getter with side-effects

    const foo = {
      get bar() {
        console.log('effect');
        return 'bar';
      }
    }
    
    const result = foo.bar;
  • accessing members of null or undefined:

    const foo = {}
    const result = foo.bar.baz;

It is my guess that setting options.treeshake.propertyReadSideEffects: false or using --no-treeshake.propertyReadSideEffects will work without problems in 95% of the use cases and can noticably reduce bundle size especially when passing around a lot of objects as function parameters.

Also, a lot of unnecessary property access checks were removed in this PR which should give a slight speed boost.

Of course we should not forget to update the documentation once this is released.

* In the future, we might also check access in all kinds of "statements"
  configurable via config file and CLI (resolves #1678)
* Make ignoring property read side effects configurable via
  treeshake.propertyReadSideEffects via config file and CLI
* Warning: Bad things can happen if you supply both
  --treeshake (or --no-treeshake) and e.g. --treeshake.pureExternalModules
  via CLI. Depending on the order of arguments either minimist crashes
  or the options have a currently unrecognized array format. Not sure
  more should be done here, maybe this is rather a documentation issue.
@kzc
Copy link
Contributor

kzc commented Nov 24, 2017

Course-grained propertyReadSideEffects should be fine for most scenarios.

Are unknown globals still considered side effect free in all cases?

@lukastaegert
Copy link
Member Author

Yes, for now you cannot use try..catch to check for the existence of globals 😉 As this is the previous behaviour and there are no open issues regarding this yet, I decided to postpone this for a later release.

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

Successfully merging this pull request may close these issues.

None yet

2 participants