Closed azhiv closed 1 year ago
Good point. Relying on Node built-ins being shimmed successfully by bundlers when building for browsers does not seem like a bullet-proof solution, as it. It looks like explicitly depending on the NPM packageutil
by browserify is the right solution here. @mroderick or @mantoni?
Related to #2448.
We have util
added as a dependency in referee
and referee-sinon
already. So it's currently implicitly installed through the @sinonjs/referee-sinon
dependency here.
:+1: for adding it as a dependency explicitly here too.
From the webpack build log above it at least seems like implicitly depending on the util
shim does not "work". Unfortunately, after testing it out (adding util
and doing yarn link
to sinon locally and using yarn link sinon
in your repo) neither does explicitly depending on it. And according to the util
docs, neither should it:
From the util
package's own description:
You usually do not have to install util yourself. If your code runs in Node.js, util is built in. If your code runs in the browser, bundlers like browserify or webpack (up to version 4 -- see this documentation for how to include polyfills like util in webpack 5+) also include the util module.
Note the version 5 caveat, as I can see you are using version 5 of webpack. Just having the dependency is not enough, you also need to configure the fallback yourself. I took a pass at figuring out how Ember does this, as I could not see any webpack config in your project, but I only stumbled upon some vegetables (Broccoli), so that is up to you to figure out :)
The docs show how it would normally look (observe the process
field):
So closing this as it seems to be down to user configurable bits. Shout out if I got it wrong!
Having a custom config in the app is fine, what we were trying to say is that sinon needs to specify the exact version of util it depends on. It would justify the need for that special configuration. In the current situation, some random version of util is picked up. In our case, we have a yarn workspace. Adding a new dep (msw.js) to one of the apps in the workspace made it necessary to add a custom config to the other apps because before adding that new dep util was resolved to an older version (0.10), after that it picked up a new version (0.12.1) across the yarn workspace which requires a custom webpack configuration.
What version of util
we depend upon is a bit hard to tell, in a way, as most of the time it is whatever is built-in to Node and that is used by most installs, but I am sure we can stick the external shim package used by bundlers to some explicit version, no problem. Still won't make the issue above go away. If you could try adding the relevant config to your project I could use that as a verification step that it works fine.
I can also just do npm i util@latest
ASAP and publish a new version without testing: whatever floats your boat.
We use ember-auto-import
which is using webpack@5 under the hood to import sinon. We've added the following settings to make it work:
autoImport: {
webpack: {
plugins: [
new webpack.DefinePlugin({
'process.env.NODE_DEBUG': 'false',
}),
],
}
}
I updated the initial repo with a fix, the tests are not failing now.
Describe the bug
util
npm package is referenced from the code but is not claimed to be a dependency as perpackage.json
. This leads to inconsistent behaviour when other modules in the consumer application also depend onutil
. Sinon is not tied to a specific version of the module, which may resolve to other versions depending on the adjacent modules.Why is this even a problem? Starting from v0.12.1
util
contains a top level condition which accessesprocess.env.NODE_DEBUG
which is not compatible with running in a browser. Depending on the bundlerprocess
may be polyfilled, but this is not done by default in Webpack starting from v5.A possible solution would be to add
util
dependency topackage.json
to capture a specific version of the library (v0.12.0 doesn't produce the issue withprocess
).To Reproduce Steps to reproduce the behavior:
yarn
+yarn test:ember -s
Expected behavior Tests are run successfully.
Screenshots
Context (please complete the following information):