Add isSealed check to is-es-module.

[jdalton]

This PR adds an Object.isSealed check to the isESModule helper. This allows objects that may have the toStringTag of Module to be mocked as long as they aren't sealed (ES namespace objects are sealed).

[fatso83]

Thank you, @jdalton! That seems like a very reasonable addition in these times of change towards ESM. Would you mind adding a minimal test to show its utility? I think you should find what you need in the test/es2015 directory

[jdalton]

Thanks @fatso83. Will do!

Update:

Test added.

[fatso83]

It looks fine and all, but I was wondering if you could try and explain to me (slowly 🐢) the actual utility of adding this (meaning: an actual scenario where this would be used)? After seeing the test, I was a bit uncertain of what I was actually looking at, as the form of the module being tested seemed somewhat contrived. I am assuming that the reason this PR comes up is that you came across some actual use case that the current check prevented, so I am just curious to what this is fixing/enabling :)

Normally, the only objects that "have the toStringTag of Module" are ES Modules. The only reason another object that is not an ES Module would fulfill this is by posing as an ES Module. So mocking something that is already posing as something else seems a bit contrived, hence my curiosity.

[jdalton]

@fatso83

It looks fine and all, but I was wondering if you could try and explain to me (slowly 🐢) the actual utility of adding this

Sure thing. In my case I write esm, the successor to @std/esm which Sinon uses for its ESM tests. It came to my attention that many folks have been using Sinon to spy on namespace objects from Babel and other transpiled output. Since folks found it useful I'm adding an options.cjs.mutableNamespace option to the esm loader. As part of that the namespace objects provided by esm will still have a toStringTag of "Module" but will not be sealed (so they will be mutable). In testing the enhancement I noticed Sinon's check for ES namespace objects was too loose and wasn't working on the unsealed simulated form which was what brought on this PR.

With the new esm option and this PR this is doable with the esm loader:

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

[fatso83]

Perfect! That makes total sense, as it revolves around tooling, and not "your mama's development" :-) Thanks for the details.

[fatso83]

5.0.8 out

[mroderick]

@jdalton thank you!

If I understand this correctly, this solves the problem of stubbing things on modules. If that's the case, then we should write something about this for the how to section, so we can point to that for when people show up with questions about modules.

[jdalton]

@mroderick

If I understand this correctly, this solves the problem of stubbing things on modules.

This doesn't solve stubbing on built-in module namespace objects. The esm loader has a new options.cjs.mutableNamespace option (on by default) that allows Sinon to work though. Folks wanting to stub/spy for modules can use esm to enable it for tests.

[mroderick]

@jdalton that was what I meant to say, when using esm, we can now mutate modules using sinon. (It's still early here)

[jdalton]

Yep! Check it out 🎉