RFC: stubbing fonction expressions in ESM modules

[forty]

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

• I don't think the same trick can work for function declaration. we luckily don't use them in our code base, but even for people who do, I think the migration is nicer if you can only update the declaration sites (and convert to function expressions your mockable functions) without touching all the call sites
• maybe we could use decorators for that mockable helper, I have not looked into it, but it does seem to be a good use case
• I don't think that mockable should be in sinon. First having it in the user code base allow to skip it in production (by detecting the test framework in my case) and second, this is a run time dependency rather than a dev deps as sinon would generally be.

Let me know what you think.

[forty]

Related issue: #1832

[mroderick]

It looks like there is some progress being made in Node's test runner: https://github.com/nodejs/node/pull/52848/files#diff-7ef19622f4f1c831ca0e6ed0edc03cb517f16969f1f1b0972a117bd3fa65c44aR1963-R1990.

Is that a viable path forwards for you?

[mantoni]

This might also be a pattern that works for you and is already supported: #2538

[fatso83]

The pattern using sinon.replace.usingAccessor(obj,prop,val) is quite a bit wordier than @forty's suggestion. Both approaches are explicit, at least.

I don't think this custom approach should be part of Sinon. After all, you can create a utility wrapper a la this:

// I have not run this, probably has some runtime issue or typo
addMockableSupport(sandbox) {

  // see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy/Proxy/get
  const handler = {
    get: function (target, prop, receiver) {
      if (prop === 'stub') {
        return function stubWithMockableSupport(obj,prop,val){       
          if(obj[prop]['__mockHandle']) { 
            sandbox.stub(obj[prop], '__mockHandle', val)
          } else {
            sandbox.stub(obj, prop, val)
          }
        }
      }
      return Reflect.get(...arguments);
    }    
  }

  return new Proxy(sandbox, handler);
}

Using that, you can now

mySinon = addMockableSupport(sinon)

// should now be able to stub out "mockables", that have been treated with the `mockable(f)` function above
mySinon.stub(someEsmModule, fooMockable)