-
-
Notifications
You must be signed in to change notification settings - Fork 31
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
remove is-negative-zero #154
Conversation
Please don't submit unsolicited PRs making large refactors to open source projects without discussing them with maintainers first. |
Wanted to make PR to my own fork, but by default it set main. Sorry about it. |
What the purpose of a fork? |
I was curious about if is-negative-zero is used/how it's used, since it seems you are using the package is-negative-zero while also having same/similar code in the repositorium - helpers/isNegativeZero.js or something like this. They are equivalent in functionality afaik. |
hm, yeah that's a good point - i do both use the dep and have the helper. It'd be better (for bundle size/deduping) for all the callsites to just use the dep directly, and for the helper to re-export the dep for compat. Is that something you'd like to convert this PR into doing? |
I mean, yeah I could. I don't see how it would change bundle size though since it's just removing one line of code for a library that has 30KB. IMO it should be the other way around, removing dependency and leaving the helper - since it's basically the same line, but without dependency. Even without considering size etc, its an extra call to NPM to download it. bartekleon#1 (without that second bad commit) is proof of concept, although your call is last |
Disk size of a package has zero bearing on bundle size - by using a dep instead of the helper, the relevant line of code (which might already be in your tree via the dep) will only appear once, instead of twice. (also, bundle size has nothing to do with download size - download size is irrelevant, only "what runs in the actual application" matters) |
Nowadays it matters less since disk space and interner speed are much bigger than they used to and yes, even if removed from this package, it might still be downloaded by other project. (at the same time now imagine 100 or 200 dependencies like this, which ultimately makes let's say 6MB to download [considering 30kb/packages], since we are also downloading tests and other "garbage" [stuff we don't actually use]. And now we have 10-100 projects on our PC... That adds up fast). That's why I said it's ultimately your call. But that being said maybe one thing that actually popped up in my head would be if you could remove some files from what is published to NPM (like tests for example, since they are not typically used in code). But probably discussion about it should be left for some issue/other way of communication, not PR comments :) |
Tests and whatnot are used - |
No description provided.