Repository navigation
[BUG] Webpack builds are failing as new release 7.5.0 requiers node.js util #554
Description
Is there an existing issue for this?
- I have searched the existing issues
Current Behavior
A webpack build is failing with the following error:
./node_modules/xxx/node_modules/semver/classes/semver.js:30:46-69 - Error: Module not found: Error: Can't resolve 'util' in '/runner/_work/xxx/node_modules/xxx/node_modules/semver/classes'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need this module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add a fallback 'resolve.fallback: { "util": require.resolve("util/") }'
- install 'util'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.fallback: { "util": false }
It was working in version 7.4.0
Expected Behavior
The webpack build should work without adding any node.js dependency as we want to use semver in the browser.
Steps To Reproduce
- Setup a webpack build
- add semver
- Run webpack
- See error above
I assume it was introduced due to this change:
v7.4.0...v7.5.0#diff-7917f8ef42ff71df5bde71c259d15feab8b147e3e6c2fde11b56cbf42cadea16R19
Environment
- npm: 6.14.17
- Node: 14.9.3
- OS: Ubuntu 20.04
- platform: Linux
Reacted by Waseem Abbas, Aurore Bonnin, JenniferStamm, David Lechner, Tristan Bastian, Jan, Ignatius Reza, Roel van Bakel, Vincent Lycoops, jianai and 19 more
Reacted by silverwind
Activity
wraithgar commentedon Apr 18, 2023
This is obviously one of those "soft" areas of semver. We have an
enginesdeclaration and test in all the versions laid out there. Other environments are technically outside the support matrix.Given the presence of tools like https://github.com/browserify/node-util specifically for things like this I wonder if fixing one's webpack setup is appropriate.
janhommes commentedon Apr 18, 2023
You might be right if you say node-semver does not support web apps. On the other hand, webpack is quite a common bundler, and a lot of people seem to use semver on the web(as you can see, in less than one day, already 4 other people seem to have run into this issue). At the same time, the breaking change is only a developer experience improvement, nothing really of high importance. Maybe there is even an easy workaround with
JSON.stringifyto fulfill both goals (not breaking, and logging the correct error)? Not sure, not a node expert.wraithgar commentedon Apr 18, 2023
I might be wrong too, this is a good issue that got opened and I'd rather have a short discussion instead of acting in haste.
If we decide to fix this, it would be good also to decide there is a basic support level in webpack et al for this lib and somehow enforce it in CI. I don't know what that looks like though.
This is definitely the right fix if we want to remove
util.ljharb commentedon Apr 18, 2023
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.
wraithgar commentedon Apr 18, 2023
JSON.stringify is not a good replacement for
util.inspectljharb commentedon Apr 18, 2023
@wraithgar you could certainly use https://npmjs.com/object-inspect if you like :-)
wraithgar commentedon Apr 18, 2023
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.
ljharb commentedon Apr 18, 2023
I completely agree.
wraithgar commentedon Apr 18, 2023
https://github.com/inspect-js/object-inspect/blob/main/util.inspect.js ?
ljharb commentedon Apr 18, 2023
@wraithgar that's skipped in bundlers via https://github.com/inspect-js/object-inspect/blob/main/package.json#L79-L81 - the sole purpose is to get at node's custom util.inspect Symbol when available.
wartab commentedon Apr 19, 2023
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.
13 remaining items
ljharb commentedon Apr 26, 2023
@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.
okdistribute commentedon Apr 26, 2023
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
rodperottoni commentedon Apr 26, 2023
Webpack is a broken bundler?
Right... 😂
ljharb commentedon Apr 27, 2023
@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
rodperottoni commentedon Apr 27, 2023
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
semvercant be used by projects using Webpack or Metro anymore.wraithgar commentedon Apr 27, 2023
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.
DomiR commentedon May 5, 2023
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.
tjenkinson commentedon May 5, 2023
Opened a PR here that does that if we go with that approach: #559
wraithgar commentedon May 5, 2023
Thank you @tjenkinson this is what was needed.