node-semver: [BUG] Webpack builds are failing as new release 7.5.0 requiers node.js util

I find it ironic that a package called ‘semver’ has introduced a breaking change in a minor release.

@okdistribute to be clear, it’s not a breaking change for its target envs - bundlers aren’t that - but also, it works fine with non-broken bundlers.

It broke my build on an npm install with a previously working package lock. I now have to pin semver to 7.4.0 so it will not get updated. I understand it wasn’t your intention to break the package, but the impact is that 7.5.0 is breaking builds without warning

@tjenkinson it’s webpack 5 that doesn’t support browsers, since “supporting browsers” assumes automatic polyfilling of core modules and globals, which is what bundlers have been doing for over a decade.

IMO if previously this library used to work in a browser without any node dependencies, the benefit of the slightly clearer error message is not worth breaking that, and maybe supporting browsers is something this package could officially support?

Hi @wraithgar, I wanted to share my thoughts on this. Personally, I think it would be best to revert the changes. As it stands, there doesn’t seem to be any other spec compliant semver package available on npm, at least not one that shows up when searching for “semver”. I strongly believe that semver should remain a library that compares version strings using “ECMAScript compliant JS”. If this lib would be using node libraries to make these comparisons more performant it would be a different story, but the marginal dx gain from using node’s util inspect for better error output doesn’t seem worth it.

Given that this repo is called “node-semver”, I can certainly understand if it were to move in this direction but then we should consider renaming the npm package to “node-semver” as well. Otherwise it’s possible that we’ll see a fork emerge soon that removes that one line. I think this package is quite popular in non-node environments like React Native, as well as many other “update handling” clients (SPAs). It would be a shame if the efforts of maintaining a version comparing library were now split up.

If we want to revert this, it needs to be done in a way that also sets up some sort of linting or other testing so that “does not use x internals from node” is part of the contract, which it wasn’t before.

Opened a PR here that does that if we go with that approach: https://github.com/npm/node-semver/pull/559

I’m removing the “Needs Discussion” flag from this issue, the ongoing comments do not feel productive. Can we please keep the comments on topic, namely:

If we want to revert this, it needs to be done in a way that also sets up some sort of linting or other testing so that “does not use x internals from node” is part of the contract, which it wasn’t before.

@rodperottoni yes, as of v5, it is a broken bundler: https://web.archive.org/web/20201011110014/https://blog.sindresorhus.com/webpack-5-headache-b6ac24973bf1?gi=8ecd10aab21d

This is just webpack 5 being broken - a working node module bundler automatically polyfills node core modules whenever possible. I don’t think it’s part of semver to maintain compatibility with broken tools.

Depending on Node for something that can run in all JS environments is a no-go, and this module strikes me as something that at least originally was meant to be universally compatible.

Hmm I have a different understanding then. if I installed a package that was meant to support browsers I wouldn’t expect it to reference any node built-in dependencies

So what defines a bundler as broken / functional is its ability to polyfill NodeJS APIs? That just sounds like your opinion, and I wholeheartedly disagree with it. It’s a shame newer versions of semver cant be used by projects using Webpack or Metro anymore.

I expect that such essential packages are tried to be as self contained as possible. Npm ecosystem already suffers from problem of huge dependency trees

So this is essentially meaning we need another dependency to be bundled with our applications

This appears to be the way webpack works, yes.

So this is essentially meaning we need another dependency to be bundled with our applications to show an error message for when someone happens to feed complete garbage to semver? Most people don’t even really know how to configure Webpack to do that when Webpack just happens to be what their application is built with (such as Angular).

That seems unreasonable and quite useless.

On top of that, that modification was completely out of scope of that pull request.

It does look like webpack itself has made the decision to make polyfills the responsibility of the consumer, and when they’re encountered it’s up to them to add where needed.

Bringing in a net new dependency for this error message seems like the wrong choice. I think this is down to “how webpack works” and “we can’t add internals that need polyfilling” is not a contract node utils should need to support.