Closed assertnotnull closed 1 year ago
Hi and thanks for making a good reproduction case! Great to have a simple reproduction repository.
Sinon makes mocks, stubs and spies. Those are plain, normal functions, and we do not do any magic to the runtime, so whatever is to be done needs to be done using normal Javascript in the given runtime. Sinon clearly states what the issue is:
Descriptor for property toBeMocked is non-configurable and non-writable
If the transpiled code restricts everyone from modifying those exports, Sinon cannot do anything by itself. This issue is not an issue with Sinon, but with your transpiler tooling.
While ts-node clearly exports a writeable object descriptor, SWC does not. This is in-line with how ES Modules are supposed to work, so SWC seems to be the correct behavior here.
Just googling "swc writable object descriptor" comes up with a few suggestions, some ok workarounds. I would still try another approach, if you just need to do this to have mutable namespaces for your modules, and that is to use the Dalton's ESM loader. It has an option to make namespaces mutable and you can register it before your SWC module.
The option in ESM:
"mutableNamespace":true
I've tried the suggestion in stackoverflow as I also found this before but it's not working either. Other libraries complexifies the tests too much that it outweighs the benefits. I guess running tests with mocks is not an option with SWC.
You can use mocks in the original sense, but nowadays mocking is more colloquially used as a term for intercepting loading of dependencies and replacing those with some test double (stub, fake, mock, dummy). That was easier when "everyone" was using derivatives of the CommonJS module system, either in Node or using some bundler like Browserify or Webpack. This was because the importing of other modules happened at runtime, so you could if/else
different dependencies at runtime or hook into the module loading like proxyquire
and similar did. This changes in 2015 when ES Modules were introduced, as they are resolved earlier and cannot be programmatically replaced like we used to, except if you do stuff to the runtime like Jest is doing; essentially transpiling everything to require
calls . This is why we essentially had issues like this from 2015 (see #831, for instance).
Once you are in true ESM land (not transpiled to ES5 and/or CJS), you cannot mutate exports in the normal runtime. For that to be allowed, you need to hook into the process earlier, by replacing the module loader. This is actually been allowed in Node for ages (Node 13?) and used by various tools. One alternative to Sinon is TestDouble, which has most of the basic functionality of Sinon, but it also bundles its own module loader for use with ESM! You can either go all in on that, or just use the module loading.
Here's the link for documentation on how to use this (it's quite painless): td.replaceEsm() You basically just need to supply an extra flag for node to use TD's module loader, and it should work. TD has very few dependencies, but one of them is Quibble, which is actually the library responsible for doing the module replacement. So you could alternatively just use that directly:
node --loader=quibble ...
# or mocha --loader=quibble ...
then
await quibble.esm('./a-module.mjs', {life: 41}, 'replacement universe');
Check the docs.
P.S. By now, I actually cannot remember if using SWC results actual ES Modules (which have immutable namespace) or something else (seeing the object descriptor thingies above. Sorry. I have the memory of a goldfish), so this might or might not work for your 🤷 If it turns out the resulting transpiled code is not ESM you can just use proxyquire
(Sinon guide).
Pure dependency injection (previously called "poor man's DI") is something I often reach for to avoid complicating my setup. You get a slight change in production code to enable better tests. You can see my describing a fuller example of this in https://github.com/sinonjs/sinon/issues/831#issuecomment-198081263
Issues related to ESM stubbing: #831, #1623, #1711, Issues related to CJS stubbing: #1248, #1648
P.S. I have some extra time today, so thought I might as well try getting this working. Testing out Quibble now.
Working fine! I forked your repo and got it working: https://github.com/fatso83/sinon-swc-bug/tree/cjs-mocking
Be aware, that this is relying on CommonJS modules. At first, I configured Quibble to do ESM replacements, but I was not able to get it working. The problem was that this was never ESM to begin with. Your .swcrc
file explicitly specifies that the module system should be CommonJS. Furthermore, I found that the defaults for the SWC configuration targets ES5. Once I figured that I was really just working with CJS it took just a minute to get it working:
it("should mock", () => {
const mocked = sandbox.fake.returns("mocked");
quibble("./other.ts", { toBeMocked: mocked });
const {main} = require("./main");
main();
expect(mocked.called).to.be.true;
});
P.S. This is the first time I have used Quibble (TestDouble). You could just as well have used Proxyquire, which is what we suggest in our how-to for replacing modules.
Another approach is of course to just modify the output of SWC to make the exports mutable (writable object descriptors). The jest-workaround
plugin for SWC does just this, so I opened https://github.com/magic-akari/jest_workaround/issues/65 to ask if it was possible to re-use this in other test runners. That would make it re-usable in Ava, Mocha, etc.
The issue with non-mutable exports is the same for Webpack and other transpilers, for that matter.
I looked into how to configure a TS project into using an ESM (not-CommonJS module transpilation) based approach and ran into one problem: when configuring TypeScript to produce ES Modules, you need to specify file extensions in your imports (not a problem). You also need to make Node aware how the ts
extension is to be handled when loading modules, as you will otherwise get TypeError: Unknown file extension ".ts" for /Users/myuser/code/sinon-swc-bug/src/main.spec.ts
. That is usually handled by setting --loader=ts-node/esm
, but you also need Quibble to be used as the loader for module replacement to work! While there is a project in Node dedicated to add support for using multiple loaders, this is not complete, so you cannot use two loaders today.
As you can currently only specify a single loader, you would then need to implement a custom loader that did the juggling of making the output of { load, resolve }
in one loader work together with the same ones from another loader. I stumbled over this example, which could be used as a starting point. I do not intend to do that right now 😄
So if using SWC, I would configure your TS project to use CJS as the module target for the time being (which is the default!), if possible and just use the approach I showed above (in the cjs-mocking
branch).
If you do not need TypeScript and just need to generally stub ESM, I put a working ESM example in the esm-no-ts branch.
@assertnotnull Did you see the fix/workaround I provided for you? Was just wondering if that worked fine, as I was thinking of updating the old link seams article with examples for modern bundlers/transpilers, as this comes up quite a bit.
I was also thinking that it might be useful to have error messages point directly to relevant docs? So instead of "Descriptor for property toBeMocked is non-configurable and non-writable" change it to something like
The descriptor for the property
toBeMocked
is non-configurable and non-writable. Sinon cannot do anything about this, as it is essentially immutable. Read https://sinonjs.org/faqs#immutable-objectdescriptor for tips on what could cause this and how to possibly resolve it.
(The link is made up by the way).
OK, I have found yet another option that works: the SWC plugin that is confusingly called jest_workaround
, which makes the exports mutable. Super easy. See my example:
https://github.com/fatso83/sinon-swc-bug/tree/swc-with-mutable-exports
The issue is that the exports of import * as Other from './other'
are getters, not fields, but since these getters are actually configurable
(due to the plugin), I just instructed Sinon to replace the getter.
There is also another option I am working on.
In any case, #2534 needs to fixed as well, though it's not blocking (as you can manually restore the accessor stubs).
I will try to gather all of these options in an article on the Sinon documentation page, if time be willing.
Thank you @fatso83 for spending much time looking into this. I find this is quite complicated to use different loaders, specially if such change was applied to a big project but the last solution is interesting. I will give it a try.
Good to hear from you! I did write that article in the end, btw, but no-one has commented and reviewed my PR. I should contain all the details one could need. Would you have a chance at reviewing it to see if this looks useful for people?
It's #2540. I attached/linked the article as PDF for easy reading
The changes https://github.com/fatso83/sinon-swc-bug/tree/swc-with-mutable-exports works!
I only had to modify the definition of mocks in the beforeEach
and add the plugin!
I must add, this is in a Nestjs project where Jest has been replaced with Mocha + Sinon. We faced memory leaks with Jest in the past.
Describe the bug Sinon is unable to mock the function when compiled with SWC but works with ts-node
To Reproduce clone the sample repo with bug
Outcome:
To see it working with ts-node replace "@swc/register" with "ts-node/register" Then run pnpm test again and it passes.
Expected behavior It should mock