Skip to content

Add new treeshaking options - #1760

Merged
lukastaegert merged 4 commits into
release-0.52from
more-treeshaking-options
Nov 25, 2017
Merged

Add new treeshaking options#1760
lukastaegert merged 4 commits into
release-0.52from
more-treeshaking-options

Conversation

@lukastaegert

@lukastaegert lukastaegert commented Nov 24, 2017

Copy link
Copy Markdown
Member

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

kzc commented Nov 24, 2017

Copy link
Copy Markdown
Contributor

Course-grained propertyReadSideEffects should be fine for most scenarios.

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

@lukastaegert

Copy link
Copy Markdown
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.

2 participants