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

Update package.json #270

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JagjitKaur
Copy link

Enable tree shaking for pako to reduce its package size in my app

Enable tree shaking
@JagjitKaur
Copy link
Author

JagjitKaur commented Mar 4, 2023

@puzrin Can anyone please help with the approval here?

@Greenheart
Copy link

Which bundler are you using? I've not heard about sideeffects in package.json before - and this option is not mentioned in the npm docs as far as I can see 🙂

@JagjitKaur
Copy link
Author

JagjitKaur commented Mar 26, 2023 via email

@Greenheart
Copy link

Thanks! I don't use webpack anymore, but I guess this will be relevant for webpack users :)

@@ -2,6 +2,7 @@
"name": "pako",
"description": "zlib port to javascript - fast, modularized, with browser support",
"version": "2.1.0",
"sideeffects": false,

Choose a reason for hiding this comment

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

According to the docs you linked, the property should be sideEffects- note the casing difference.

Suggested change:

Suggested change
"sideeffects": false,
"sideEffects": false,

Copy link

@JagjitK JagjitK Mar 27, 2023

Choose a reason for hiding this comment

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

I have done the change. @Greenheart Could you please approve?
JagjitKaur#1

Choose a reason for hiding this comment

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

Could you push the change to the same branch as this PR is based on (JagjitKaur:Pako/tree-shaking)?

I'm not a maintainer of this project, so I can't decide whether to merge or not.

However, this is consistent with the Webpack docs - hope it helps your project :)

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.

3 participants