Bad typing for toHaveBeenCalledOnceWith causing problems

[Xunnamius]

Bug

• package version: 3.1.0
• node version: 18.10.0
• npm (or yarn) version: 8.19.2

What happened:

A test with the following line passes, but TypeScript complains with Expected 0 arguments, but got 1. ts(2554):

expect(revealSpy).toHaveBeenCalledOnceWith({ nodes: [node], index, parent });

reveal and revealSpy are defined as follows:

const revealSpy = jest.spyOn(mdastUtilHidden, 'reveal').mockImplementation(() => undefined);

// Exported as part of mdastUtilHidden
function reveal<Nodes extends Hidden[]>({
  nodes,
  index,
  parent
}: {
  nodes: Nodes;
  index: number;
  parent: Parent<Node<Data>>;
}) {
  parent.children.splice(
    index,
    1,
    ...nodes.flatMap((n) => n.hiddenChildren.map((n) => removePosition(n)))
  );
}

I saw a recent commit described as "fixing toHaveBeenCalledOnceWith," so this may be a regression. Potentially related to #517. I've used @ts-expect-error to ignore the TS error for now. EDIT: expect(...).toHaveBeenNthCalledWith(1, ...) also works as poor man's substitute. Thanks for your work on this package!

Reproduction repository: https://github.com/Xunnamius/unified-utils/blob/b84a05ab0a10dd38b2c46062452f6c3e6cec194a/packages/mdast-util-hidden/test/unit-index.test.ts#L215

[loyaj]

I'm seeing the same problem.

I just started using jest-extended, specifically because I wanted toHaveBeenCalledOnceWith. It is offputting to get hints in my IDE like Invalid number of arguments, expected 0 when using this matcher.

I agree, it seems like this matcher was added recently and needs additional implementation

[keeganwitt]

Yea, this was a simple goof in the type file. We currently lack test coverage of our types, but this is something we plan on adding.

[konstantinmv]

Why has this been closed? Can we close the issue when this would be solved (with the new version, I suppose?)

[WtfJoke]

Any plans for releasing this fix @keeganwitt ?

We would like to have used toHaveBeenCalledOnceWith but decided not to, as we would need to put @ts-ignore currently.

[keeganwitt]

I don't have permission to publish new versions and it doesn't look like we have our ci.yaml setup to automate the publishing. @SimenB can we look into doing another release?

[keeganwitt]

Simen just did a 3.2.0 release that contains this fix.

[WtfJoke]

Wow, that was fast 🚀
Thank you @SimenB and @keeganwitt ❤️

[Maxim-Mazurok]

@keeganwitt I don't think this was properly addressed, now I'm getting Expected 1 arguments, but got 3.ts(2554) for expect(someMockFunction).toHaveBeenCalledOnceWith("some_file", 1, 2);

I believe that toHaveBeenCalledOnceWith<E extends any[]>(...params: E): R; (inspired by toHaveBeenNthCalledWith definition) should do the job.

Should we reopen this or create a new issue?

[keeganwitt]

@keeganwitt I don't think this was properly addressed, now I'm getting Expected 1 arguments, but got 3.ts(2554) for expect(someMockFunction).toHaveBeenCalledOnceWith("some_file", 1, 2);

I believe that toHaveBeenCalledOnceWith<E extends any[]>(...params: E): R; (inspired by toHaveBeenNthCalledWith definition) should do the job.

Should we reopen this or create a new issue?

I believe the fix we shipped makes the type match the current implementation. Only the first argument is actually compared.

const mock = jest.fn();
mock('hello', 'there');
expect(mock).toHaveBeenCalledOnceWith('hello', 'not there');

Passes even though it'd be preferable that it not. Being able to do this is what issue #517 is about.

[Maxim-Mazurok]

Yeah, good point, types change should be part of the fix for #517 in this case, thank you!