nodejs / tooling

Advancing Node.js as a framework for writing great tools
171 stars 15 forks source link

Include SemVer in Node.js #146

Open bnb opened 2 years ago

bnb commented 2 years ago

It would (imo) be valid to say that SemVer a fundamental and essential part of the ecosystem. All modules rely on it, Node.js itself uses it for versioning, and it's currently sitting at 193m downloads last week, and 187m downloads the week prior - with a growth of 6m downloads in a week.

During both of the tooling sessions at the collaborator's summit, we discussed vendoring semver. There seemed to be general consensus, but I wanted to create an issue to begin the discussion.

Discussion points:

I'm sure there's a few more points here, but wanted to get this down so we can begin to have it.

ruyadorno commented 2 years ago

To add to the vendor scope/idea: Since the original conversation about including semver into Node.js during the collaborator summit, a thread was spawned in the node repo (ref: https://github.com/nodejs/node/issues/43413) about the idea of vendoring dependencies but I personally find that conversation got derailed due to the suggestion of opening the door to potentially adding any arbitrary package from the ecosystem, I believe if we're to start again that conversation it should be under the prism of only vendoring libraries that are already under the Node.js project (such as undici) and only consider including semver once it gets transferred over to Node.js.

ruyadorno commented 2 years ago

A different idea from these originally pointed out in this thread that @darcyclarke brought up in today's meeting is to potentially try to see if there's interest in adding some level of semver support to the language specification itself, potentially spawning a TC39 study group (not sure on this terminology) as a better place to collaborate.

ljharb commented 2 years ago

I suspect that would be a tough sell unless there’s very compelling universal (browser, IoT) use cases I’m unaware of.

lirantal commented 2 years ago

Joining @ruyadorno's concern, if I understood correctly, that vendoring ecosystem dependencies would easily open the door to malicious packages in a nested tree to slip in. However, there's value in having common capabilities like semver vendored in and be semi-part of the runtime if so to say and I am wondering if the following idea might make a good compromise - both undici and semver are 0 dependencies (semver technically has 1 but authored by same person), and both are part of trusted individuals in the ecosystem. And so with that intro, could we make the case that whichever dependencies we vendor have to uphold to such specific thresholds? for example:

styfle commented 2 years ago

semver technically has 1 [dependency] but authored by same person

Would that dependency (lru-cache) also be exposed or just semver?

ruyadorno commented 2 years ago

Would that dependency (lru-cache) also be exposed or just semver?

Taking a quick look at the code it seems that lru-cache is only used in a single place in order to memoize a hot path, with that in mind it doesn't sound unfeasible to replace that with any memoize/cache implementation that core is already using or alternatively, look into also transferring that package over to the nodejs org.

That's a good point @styfle, this is an item that would need to be taken into consideration if we're to include semver in node core, regardless of how it happens (vendor, util, etc).

darcyclarke commented 2 years ago

The quick facts/data on node-semver's adoption:

Compared to the Ecosystem

Screen Shot 2022-08-18 at 12 29 20 PM ref. https://npmtrends.com/lodash-vs-react-vs-semver

Compared to the npm CLI Team's Supported Projects

Screen Shot 2022-08-18 at 12 33 28 PM

As shown above, node-semver represents a disproportionate amount of the installs we see & is only growing. When compared to other popular packages (ex. react or lodash) the package dependants to install ratio is extremely low which is a good indicator that the usage is being driven by end-users/consumers (not maintainers).

Name Dependants Downloads (weekly)
semver ~22k ~199.5 million
lodash ~120k ~45.5 million
react ~90k ~15.5 million

I know I was the one to originally suggest potentially vendoring &/or including the capabilities of node-semver into core (at OpenJS World back in June) but I've taken a step back since & realized this likely needs to be a bigger initiative (I also just am not a fan of the vendoring concept in-general, but appreciate the work @jasnell & others did to investigate that fully). I'm proposing we bring SemVer & the capabilities of node-semver module to TC39 &/or W3C.

Beyond the CLI & cache dependency (which can be easily bundled or removed), there's nothing really unique or overtly tied to Node about node-semver.

Unfortunately, there are nuances to node-semver which deviate from the canonical SemVer "standard" (which is why you'll see many folks, including myself reference the project as "node-semver" even though the package name its distributed under is "semver").

The SemVer org, unfortunately, has stagnated & not kept pace with the innovations in package/version management. Other language ecosystems (ie. Ruby, Python etc.) have also chosen to deviate from the "standard" which has created a fragmented understanding of what is & isn't valid in regards to SemVer Ranges & Comparators. That said, these differences should not prevent us from trying to unify the understanding within the JavaScript community itself & we should scope the unification effort to this language to avoid undue scope creep.

As an easy first step, we can & will abstract the core library to be independent of dependencies & runtime-agnostic; immediately allowing other ecosystems to share our understanding of versioning (ex. bun, deno & the front-end). Notably, there is prior art here with somewhat significant adoption (ref. https://www.npmjs.com/package/@storybook/semver & https://www.npmjs.com/package/compare-versions).

A rough breakdown of how I imagine the next 12-24 months in regards to this overarching initiative:

This is obviously ambitious & I'm certain things will change along the way here but hopefully this initial outline gets some folks interested/excited.

ljharb commented 2 years ago

I don’t see how it would fit with W3C, since it has zero correlation with the web - as for TC39, I’d expect a lot of pushback for something node-specific (since browsers wouldn’t likely want to implement something that has minimal use on the web).

I agree that vendoring is not an ideal solution, but i don’t understand why node, and only node, isn’t the place to house this package and/or implementation.

bakkot commented 2 years ago

I would put it a slightly different way: in my estimation, browsers are very unlikely to be interested in shipping semver to their users. So even if W3C or TC39 were interested in standardizing it, it would be dead letter. I could be wrong about this, but you should probably check before going any further down this path.

I do think there's value in having this API widely available and standardized across server-side runtimes, like node/deno/bun - especially if there's buy-in from other languages and ecosystems - but the mentioned standards bodies are unlikely to want to be responsible for a standard which is not particularly useful in browsers. (Unless there is some reasonably common use case in browsers I'm missing?)

This perhaps speaks to the need for a different body where server-side APIs like this could be standardized. Or you could just write up a spec and conformance tests and see if you can get Deno and Bun to buy into it, rather than trying to spin up an actual standards org.

styfle commented 2 years ago

Probably not W3C or TC39 but perhaps https://wintercg.org would be interested in standardizing

jasnell commented 2 years ago

It's something that can be discussed but unless there's an API proposal to standardize around that is implemented by multiple runtimes it's going to be difficult to make progress

bnb commented 2 years ago

I am also -1 on TC39/W3C proposal. I don't currently see a reason why it'd be useful in the context of The Web in a general sense. I'd potentially be +1 on it going into WinterCG as a standard, but I'd want that standard to be 1:1 with what node-semver currently does which AFAIK is slightly different from the semver spec.

ruyadorno commented 2 years ago

heads up, there's currently a PR open for it: https://github.com/nodejs/node/pull/45127

jasnell commented 2 years ago

To be clear, the pr only adds semver as an internal dependency. It does not expose the API to users in any way.

isaacs commented 2 years ago

My comment on the challenges of standardizing version ranges: https://github.com/semver/semver/pull/584#issuecomment-1125354556

tl;dr - That PR documents node-semver's understanding of version range specifier semantics, which is demonstrably a pretty effective approach that has worked well for the JavaScript community. But it will likely never be (and perhaps should never be) a standard across all SemVer implementations in all communities.

The tests in node-semver are fairly extensive, and as a zero-deps module, it should be relatively trivial to pull into node. But if there's any question about intent or de facto behavior that can't be answered by the code (or if there's some value in just having the spec available in documentation), then that PR can be useful.

Personally, I think a node:semver module in core (or even in the JavaScript standard library) makes a ton of sense. It's effectively "done". It's had over a decade of play testing, and any changes are disruptive enough to be ill advised anyway, so can pretty much just be cast in amber and left alone. There's no other implementation in JavaScript that gets any considerable usage, so there's little risk of squashing competition.

isaacs commented 2 years ago

I'd want that standard to be 1:1 with what node-semver currently does which AFAIK is slightly different from the semver spec.

The SemVer specification doesn't actually specify much in terms of parser behavior, and doesn't specify anything with how version ranges are to be handled. It focuses entirely on the correctness of form itself, and author/consumer semantics implied.

node-semver only deviates from the standard in that it has a "loose mode" which allows some things the SemVer spec doesn't, mainly for backwards compatibility with earlier versions of the spec. By default, it has rejected anything that isn't compliant SemVer 2.0 for many years now, but there's some very old stuff on the npm registry that predates SemVer 2.0, so npm uses loose mode judiciously and follows Postrell's law in those cases.

LeaVerou commented 1 year ago

FWIW I would see quite a lot of value in having a semver API available in client-side JS as well. Peer dependencies are a common case: often your module gets an optional dependency passed in at runtime through a method, and it needs to inspect its version (many libraries include their version number in their bundle, like Library.version) and make decisions at runtime.

bnb commented 1 year ago

what are the current blockers on this? Is it a +1 from the TSC? Is it a "yes this is fine" from the npm team?

isaacs commented 1 year ago

If this is happening, I think it'd be a good idea to finish the perf improvement work in progress first. As the original author, I'm in favor of seeing it in node core, but I'm not on the tsc or the npm team any longer, so it's up to them 🙂

bnb commented 1 year ago

perhaps a @nodejs/tsc ping is in order for next steps?

I'm totally fine with the perf improvement work being a prerequisite - I know @jasnell's son (trying to find his handle and struggling, I'm sorry to not ping directly!) was doing some of that and apparently seeing success.

cjihrig commented 1 year ago

Here is @flakey5's PR: https://github.com/nodejs/node/pull/45127

I'm a huge +1 to porting semver to core, but would be -1 to vendoring the module from npm and exposing it directly.

ljharb commented 1 year ago

@cjihrig can you elaborate on why (not expose the vendored module)?

cjihrig commented 1 year ago

The biggest things are consistency around errors and primordials if the code is user facing. I'm just guessing and have not looked at the code, but there may be room for cleaning up other things if they exist (support for older Node versions, etc.). Since the module is essentially done, it doesn't seem like we would be stuck in a position of porting many future changes.

ljharb commented 1 year ago

Ah, those both make sense - a node wrapper around the vendored module would cover errors, but not primordials.

I would be more than happy to author a primordials-using JS port (or to pair with @flakey5 if they're interested) once it's decided which parts of the API are desired.

jasnell commented 1 year ago

Just fyi @flakey5 and I are both on vacation this week. I've mentioned to him that this conversation has come back around. His original PR vendoring it in is still open. When we are back at home I'm sure he'll take a look

flakey5 commented 1 year ago

Apologizes for late response, been a bit busy.

I would be more than happy to author a primordials-using JS port (or to pair with @flakey5 if they're interested) once it's decided which parts of the API are desired.

I'd definitely be interested in helping out with this.

ljharb commented 1 year ago

Wonderful! Once we get some direction about the API - a list of things to include, or exclude, i suppose - then I'll reach out and we can set up some pairing time.

flakey5 commented 1 year ago

Sounds good!

isaacs commented 1 year ago

Perhaps relevant to this, I've started on a userland port of lib/internals/per_context/primordials.js, with the intention of making glob/minimatch suitable for inclusion in core. I'm not sure yet how badly it'll impact performance, but it could be worth playing with. If the userland node-semver is a drop-in replacement with just a single line changed for core's internal primordialized version, I figure that could be a lot easier to pull patches in the future.

It's brand new, so extra eyeballs on it would be great: https://github.com/isaacs/node-primordials

mbtools commented 5 days ago

I removed the lru-cache dependency in 7.6.1 😀

Meanwhile, semver is clocking 414 million downloads per week 🤯 Can someone use an AI to calculate the carbon footprint of that?

It's insane NOT to include it in node. Let's do it!