Dependency graph improvements (via npmgraph.js.org)

[broofa]

[Probably more of a tracking issue here than an actual issue in and of itself. I'm putting this up mostly because I'm curious if the express team has considered any of these concerns than out of any real expectation of action.]

See https://npmgraph.js.org/?q=express.

Under "Modules with Multiple Versions":

• http-errors@2.0.0 is pinned to statuses@2.0.1, while the rest of the graph depends on statuses@2.0.2. Having the same clause appear in the dependencies section of the involved modules would avoid unnecessary duplication here.

Under "Suggested Replacements" (these come from the [e18e module-replacements project](https://github.com/es-tooling/module-replacements project)

• 6 of the suggestions - es-define-property, es-errors, function-bind, gopd, has-symbols, hasown - are part of the get-intrinsics module... all of which are suggested to be replaced with modern native equivalents. (This begs the question, "How much value is get-intrinsics really adding? attn @ljharb)
• body-parser - I'm skeptical of this one. module-replacements suggests replacing with inline code or milliparsecs, but I'm guessing express has more stringent requirements that make this non-actionable? Might be worth getting someone from e18e to comment on this. attn @43081j
• inherits - module-replacements suggests using native class syntax? Seems reasonable, given that ES class syntax has been widely available for a long time now.
• qs - This is pulled in by body-parser so may come for free if there's a viable alterantive to that library. module replacement suggests using native URLSearchParams, or other (mostly ESM-friendly) alternatives. Again might be worth input from e18e, attn @43081j

[43081j]

• 6 of the suggestions - es-define-property, es-errors, function-bind, gopd, has-symbols, hasown are part of the get-intrinsics module... all of which are suggested to be replaced with modern native equivalents

this entire subtree comes from qs. its a combination of stuff to prevent global namespace mutation, and stuff to support older node. express doesn't need any of it anymore, but qs does afiak. there's discussion in #5783 about dropping qs since the default in latest major of express is to use the built in parser anyway

body-parser - I'm skeptical of this one. module-replacements suggests replacing with inline code or milliparsecs

i think you're right and express should keep this as is. it'd be too big of a breaking change to move away from body-parser. if the qs discussion results in having no opinionated query string parser, body-parser would shrink a lot.

basically express did a lot of good work in the last major that improved all sorts of stuff. the last remaining thing seems to be dropping qs and most of these points will be solved

[bjohansebas]

http-errors@2.0.0 is pinned to statuses@2.0.1, while the rest of the graph depends on statuses@2.0.2. Having the same clause appear in the dependencies section of the involved modules would avoid unnecessary duplication here.

We already have a PR to fix this and it has been merged, so it will be resolved in the next version of this module. jshttp/http-errors#126

body-parser - I'm skeptical of this one. module-replacements suggests replacing with inline code or milliparsecs, but I'm guessing express has more stringent requirements that make this non-actionable? Might be worth getting someone from e18e to comment on this. attn @43081j

-1, if there are improvements to be made, PRs are more than welcome, but replacing it with another module that isn’t maintained by us doesn’t sound very appealing to me.

However, I would like to talk with the teams behind some of this package’s dependencies, like raw-body and iconv-lite, as it seems they don’t have a clear future roadmap and they are dependencies of this package.

inherits - module-replacements suggests using native class syntax? Seems reasonable, given that ES class syntax has been widely available for a long time now.

Probably, we could try using classes, I don’t see anything wrong with that. However, I think we should wait until the performance team can measure the impact of these changes.

[ljharb]

The purpose of get-intrinsics (and most of those packages) is robustness in the face of environment modification (they already use native equivalents internally when available), and module-replacements chooses not to care about that concern, which means that unless express also doesn't care about it, module-replacements isn't appropriate to apply to express.

[43081j]

worth noting that it only happens as a result of pulling in qs

the rest of express doesn't have this concern, already. if a maintainer can come along and say they do have this concern, they should rework the rest of express to use these utilities, since that's not how it currently works.