[Bug]: jest-mock/spyOn property name check is "truthy" instead of "defined" or "existent"

[octaharon]

Version

29.5

Steps to reproduce

Minimal Typescript example with real-world use case

enum WhyNot {
    A,
    B,
    C
}

const DynamicDispatchObject:Record<WhyNot,CallableFunction>={
    [WhyNot.A]:()=>{},
    [WhyNot.B]:()=>{},
    [WhyNot.C]:()=>{}
}

// Throws with 'No property name supplied', cause WhyNot.A === 0
const spiedFunction = jest.spyOn(DynamicDispatchObject, WhyNot.A);

// Typescript fails due to object 'keyof' type inference
const spiedFunction = jest.spyOn(DynamicDispatchObject, String(WhyNot.A));

Without Typescript it's still an issue, though the pattern is not that common in human-written code

const DynamicDispatchObject={
0: console.log,
1: console.info
}

// throws with 'No property name supplied'
jest.spyOn(DynamicDispatchObject, 0); 

// WORKS
jest.spyOn(DynamicDispatchObject, "0"); 

Expected behavior

I expect any existing object property (that is a Function) should be available for mocking and spying

0 is perfectly valid object key, and can be used interchangeably with "0" due to javascript native type coercion, so I'd expect mocking out methods, referenced by those keys, should be possible. In typescript I'd expect it to work too, without any typecasting or //@ts-ignore, due to the nature of static type checking. That is fixed by using string Enums or starting them explicitly with 1, but that might not be the desired behaviour in some cases.

Let alone I could as well use negative numbers as keys, which would also fail to work with spyOn

Actual behavior

Pretty expectedly and non-positive integer keys in objects can't be mocked due to the way the property name check is done:

 if (!methodKey) {
      throw new Error('No property name supplied');
    }

which is too broad of a restriction, and I'd further argue that the check like this is even redundant, 'cause few lines below there's a dereference with a sort of property existence check, which could be done in the first place

    const original = object[methodKey];

    if (!this.isMockFunction(original)) {
      if (typeof original !== 'function') {

Additional context

No response

Environment

System:
    OS: Windows 10 10.0.22621
    CPU: (20) x64 12th Gen Intel(R) Core(TM) i7-12700H
  Binaries:
    Node: 18.12.1 - C:\Program Files\nodejs\node.EXE
    npm: 9.3.1 - C:\Program Files\nodejs\npm.CMD
  npmPackages:
    jest: ^29.3.1 => 29.5.0

[mrazauskas]

Just open a PR to fix this. I guess the check should be:

if (methodKey == null) {
  throw new Error('No property name supplied');
}

[octaharon]

Just open a PR to fix this. I guess the check should be:

if (methodKey == null) {
  throw new Error('No property name supplied');
}

that would actually not fix the issue but would rather fail the check when empty string is passed as method name

of many options to exclude all numbers that are converted to string "0" (like 0, -0.0 , 0x0 and so on) from falsy check I've chosen to go with Number.isFinite which doesn't affect NaN and BigInt keys (I hope there's no good reason to use these as indices)

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.