0

I'm converting some old JS code to TS, and the tests all use mocha/sinon with stubbing of functions using old require style imports without destructuring, and also rely on the application code to do the same.

So when the application code does

import {someFunc} from 'mymodule'

and the tests do

const mymodule = require('mymodule');
sinon.stub(mymodule, 'someFunc', ...)

... then someFunc won't be stubbed by the application since it was bound at compile time due to the destructuring. I think every developer trying to modernize old JS code has run into this problem. :(

Now, as hesitant as I am to using old-school import * as mymodule in my application code and call functions in that module using mymodule.someFunc(), I can sort of live with that to get the tests working. BUT, here comes the caveat - there are also some cases where modules use other functions in the same module which are stubbed by tests:

// mymodule.ts
export function someFunc() {
  return someOtherFunc();
}

export function someOtherFunc() {
  return 42
}

// mymodule.test.js
const mymodule = require('mymodule');
sinon.stub(mymodule, 'someOtherFunc');

The tests will fail to stub someOtherFunc since it's already bound at compile time when someFunc calls it.

Here, a solution I've found to work is that the module uses exports.someOtherFunc() to call a function in the same module, and while this is even uglier, a more severe problem is that exports is typed as any, losing all type safety.

I know that the proper solution probably is to refactor the entire application to have modules return objects, classes or similar (which enables stubbing properties within these objects) instead of separate functions - but I don't really have time to rewrite the entire application. Is there some solution that at least maintains type safety when calling sibling functions in the same module via the exports object? The only other solution I've come up with is to split modules into smaller pieces so that no function calls another function in the same module which needs to be stubbed for tests, but this will also be a substantial piece of refactoring.

JHH
  • 8,567
  • 8
  • 47
  • 91
  • Did you look into migrating from mocha ? I don't think your problem exists with jest – Axnyff Apr 26 '23 at 19:53
  • 2
    @Axnyff Jest has exactly the same “problem” - https://stackoverflow.com/q/45111198/3001761. It’s just JS, they’re not accessing each other via the module object. The solution is already here: _don’t_ mock functions within the same module from one another. That’s an implementation detail, either ignore it (just test `someFunc` returns `42`) or move it outside the module boundary. – jonrsharpe Apr 26 '23 at 19:59
  • 1
    @Axnyff It exists regardless of framework, the problem is that stubbing a function object after it has been read has no effect. – JHH Apr 26 '23 at 20:31

1 Answers1

0

This is one solution I came up with that maintains type safety, exports only the desired functions in a single object, and enables stubbing of the exported functions.

// mymodule.ts
function baz() {
  // local unstubbable function
  return 'baz'
}

export default {
  // stubbable function
  foo() {
    return 'foo';
  },

  // stubbable function
  bar() {
    return this.foo() + baz();
  }
}

// mymodule.test.ts
import { stub } from 'sinon';
import mymodule from './mymodule'

console.log(module.bar());

const barStub = stub(module, 'bar').returns('stubbed');
console.log(module.bar());

barStub.restore();
console.log(module.bar());

It feels slightly cleaner, relying on this rather than exports, and also disables accidental destructured imports such as import {foo} from 'mymodule'; as the only export is the default export.

A potential downside is that local functions cannot call functions inside the default export, but if that's needed, the default export could be stored in an object as well:

function baz() {
  // not stubbable
  return api.foo();
}

const api = {
  // stubbable function
  foo() {
    return 'foo';
  },

  // stubbable function
  bar() {
    return this.foo() + baz();
  }
}

export default api;

If needed, export default api could be replaced with export = api if the legacy JS code uses const module = require('./module') and esModuleInterOp and/or allowSyntheticDefaultImports doesn't work.

JHH
  • 8,567
  • 8
  • 47
  • 91