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

So many dependencies.... #1417

Closed
jgeewax opened this issue Jul 10, 2016 · 17 comments
Closed

So many dependencies.... #1417

jgeewax opened this issue Jul 10, 2016 · 17 comments
Assignees
Labels
core type: question Request for information or clarification. Not an issue.

Comments

@jgeewax
Copy link
Contributor

jgeewax commented Jul 10, 2016

Quite a few of these are really small, and mention that the functionality might already be included by something like lodash... Since we're depending on so many, is it time to make the cut over to use lodash or similar? Also, lodash has modularized packages that might be worthwhile...

  • propprop -> lodash.property
  • array-uniq -> lodash.uniq
  • arrify -> lodash.castarray
    etc
@stephenplusplus
Copy link
Contributor

If we shrink the number of our dependencies in favor of lodash, then the download size goes up:

$ npm install --save lodash
$ ls node_modules/lodash*
lodash/ 591 files
lodash._baseassign/ 4 files
lodash._basecopy/ 4 files
lodash._baseflatten/ 4 files
lodash._bindcallback/ 4 files
lodash._createassigner/ 4 files
lodash._getnative/ 4 files
lodash._isiterateecall/ 4 files
lodash.assign/ 4 files
lodash.flatten/ 4 files
lodash.isarguments/ 4 files
lodash.isarray/ 4 files
lodash.keys/ 4 files
lodash.noop/ 4 files
lodash.restparam/ 4 files

If we simply switch over to using the lodash module versions of things we are already using (lodash.property, etc), what benefit does that give us? I haven't dug very deep, but at a quick glance, arrify works differently than castArray in how it treats undefined arguments.

So we're trading one metric (dep count) for another (download size), and I don't think either one really matters-- at least not in this scenario. But using small, focused modules that are doing the job we need and nothing more offers a bigger win than the other options at this point, doesn't it?

@jgeewax
Copy link
Contributor Author

jgeewax commented Jul 10, 2016

I'm worried about the bus-factor (seeing as most of these libraries are owned and maintained exclusively by you).

@stephenplusplus
Copy link
Contributor

I don't understand how that's relevant?

@jgeewax
Copy link
Contributor Author

jgeewax commented Jul 10, 2016

If you're hit by a bus and there's a problem, we have to actually figure out what to do. More maintainers and more users usually means more coverage in case there's a problem. It also means that people who need to debug a problem have to go look at how arrify works (and how it's different from castArray).

I'm just a smidge worried because creating so many packages hints at a case of NIH syndrome.

@stephenplusplus
Copy link
Contributor

Gotcha on the bus part. If you need fixes on the modules and I'm not... available... you can transition to a new one after the appropriate grieving period :)

I only wrote one of the modules which you mentioned above, the other two are @sindresorhus. Small modules is an npm philosophy. This is not NIH syndrome. This is npm and Node.

@jgeewax
Copy link
Contributor Author

jgeewax commented Jul 10, 2016

OK you own only one of the dependencies for all of gcloud-node? I thought it was more when I looked last....

@stephenplusplus
Copy link
Contributor

In the case of propprop, I can deprecate that in favor of lodash.property. The modularized releases of lodash weren't available back then. If I have any others, I'll do the same.

@stephenplusplus
Copy link
Contributor

No, I own a few (google-auto-auth, gce-images, retry-request, more). I'm just talking about the ones that can be replaced by lodash equivalents.

@stephenplusplus stephenplusplus added enhancement type: question Request for information or clarification. Not an issue. core labels Jul 10, 2016
@stephenplusplus
Copy link
Contributor

Would you like me to make anyone owners on the other modules I own? I would gladly drop most of them in the GoogleCloudPlatform org.

@jgeewax
Copy link
Contributor Author

jgeewax commented Jul 10, 2016

I get that Node "does lots of packages" (even though it leads us to do some stuff I can't say I fully agree with -- see http://www.haneycodes.net/npm-left-pad-have-we-forgotten-how-to-program/). My worry is simply that we're taking dependencies on a lot of things rather than a few larger (likely better maintained) things.

I noticed a few patterns when looking through our deps:

  1. There are a lot of packages.
  2. a good chunk of them seem like they're common utility functions, which might be swapped out with a library like lodash.
  3. another chunk of them seem focused on streams, which might be swapped out with a library like mississippi/2
  4. a few of these are google cloud specific, which makes me wonder if maybe those are better owned by GoogleCloudPlatform... not sure if that's the right answer, but maybe.
  5. a lot of these packages (i counted 10 which is about a third) are owned by @stephenplusplus

as always, point out where i have things wrong. im all for languages doing things their own way, but i cant say i understand the 30+ deps where 10+ are utility/stream-focused.... any help you can offer would be appreciated.

stuff to be swapped by (say) lodash

  • "array-uniq": "^1.0.2",
    
  • "arrify": "^1.0.0",
    
  • "methmeth": "^1.0.0",
    
  • "prop-assign": "^1.0.0",
    
  • "propprop": "^0.3.0",
    
  • "lodash.flatten": "^4.2.0",
    
  • "string-format-obj": "^1.0.0",
    

stuff to be swapped with (say) mississippi or mississippi2

  • "concat-stream": "^1.5.0",
    
  • "duplexify": "^3.2.0",
    
  • "through2": "^2.0.0"
    
  • "pumpify": "^1.3.3",
    
  • "stream-events": "^1.0.1",
    
  • "split-array-stream": "^1.0.0",
    

stuff that seems googley and might be better owned elsewhere

  • "gce-images": "^0.2.0",
    
  • "gcs-resumable-upload": "^0.7.1",
    
  • "google-auto-auth": "^0.2.4",
    
  • "google-proto-files": "^0.2.1",
    

libs owned by stephenplusplus

  • "hash-stream-validation": "^0.2.1",
    
  • "string-format-obj": "^1.0.0",
    
  • "methmeth": "^1.0.0",
    
  • "prop-assign": "^1.0.0",
    
  • "propprop": "^0.3.0",
    
  • "gce-images": "^0.2.0",
    
  • "gcs-resumable-upload": "^0.7.1",
    
  • "google-auto-auth": "^0.2.4",
    
  • "google-proto-files": "^0.2.1",
    
  • "stream-events": "^1.0.1",
    

ones that i guess seem ok...

  • "JSONStream": "^1.0.7",
    
  • "async": "^1.4.2",
    
  • "create-error-class": "^2.0.1",
    
  • "ent": "^2.2.0",
    
  • "grpc": "^0.14.1",
    
  • "hash-stream-validation": "^0.2.1",
    
  • "dns-zonefile": "0.1.18",
    
  • "extend": "^3.0.0",
    
  • "mime-types": "^2.0.8",
    
  • "is": "^3.0.1",
    
  • "once": "^1.3.1",
    
  • "rgb-hex": "^1.0.0",
    
  • "retry-request": "^1.2.3",
    
  • "request": "^2.70.0",
    
  • "modelo": "^4.2.0",  <- do we use this all the time though? 
    

@stephenplusplus
Copy link
Contributor

stephenplusplus commented Jul 10, 2016

  • array-uniq - I'm not opposed to switching to lodash, though I see it as a wasted effort (as stated earlier)
  • arrify - discussed above; not identical, would rather stick with what we're using.
  • methmeth - discussed later
  • prop-assign - discussed later
  • propprop - discussed later
  • lodash.flatten
  • string-format-obj - discussed later

  • hash-stream-validation -> nothing existed like this, so I made it
  • string-format-obj -> many existed with strange quirks of how you need to use them. If this exists in lodash, I'll gladly replace. The way I designed it turned out to closely match what is now available in Node with template literals. I bet I could find a template literal shim and deprecate string-format-obj.
  • methmeth -> same, didn't see that this exists anywhere. Does lodash have it? Again, I'll glady replace.
  • prop-assign -> same as above
  • propprop -> same as above

  • gce-images - happy to move to the org or add owners.
  • gcs-resumable-upload - same as above.
  • google-auto-auth - same as above, though google-auth-library is still supposed to receive changes that deprecates this one (Remove dependency on google-auto-auth and use google-auth-library #832), so I can't see the org wanting to claim it.
  • google-proto-files - happy to move to the org or add owners.

"modelo": "^4.2.0", <- do we use this all the time though?

We use it where we need multiple inheritance, which Node doesn't support out of the box. A Pub/Sub Subscription is both an EventEmitter and a GrpcServiceObject, for example.

I don't want to point out where you're wrong, you're just stating your thoughts. But to respond to your points:

There are a lot of packages.

We have a big library that supports many unique APIs, which is why we have many dependencies. Instead of being a thin layer on top of an HTTP transport, we set out to decorate things so that the user is happier. That carried with it the cost of dependencies.

a good chunk of them seem like they're common utility functions, which might be swapped out with a library like lodash.

I'll need to do a deeper dive into what lodash offers, but it seems like maybe we can replace 2-3 of our dependencies here. I wouldn't say that's worth it, personally. Not sure what the magic number would be that makes me "feel" like it's "worth it"-- maybe 10. I like small, focused modules, so my magic number might be higher than yours.

another chunk of them seem focused on streams, which might be swapped out with a library like mississippi/2

Streams are hard, the number of libraries reflects the number of scenarios, use cases, and problems that need to be solved. Mississippi is just a wrapper around some of them, but not the ones we need.

a few of these are google cloud specific, which makes me wonder if maybe those are better owned by GoogleCloudPlatform... not sure if that's the right answer, but maybe.

As stated above, I'm willing to move repos or add owners to make the concerned parties feel more comfortable.

a lot of these packages (i counted 10 which is about a third) are owned by @stephenplusplus

For the google-themed modules, see above. For the bus factor, I answered that earlier in the thread. For the "NIH syndrome", I addressed that earlier in the thread as well as in this post.

@sindresorhus
Copy link

stuff to be swapped with (say) mississippi or mississippi2

"concat-stream": "^1.5.0",
"duplexify": "^3.2.0",
"through2": "^2.0.0"
"pumpify": "^1.3.3",
"stream-events": "^1.0.1",
"split-array-stream": "^1.0.0",

Uhm, mississippi is just a collection of many of those modules. https://github.com/maxogden/mississippi/blob/24ad398bf642bc7a55abf4f1e820aac5e443efcc/index.js

@jgeewax
Copy link
Contributor Author

jgeewax commented Jul 11, 2016

Gotcha -- if we really think we're doing the right thing, then this LGTM. I'll close by EOD unless we get a bit more feedback.

@stephenplusplus
Copy link
Contributor

I will close on your behalf, but just re-open if you still wanted to invite anyone to the discussion.

@jmdobry
Copy link
Contributor

jmdobry commented Jul 13, 2016

I think the most important part of this discussion is the part about the "googley" packages (though maybe not google-auto-auth, per your comment). Either moving them to GoogleCloudPlatform org or adding some Googlers as collaborators works for me. I'd think that the protos (googleapis org) team would be interested in owning/generating google-proto-files.

@stephenplusplus
Copy link
Contributor

👍 to everything. I'll start by adding you both to the googley packages and as npm owners.

@stephenplusplus
Copy link
Contributor

Done!

  • google-proto-files
  • google-auto-auth
  • gce-images
  • gcs-resumable-upload

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

4 participants