Express: Remove some of the less popular and reviewed micro modules dependencies

[ShaneQful]

Wanted to see if there was an appetite for this before I went ahead and did too much work on it but I think it would be a good idea to remove some of the micro package or single function dependencies which aren't heavily reviewed.

[ShaneQful]

I've opened a pull request as an example of a module which could easily be removed.

[dougwilson]

No, we want no giant utils files in Express. We have been trying to get rid of code from Express core for a long time now, and this would be the opposite of the direction we are going in.

[ShaneQful]

I wouldn't be as worried about some of the jshttp which are clearly maintained by the express maintainers more about the other smaller dependencies.

[dougwilson]

If you are worried, you should be committing your node_modules directory somehow to preserve it for your application deployments. We are not concerned with these dependencies, as we picked them at the time they were added by careful evaluation, even knowing the author, typically. This has been a dependency for years and there is no intention of removing any dependencies from our tree at this time.

[dougwilson]

The fact that a package can be unpublished from npm has always been know to myself for all the time I have ever used npm. I would expect anyone using npm, or at least those authoring to npm, know how the system works.

If the potential of an unpublish concerns you, perhaps you should put this energy into helping npm change this policy or creating an alternative to npm, rather than moving project back into a non-code-sharing, copy-and-paste world.

@expressjs/express-tc for additional opinions here.

[dougwilson]

Also, @ShaneQful so far this has only been a discussion of the issue in a generic sense. Perhaps it could be useful if you are trying to convince us to elaborate more on your statements.

I think it would be a good idea to remove some of the micro package or single function dependencies which aren't heavily reviewed.

So this statement brings a lot of questions to my mind:

1. Do you have a list of specific modules in my here and can you state the specific concerns for each one? Perhaps the issue is with something in particular but we are gettin g lost in the discussion being too generic here.
2. Can you define what makes a module less popular or reviewed? I have personally read the code for all current dependencies, and as the policy is to pin specific versions, and they typically do not have sub dependencies, the code won't change without us version bumping it, at which time we review it once again.
3. I don't understand what makes a dependency with a single function bad. Can you explain? Eventually this very package will only consist of a single function. What that make express a terrible module no one should depend on any more?

[LinusU]

I agree 100% with @dougwilson, starting to copy and paste small code snippets into our code base is certainly not the best way to solve the issue at hand...

[jonathanong]

Azer unpublishing left-pad was an extreme case. If you want to protect yourself from that, bundle your modules as @dougwilson said.

We should strive to be modular. We shouldn't inline anything.

That being said, I think we should consider what modules we use. If we maintain the dependency, that's okay. If we don't, we should either have some control of it or be assured that it's maintained by someone we trust. For example, I'd prefer if our dependencies preferred lodash utilities vs random micro libraries (like left-pad).

[wesleytodd]

Has anyone considered registering the npm organization for expressjs? This would also enable us to publish our own safe forks under the @expressjs scope on npm. I don't think this is _necessary_, but is is one step we could do to utilize safe and controlled versions of deps not maintained by expressjs core team members.

There is one other step we could take that is a bit more extreme, but might actually be better than the other proposed solutions I have seen from the community. That solution would be publishing fully packaged versions of express where all deps were bundled into one file using browserify --node. This would provide multiple benefits:

• Secured and tested dependencies. At publish time the deps were vetted, we could be sure that no third party had the ability to modify the code after publish
• Faster startup times. Because the built version of express would be a single file, the node runtime would actually start faster
• Users could be assured that the code they are running was the EXACT code vetted by the express core team, not relying on NPM policy to get in the way

We could even do something like publish a version called express-bundled that would be a dependency that users can install, and let users who want to "risk"[1] installing all of express' sub dependencies individually do so as usual.

[1]. I really don't think it is a risk.

[ShaneQful]

Do you have a list of specific modules in my here and can you state the specific concerns for each one? Perhaps the issue is with something in particular but we are getting lost in the discussion being too generic here.

utils-merge and to a much lesser extent because it is controlled by a contributor array-flatten. I'd have to do a little more looking to see if there are others.

Can you define what makes a module less popular or reviewed? I have personally read the code for all current dependencies, and as the policy is to pin specific versions, and they typically do not have sub dependencies, the code won't change without us version bumping it, at which time we review it once again.

While I was looking at the modules which my projects depend on, initially I was using stars on Github. This obviously isn't an ideal way to decide if a project is reviewed but I imagine there is some correlation. I think another way to define this is the number of contributors or if it's under the control of a group of developers like jshttp, lodash, pillarjs or components.

I don't understand what makes a dependency with a single function bad. Can you explain? Eventually this very package will only consist of a single function. What that make express a terrible module no one should depend on any more?

I don't have an issue with single function packages but single function packages which are controlled by a single developer for instance I would be much more comfortable if utils-merge was replaced with lodash's _.merge.

[dougwilson]

@ShaneQful you never listed what the specific concerns were for having a pinned dependency on the utils-merge module is. Can you list the specific concerns you have with this module, please?

if utils-merge was replaced with lodash's _.merge.

Does lodash supply micro packages as well? I think it would be pretty ridiculous to add all of lodash to express just to get a tiny function.

[LinusU]

Does lodash supply micro packages as well?

It does. However, that depends on a few other lodash functions in turn, might not be that bad but it makes a clean installation 184 Kb instead of utils-merge 20 Kb...

[dougwilson]

Being by a single developer does not mean anything by itself. Part of when this packages are picked, we look really hard at these things, like I said before.

Have you eve looked at who the author is? https://github.com/jaredhanson?tab=repositories I would think the same user that has Passport.js repo is pretty safe, even if it's just a simple module. He just wanted to share a function between projects, and we didn't want to copy and paste it around here, either.

I just don't understand what your concern with depending on this module is, so please, elaborate on what your specific concerns are, so we can work to address those concerns, instead of assuming the solution you are presenting is the only possible way to address them.

[LinusU]

I don't really know what is a good metric for comparing npm modules, here are a few merge functions:

| Name | Github Stars | Monthly downloads |
| --- | --- | --- |
| lodash.merge | 15 550 ¹ | 1 650 998 |
| utils-merge | 28 | 4 998 889 |
| xtend | 187 | 12 329 749 |
| object-assign | 385 | 13 067 859 |

_¹ The entire lodash is one repo_

_edit: just want to clarify that I don't think we have to change at all, as @dougwilson said and as I believe myself, the author of utils-merge is not just suddenly going to remove his packages_

[ShaneQful]

@dougwilson That's fair enough, I can certainly see where your coming from and if you guys trust the dependencies that really does a lot to assuage my initial concerns with them. Clearly a lot more forethought has gone into them than I originally thought. Thanks for talking it through and giving me more context.

Playing devils advocate, I think the only other argument that could be made regarding switching the dependencies to something like lodash is for consistency e.g. lodash has a couple of your single function dependencies as part of it e.g. it has both a merge and a flatten function.

[wesleytodd]

@ShaneQful Technically "consistency" would argue for using object-assign, since it is the most installed, assuming they have the exact same behavior.

I am for moving to more popular packages because that reduces the number of modules my projects depend on and also the build size for my browser code, but that is only a small consideration compared to having reliable and backward compatible code.

[LinusU]

I personally think that the biggest reason for switching to object-assign would be that it's a pollyfill for Object.assign that is now a days builtin into Node. That way we can simply drop the dependency when we drop Node 0.x support...

[wesleytodd]

@LinusU I agree, and not that we are using them yet, but it has support for newer es201* features which would be nice if we decide to start using them.

[dougwilson]

I also agree that it makes sense to move to Object.assign in places where objects are being merged, as it is essentially a standardized way to perform merges. I would say unless there is a very good case to make that switch within the 4.x line, we should add that as a desire to have in the 5.0 release instead.

As for the general discussion, I don't think there is much more to say on the topic. In general, a lot of these kinds of "micro libraries" are eventually being incorporated into JavaScript itself (like Object.assign) and it's just a matter of time before just using the native operations from JavaScript instead.

In addition, we talked about the desire to keep out copy-and-pasted code (why would we want to have a file full of tests for a merge function, btw, or PRs every so often trying to optimized this?). From the talking, we also mentioned that we do thoroughly vet the dependencies that Express.js takes on, typically almost all are covered by the Express TC, just because it works out better in the long run this way.

We also touched on the dependency policy Express.js has had, though before closing this issue, I will outline it here for those reading:

• A dependency that is added is thoroughly-read from top-to-bottom for the code it contains, looking for if it feel well-written.
• We look at the test suite of that module, and weigh the complexity of the code to the thoroughness of the test suite, and how often it is run (is it setup in Travis for every commit?).
• We only depend on exact versions of dependencies that are not under the Express TC (formally it was those that are not under the main developers, even personal ones).
• We take into consideration to the dependencies of our dependencies, applying this same logic. A large version range on a dependency of a dependency may disqualify it from being our dependency, due to general risk of stuff randomly breaking.
• The entire "npm unpublish" situation we have been aware of as long as npm has existed with this capability, though I personally have never agreed that that should be a feature of a central code registry. This is not something that can easily be overcome, so unless there is knowledge of some immanent danger/disaster, we will deal with anything along those lines as they happen, not prior.

I hope all that makes sense. If there is a desire to discuss general dependency policies, I welcome someone opening an issue at https://github.com/expressjs/discussions/issues to have a longer discussion.

I'm going to close this issue for now, as I think the discussion is generally over and has been around for 2 days. The topic has started to trail into changing different things, rather than just not depending on various modules just because.

[dougwilson]

As an additional follow-up here, npm has changed the unpublish policy and none of our dependencies can be unpublished without npm, Inc. agreeing that it should be done, as in unpublishing is no longer under the author's complete control.