Can't stub the module (object) function in the latest update

[eugenet8k]

Describe the bug After https://github.com/sinonjs/sinon/pull/2508 was merged and the new sinon v15.0.4 is published a lot of code in our project stopped working. We can an error when trying to stub the object function in a special case. The same approach work in v15.0.2

The example code that shows the problem based on attempt to stub ChaplinJS utils object.

import Chaplin from "chaplin";
import sinon from "sinon";
import routes from "./routes";

new Chaplin.Application({ routes });

console.log("before stub", Chaplin.utils.redirectTo({ url: "/test/" }));

const sandbox = sinon.createSandbox();
sandbox.stub(Chaplin.utils, "redirectTo").callsFake(() => "stubbed");

console.log("after stub", Chaplin.utils.redirectTo({ url: "/test/" }));

To Reproduce Open code sandbox based on v15.0.4 and see console log https://codesandbox.io/p/sandbox/sinon-module-stub-broken-15-0-4-9e1f4u

Console:

before stub true
bootstrap:27 Uncaught TypeError: Cannot redefine property: redirectTo
    at Function.defineProperty (<anonymous>)
    at wrapMethod (sinon-esm.js:4607:16)
    at Function.stub (sinon-esm.js:3803:44)
    at Sandbox.stub (sinon-esm.js:3247:39)
    at ./src/index.js (index.js:10:9)
    at __webpack_require__ (bootstrap:24:1)
    at startup:6:1
    at startup:6:1

Expected behavior Open code sandbox based on v15.0.2 and see console log https://codesandbox.io/p/sandbox/sinon-module-stub-working-15-0-2-tipumo

Console:

before stub true
index.js:12 after stub stubbed

[fatso83]

Yup, seeing the descriptor is non-configurable, so need to look into the previous version to understand how this could have worked before.

[fatso83]

Found the issue. Previously we the object descriptor was pretty much left unchanged (apart from the value prop), but leaving configurable: false on a stub would make it impossible to remove, so this was added in 15.0.4 to make it possible to create stubs that would shadow a method on the prototype.

Unfortunately, it introduced a regression for cases where the object descriptor was not from the prototype, but the object itself. If configurable is false then it cannot be changed back into something else, which is why it throws. Will add a regression test and fix asap with a check for isOwn before modifying the descriptor.

[fatso83]

The sandboxes were fine, but I was unable to get them running locally as unit tests as they depended on window existing. If you want to test out the fix in #2515 and see that it fixes your issue, you can just have a look at the diff and copy it manually into the right section (the line it fails) in node_modules/sinon/pkg/sinon-esm.js:

-        methodDesc.configurable = true;
+
+        // you are not allowed to flip the configurable prop on an
+        // existing descriptor to anything but false (#2514)
+        if (!owned) {
+            methodDesc.configurable = true;
+        }
+

[eugenet8k]

@fatso83 yes, the fix works well for our case, thank you!

[fatso83]

Out as Sinon 15.1.0 (minor due to also getting the new clock.jump() method)