sinonjs / sinon

Test spies, stubs and mocks for JavaScript.
https://sinonjs.org/
Other
9.67k stars 769 forks source link

RFC: stubbing fonction expressions in ESM modules #2621

Open forty opened 1 month ago

forty commented 1 month ago

Is your feature request related to a problem? Please describe.

We have a large nodejs code base full of fonction expressions such as

export const myFunction = () => {
   return true
}

it was all nice, and we were able to mock such functions with sinon just fine. But, all nice things have an end, and we now have to migrate to ESM (no specific needs except that third party modules start dropping support for commonjs) and of course we have to deal with ES Modules cannot be stubbed.

We could move everything to top level objects or default export, but then we would have to change lots of code (as we need to modify all the imports and the call sites) and most annoyingly, we need to somehow make sure that all the call to the function are done using the reference in the top level object,ie

const myFunction = () => {
   return true
}
const myFunction2 = () => {
   return myFunction()
}
export const MyModule { myFunction, myFunction2 };

is no good, as stubbing MyModule.myFunction will not work for myFunction2.

Describe the solution you'd like

I figured out a solution that solve my problem in a rather idiomatic JS way (which means it's both horrible and nice) I defined a small helper function, which adds a property __mockHandle to a function, which contains the actual function, and is used in the body of the function:

export const mockable = (f) => {
    // in actual code, I detect our test framework and returns the function as is when not in test
    // to avoid any perf penalty and messing stack traces in production
    const fWithHandle = Object.assign(
        (...args) => {
            return fWithHandle.__mockHandle(...args);
        },
        {
            __mockHandle: f
        }
    );
    return fWithHandle;
};

this can then be used that way

export const myFunction = mockable(() => {
   //
})

and in tests

import * as MyModule from './mymodule';
sinon.stub(MyModule.myFunction, '__mockHandle').returns(false);

This seems to work fine, and I'm happy with the solution. What I'm wondering if it would be interesting to add some simple support in sinon for this pattern to allow implicitly use __mockHandle (actual name tbd) when calling the stub function with a single parameter to allow doing:

sinon.stub(MyModule.myFunction).returns(false);

I think it would not be a lot of code, is not very intrusive and it would make this pattern a bit nicer to use.

Describe alternatives you've considered

Already mentioned above.

Additional context

Some extra thoughts

Let me know what you think.

forty commented 1 month ago

Related issue: https://github.com/sinonjs/sinon/issues/1832