fix: replace is-number dep with a one-liner

[talentlessguy]

This PR replaces is-number package with a one-liner with identical code. It passes all the tests (npm run test).

This tiny change saves 440GB weekly traffic:

Package size report
===================

Package info for "[email protected]": 33 kB
  Released: 2019-04-07 06:04:37.03 +0000 UTC (277w2d ago)
  Downloads last week: 43,837,006
  Estimated traffic last week: 1.5 TB

Removed dependencies:
  - [email protected]: 10 kB (30.06%)
    Downloads last week: 43,875,245
    Downloads last week from "[email protected]": 43,837,006 (99.91%)
    Estimated traffic last week: 440 GB
    Estimated traffic from "[email protected]": 440 GB (99.91%)

Estimated package size: 33 kB → 23 kB (69.94%)
Estimated traffic over a week: 1.5 TB → 1.0 TB (440 GB saved)

[talentlessguy]

Another option is using typeof min === 'number' but I need to investigate whether anybody is supplying string-as-number input for this lib

[VigoTes]

Great job

[jonschlinkert]

but I need to investigate whether anybody is supplying string-as-number input for this lib

It's almost always going to be a string. This library was created to use with parsed values.

Does this pass all of the is-number unit tests?

[mrlkn]

Similar PR opened two years ago and closed by @jonschlinkert

You are single handedly contributing to the climate change for two years.

[talentlessguy]

but I need to investigate whether anybody is supplying string-as-number input for this lib

It's almost always going to be a string. This library was created to use with parsed values.

Does this pass all of the is-number unit tests?

I haven't tried it against is-number test suite but instead ran tests for this lib locally. in case its mostly expecting string input, I will not replace it with typeof

[jonschlinkert]

You are single handedly contributing to the climate change for two years.

This PR and this entire way of thinking is a waste of time and energy.

[talentlessguy]

guys let's be nice, the intent of this PR is to reduce network bandwidth because npm ships stuff as tarballs (and not source directly) and to reduce supply chain / dependency graph (it takes time to parse it + adds potential security risks), I have no political or any other reasons for this and don't approve of insulting people

no need to be rude to each other

[talentlessguy]

Does this pass all of the is-number unit tests?

happy to report that yes, it does: 111 passing (9ms)

[jonschlinkert]

guys let's be nice, the intent of this PR is to reduce network bandwidth because npm ships stuff as tarballs (and not source directly) and to reduce supply chain / dependency graph

NPM doesn't bundle your code. NPM is a package manager. This is like saying you should remove your README because it saves bandwidth.

[talentlessguy]

guys let's be nice, the intent of this PR is to reduce network bandwidth because npm ships stuff as tarballs (and not source directly) and to reduce supply chain / dependency graph

NPM doesn't bundle your code. NPM is a package manager. This is like saying you should remove your README because it saves bandwidth.

npm doesn't bundle it, but it downloads the whole tarball, which wastes bandwidth (people still pay for internet) and slows down CI during installs. In a scale of one package it might not be noticeable, but when you got a lot of them (and you usually do) it slows things down monumentally

also yes, ideally a package should ship as small README as possible. there's 0 reasons for a library consumer to have it stored on their machine. only source files and package manifest matter

[jonschlinkert]

but it downloads the whole tarball

This is how development works. Nothing is optimized for production.

but when you got a lot of them (and you usually do) it slows things down monumentally

I'd rather have builds that take a little longer with code that works, than fast builds with shitty code.

---

To all of the people that gave this a thumbs down, first of all I was being felicitous. Second, you clearly didn't understand the point I was making: this PR and ones like it will increase bundle size for your END USERS.

As I stated above, nothing is optimized for PRODUCTION by the PACKAGE MANAGER. Reducing dependencies might make things download faster for you personally during dev, but there is 1) an exponential number of end users for every developer, 2) an exponential number of downloads of bundled production code for every one download of a package during development. Every time I see stats about download sizes like the one above it makes me cringe. If every dependency was a collection of micro-libraries, packages would be a fraction of the size they are today, because no code would be duplicated, and only the best variants of any given concept would be used. Instead, PRs like this one are making it increasingly likely that packages will be more bloated with more bugs over time.

[ineanto]

but it downloads the whole tarball

This is how development works. Nothing is optimized for production.

but when you got a lot of them (and you usually do) it slows things down monumentally

I'd rather have builds that take a little longer with code that works, than fast builds with shitty code.

I don't think it can be answered as simply as "this PR introduces shitty code". As demonstrated by the above post by @talentlessguy, the is-number test suite passes without any error. This PR is about taking an easy-to-make decision: reduce bandwith usage at the cost of litteraly nothing.

[talentlessguy]

but it downloads the whole tarball

This is how development works. Nothing is optimized for production.

but when you got a lot of them (and you usually do) it slows things down monumentally

I'd rather have builds that take a little longer with code that works, than fast builds with shitty code.

but it downloads the whole tarball

This is how development works. Nothing is optimized for production.

but when you got a lot of them (and you usually do) it slows things down monumentally

I'd rather have builds that take a little longer with code that works, than fast builds with shitty code.

...this is the same code as in is-number.... wtf

[churchianity]

but it downloads the whole tarball

This is how development works. Nothing is optimized for production.

but when you got a lot of them (and you usually do) it slows things down monumentally

I'd rather have builds that take a little longer with code that works, than fast builds with shitty code.

"shitty code" isn't a productive or substantive way to describe the changes in this, or any PR. If you have an issue with it, please describe it.

[doowb]

If you have an issue with it, please describe it.

I have a couple of issues with this:

1. I was about to close it as a duplicate because in another PR, it was already stated that reducing dependencies alone is not a goal in this project. And that doing so, would cause even more work downstream.

2. The original PR did not copy all of is-number, which may cause unforeseen issues later. There's a reason that is-number has been iterated on and refactored to be stable and reliable. By inlining that code (even if it is small), puts more maintenance burden on this library. If a bug is found and fixed in is-number, then this library and any other dependents get the benefit of that fix. Without using is-number as a dependency, we'd have to do more work to monitor the source dependency and update the code here.

As for bundle size... I agree with @jonschlinkert that your final build step can produce a compact bundled version of your application. Also, for CI builds, that bandwidth is most likely between (or even within) data centers.

[paulmillr]

I've merged it.

If a bug is found and fixed in is-number

Something as simple as is-number should not have a separate library.

Looking at the code, it's also misleading. It's not is-number. It is (js integer) number or a string which looks like js integer.

[43081j]

just to add my two cents - its unlikely we'd do such a breaking change here, but i think really this library should accept strictly numbers and numbers-as-strings (things which Number(str) will suffice)

more for anyone seeing this and thinking of copying the code to other projects - maybe don't. just assert that typeof n === 'number' (and throw if it isn't) or, if you must support strings, something like typeof n === 'number' || !Number.isNaN(Number(n)).

[jonschlinkert]

Since I'm still getting notifications about this and other packages with is-number...

i think really this library should accept strictly numbers and numbers-as-strings (things which Number(str) will suffice)

That's why is-number was used! The fact that we even need to discuss it is more than enough justification to use a dependency for it.

Dependencies can REDUCE bundle size

1. Package size is only relevant during development. Who cares if you save 20kb during dev? It's insane that people care about this, while using bloated 2Gb (literally) monstrosities like Gatsby without a care in the world.
2. Using dependencies is more likely to reduce bundle size for end users. The bundled size of is-number is 411 bytes before it's minified. Every time someone does a PR like this one, we will obviously geometrically increase bundle sizes.

If you want to cut down on package size, go do PRs to remove tests, fixtures, benchmarks, docs, and any other garbage that ends up in node_modules but should have been excluded from the published package. Then, do PRs to use dependencies instead of inlined code for anything that's written more than once throughout the entire dependency tree. I'll say that again: do the exact opposite of what this PR does. Every time someone does a PR like this one, you are increasing the odds that the bundle size will geometrically grow over time. That is a mathematically provable.

Honestly things like AI autocompletion make it easier and easier to just inline something instead of using dependencies, or at least organizing that code in a central place, so it can be imported by other modules. But that's not a good thing. We're going to have increasingly bloated code, with inconsistent implementations of the same thing duplicated multiple times in one tree. But hopefully AI will solve that too...