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

Make all properties in TreeshakingOptions optional #2779

Merged
merged 1 commit into from Mar 28, 2019

Conversation

ndelangen
Copy link
Contributor

This is what the docs are telling me the types should be: https://rollupjs.org/guide/en#treeshake

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes
  • no

Breaking Changes?

  • yes
  • no

Description

I was using the Node API in typescript and typescript was telling me:

Type '{ pureExternalModules: boolean; }' is not assignable to type 'boolean | TreeshakingOptions'.
  Type '{ pureExternalModules: boolean; }' is missing the following properties from type 'TreeshakingOptions': annotations, propertyReadSideEffectsts(2322)
rollup.d.ts(286, 2): The expected type comes from property 'treeshake' which is declared here on type 'RollupOptions'

I was able to fix the warning by adding all 3 properties to the TreeshakingOptions.

Looking at the docs, I concluded that the interface is actually wrong.

I'm unsure this qualifies as a bugfix.

This is what the docs are telling me the types should be
@ndelangen
Copy link
Contributor Author

Have an awesome day people, this package is awesome!

Copy link
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks a lot! Small fixes like this are always very welcome!

@lukastaegert lukastaegert merged commit b3578ae into rollup:master Mar 28, 2019
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