Typescript: Sinon stubbing failed after upgrade to 3.9.2

[zorji]

TypeScript Version: 3.9.2, 3.9.3

Search Terms: sinon, stub

Code

// file: ./src/stubber/index.ts
export { toStub } from './to-stub'

// file: ./src/stubber/to-stub.ts
export const toStub = () => {
  console.log('real stub')
}

// file: ./src/start.ts
import * as sinon from 'sinon'
import * as stubber from './stubber'
(async () => {
  sinon.stub(stubber, 'toStub').callsFake(() => {
    console.log('fake stub')
  })
  stubber.toStub()
})()

Run the start.ts script by compiling it with tsc or simply use ts-node.
I'd expect it print fake stub but now it prints real stub. It's working as expected for [email protected] or lower, start breaking from 3.9.2.

Expected behavior:

Print "fake stub"

Actual behavior:

Print "real stub"

Playground Link: Can't provide Playground link because it contains multiple files

Related Issues: Nope

[nico1510]

The same problem exists for jasmine's spys. I tracked the problem down to this change https://github.com/microsoft/TypeScript/pull/32264. After this change get and set properties of modules imported with star syntax are no longer enumerable which then causes a crash in jasmine's check here.
Most likely the same can be said for Sinon and I also saw a comment from someone having this problem with jest here.

Should we tell those libraries to adapt their code or can we have a flag to keep those properties enumerable?

[msholty-fd]

I found this code to break when upgrading from 3.8.2 to 3.9.2. We use ts-jest to transform our TypeScript Jest tests.

import * as foo from 'some/path'
jest.spyOn(foo, 'bar');

I would get an error like:

<spyOn> : bar is not declared writable or has no setter

I resolved this by doing this workaround:

jest.mock('some/path', () => ({
  ...jest.requireActual('some/path'),
}));
const foo = require('some/path');

It's not pretty, but it at least unblocked me from upgrading.

[RyanCavanaugh]

ES module exports are specified to not be enumerable, so this is the expected behavior. Prior behavior was out of compliance with the spec.

[drtyh2o]

Does your comment still apply if we are using "commonjs" modules, @RyanCavanaugh? I noticed the following difference between the emitted modules:

Before

seconds: [Function: seconds],
minutes: [Function: minutes],
hours: [Function: hours],
days: [Function: days]

After

days: [Getter],
hours: [Getter],
minutes: [Getter],
seconds: [Getter]

Both modules work fine, but the latter (3.9) breaks our ability to mock the imported functions with Sinon. We've had to roll back to 3.8 until we can resolve.

[RyanCavanaugh]

If you're targeting commonjs but writing ES module imports/exports, TS is still going to try to give you ES module behavior.

This code is effectively not legal ES, since it's attempting to modify (indirectly via sinon.stub) a readonly property (the properties of something imported with import * as name are not mutable). It should never have worked.

[typescript-bot]

This issue has been marked as 'External' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[squidfunk]

See https://github.com/jasmine/jasmine/issues/1817#issuecomment-630110087 for a temporary workaround.

[adjerbetian]

@RyanCavanaugh Sorry, I didn't understand your answer when you said

If you're targeting commonjs but writing ES module imports/exports

I created a demo project here typescript-3.9-commonjs with a very simple node script.
Do you mean that I should write index.ts differently?

[JordanPawlett]

I found this code to break when upgrading from 3.8.2 to 3.9.2. We use ts-jest to transform our TypeScript Jest tests.

import * as foo from 'some/path'
jest.spyOn(foo, 'bar');

I would get an error like:

<spyOn> : bar is not declared writable or has no setter

I resolved this by doing this workaround:

jest.mock('some/path', () => ({
  ...jest.requireActual('some/path'),
}));
const foo = require('some/path');

It's not pretty, but it at least unblocked me from upgrading.

Just the workaround i was looking for!

[denieler]

Yeah, having the same issue, very annoying 👍

[ahnpnl]

Hi @SimenB not sure if you are aware of this ? I assume once jest supports fully esm this won’t be an issue ?

[SimenB]

Jest module mocks should work regardless, but you probably need to use import() so it's not evaluated statically.

That said, semantics in Jest are probably off for ESM behaviour up until ESM support stabilises