Esm: Unable to mock modules with sinon

[nscarcella]

I'm trying to find a way to mock transitive imports on my unit tests and have found many sources that suggest that something like this should work:

import * as Service from './serviceToMock'
import { someFunctionThatCallsMyOperation } from './controllerThatUsesTheService'
sinon.stub(Service, 'myOperation').return(5)
someFunctionThatCallsMyOperation() // Ends up receiving a 5 as answer

Problem is, I'm getting an error that makes me believe Service is frozen: TypeError: Cannot delete property 'myOperation' of [object Module].

Is this actually the case? I'm aware that built-in modules get frozen, but these are my own sources.
Is there any known way or better practice to approach this?

[jdalton]

Hi @nscarcella!

The namespace object created by import * ... is essentially immutable. I could make this mutable but that would be against spec and is avoidable if you expose a export default Service object that can be mocked when imported as import Service from "./serviceToMock".

[nscarcella]

Thanks, @jdalton! That works for me.

[jdalton]

@nscarcella Great! Let me know if anything else comes up!

Update:

I'm considering making the object mutable, in a mock mode option, to better address things like https://github.com/webpack/webpack/issues/6979 and https://github.com/webpack/webpack/pull/7132.

[gmathieu]

@jdalton in response to https://github.com/webpack/webpack/pull/7132#issuecomment-390522301, looking at the esm tests in that PR covers most use cases https://github.com/webpack/webpack/pull/7132/files#diff-fd1c12466f1a82071f9854ef60ea4936R40: mock an import (esm.js) consumed by a file under test (output.js).

I would also add that the mock isn't always a direct import of the file under test. Loaders such as inject-loader make that assumption which is too limiting. I didn't create a test case for this specific scenario but know it works (we've now upgraded to webpack 4 with the custom plugin that makes exports mutable).

[jdalton]

I'm bikeshedding the options.cjs name.
Which do you all prefer _(other suggestions welcomed too)_:

• unsealedNamespace
• mutableNamespace
• writableNamespace

[gmathieu]

I vote for "mutableNamespace"

[Janpot]

@jdalton Would you consider something like loader hooks to make it possible to build this in "user-land"?

[jdalton]

@gmathieu

I'm digging mutableNamespace too!

@Janpot

Module loader hooks aren't fleshed out enough to adopt and will likely be less powerful than current solutions today, so while they may be an option in the future it isn't super clear at the moment.

Since esm is about meeting the ecosystem where it is today and allowing a path forward we'll simply add this to the cjs interop options _(on by default)_. A big theme of esm is that users can get coding fast, and that things they're doing today should "just work".

[Janpot]

Yep, I understand. It's just that sometimes it seems that node can't get to an implementation of ESM because they have to try to fit in the features that transpilers created to "just make things work". (And I'm saying this without putting any blame of sorts, just as an observation)

[jdalton]

It's just that sometimes it seems that node can't get to an implementation of ESM because they have to try to fit in the features that transpilers created to "just make things work".

It's not really a technical problem, it's more a people problem.
Technically you can compile esm into Node today and have working out-of-the-box ESM.

[Janpot]

yes, that's what I meant, "can't get to a consensus"

[jdalton]

Sacrificing the developer experience, as webpack has done in this instance, isn't the right call. It's not up to the ecosystem to make it easy for platform implementors. The ecosystem meets developer needs head on and it's up to the platform to move more closer to that. For webpack all it does is open them up to competition, from folks like parceljs, who will fill the dev experience gap.

[Janpot]

As far as I know, webpack hasn't taken an official stance on this. All I did was question whether non-standard functionality belongs in the core of the tool, or rather in a plugin (if possible). Somebody provided a plugin solution and the issue got closed by the author.
Personally, I think a plugin solution is actually a better developer experience. The flag you create is in the best case a temporary feature (as suggested in that discussion) and in the worst case a flag you have to support forever. The plugin system is already a permanent feature of the platform.

[jdalton]

As far as I know, webpack hasn't taken an official stance on this.

The instance I was referring to was the broken mocking scenario in webpack 4. In this case the stance is inferred by not addressing the issue.

All I did was question whether non-standard functionality belongs in the core of the tool, or rather in a plugin (if possible).

In this tool, esm, yes it belongs. Non-standard functionality is our starting point :yum:

Somebody provided a plugin solution and the issue got closed by the author.

Workarounds buried in issues aren't great _(btw here's my take on it)_. For this scenario plugins are fine but they should be documented and maybe even pulled into webpack-contrib.

The flag you create is in the best case a temporary feature (as suggested in that discussion) and in the worst case a flag you have to support forever.

Nothing is forever. A flag would exist until a platform solution comes along or until webbpack dropped it in a major release.

[Janpot]

Non-standard functionality is our starting point

Well, let me adjust then to align more with the spirit of what I mean: "non-standard with no intention to make it a standard, as in, will this feature be specced somewhere"

Workarounds buried in issues aren't great

agreed

btw here's my take on it

I think this is a lot cleaner than the solution in the ticket

For this scenario plugins are fine but they should be documented and maybe even pulled into webpack-contrib.

agreed

Nothing is forever.

Indeed, nothing is forever, but it's there until somebody removes it. With having and removing it comes a maintenance burden, both for the tool maintainer as well as the user. With a third party plugin you take this burden away from the tool maintainer and place it on a plugin maintainer.

Tbh, take my line of thinking as half philosophical. I understand very well that people want to/have to "get things done".

[jdalton]

Well, let me adjust then to align more with the spirit of what I mean: "non-standard with no intention to make it a standard, as in, will this feature be specced somewhere"

The ecosystem today, regardless of whether parts are-or-are-not likely to be standardized, is our starting point :yum:

With having and removing it comes a maintenance burden,

In this case the burden is minimal _(for esm at least)_.

Tbh, take my line of thinking as half philosophical. I understand very well that people want to/have to "get things done".

For me the _"get things done"_ has more weight. I've watched folks talk theory/philosophy for years and deliver :doughnut: for users.

[jdalton]

@gmathieu @nscarcella

Patched https://github.com/standard-things/esm/commit/6fc556c7288a8c55946868c371c1ea5511600373, https://github.com/standard-things/esm/commit/4599635a810b5a5815e41cc36a75a7dcf0e4d35a, https://github.com/standard-things/esm/commit/d642e0007a9d69647ac563cee872eb255ec0387a.

Update:

esm v3.0.37 and Sinon v5.0.8 have been released :tada:

This is now possible:

import sinon from 'sinon'
import assert from 'assert'
import * as obj from './index'

assert.equal(obj.foo(), 1)

const spy = sinon.spy(obj, 'foo')

obj.foo(42)
obj.foo(1)

console.log(obj.foo.isSinonProxy) // true
console.log(spy.withArgs(42).calledOnce) // true
console.log(spy.withArgs(1).calledOnce) // true

[jdalton]

_Note:_ Just locking the thread because it started to become a catch-all for Sinon questions. The hope is that by locking this issue it will encourage those with questions to open new separate issues.