fix(spy)!: reset existing spy on repeated spyOn

[hi-ogawa]

Description

• Closes Feature Request: Create spyReset to only reset spies, not other mocks #7348

Is this a breaking change again? 🤔

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
• Ideally, include a test that fails without this PR but passes with it.
• Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Tests

• Run the tests with pnpm test:ci.

Documentation

• If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

• Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

[sheremet-va]

Why do we need to restore it? I would expect we can just reset the state

[hi-ogawa]

mockReset would be weird. I pushed it to show how tests fail. The previous issue #6419 will come back at least.

[hi-ogawa]

Or probably 346794b is what you meant?

[NateRadebaugh]

Latest looks like it should do the trick. I'd be happy to test this once it's released.

[pkg-pr-new]

@vitest/browser

npm i https://pkg.pr.new/@vitest/browser@7359

@vitest/coverage-istanbul

npm i https://pkg.pr.new/@vitest/coverage-istanbul@7359

@vitest/coverage-v8

npm i https://pkg.pr.new/@vitest/coverage-v8@7359

@vitest/expect

npm i https://pkg.pr.new/@vitest/expect@7359

@vitest/mocker

npm i https://pkg.pr.new/@vitest/mocker@7359

@vitest/pretty-format

npm i https://pkg.pr.new/@vitest/pretty-format@7359

@vitest/runner

npm i https://pkg.pr.new/@vitest/runner@7359

@vitest/snapshot

npm i https://pkg.pr.new/@vitest/snapshot@7359

@vitest/spy

npm i https://pkg.pr.new/@vitest/spy@7359

@vitest/ui

npm i https://pkg.pr.new/@vitest/ui@7359

@vitest/utils

npm i https://pkg.pr.new/@vitest/utils@7359

vite-node

npm i https://pkg.pr.new/vite-node@7359

vitest

npm i https://pkg.pr.new/vitest@7359

@vitest/web-worker

npm i https://pkg.pr.new/@vitest/web-worker@7359

@vitest/ws-client

npm i https://pkg.pr.new/@vitest/ws-client@7359

commit: e566197

[hi-ogawa]

@NateRadebaugh Feel free to try the package on pkg-pr-new #7359 (comment)

[netlify]

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	e566197
🔍 Latest deploy log	https://app.netlify.com/sites/vitest-dev/deploys/67abdd7c4a96860008a2f8f8
😎 Deploy Preview	https://deploy-preview-7359--vitest-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[dskloetd]

So creating a new spy will reset the old spy?
I want to be able to define mock behavior in beforeEach and then spy on the function in individual tests if I need to check if the function was called. But this shouldn't remove the mock behavior unless I explicitly reset it.

[hi-ogawa]

@dskloetd Can you try out the package to see if it works for you? #7359 (comment) If it doesn't, can you provide a reproduction?

[dskloetd]

I wasn't sure how to try the package from the branch so I just added the currentStub.mockReset(); line in my node_modules/@vitest/spy/dist/index.js.

Here's an example of something that breaks as a result:

    const obj = {
      foo: () => 1,
    };

    vi.spyOn(obj, "foo").mockReturnValue(2);
    vi.spyOn(obj, "foo").mockReturnValueOnce(3);
    expect(obj.foo()).toBe(3);
    expect(obj.foo()).toBe(2);

Imagine that in realistic tests the default return value is defined in beforeEach and the one-off in the test. So not on lines next to each other, which can just be reversed in order.

Here's another example of something that breaks:

    vi.spyOn(obj, "foo").mockReturnValue(2);

    const spy = vi.spyOn(obj, "foo");
    spy.mockClear();  // <-- the became necessary recently
    expect(obj.foo()).toBe(2);  // <-- this breaks with the current PR
    expect(spy).toBeCalledTimes(1);

[NateRadebaugh]

@dskloetd thanks for testing this. I also ran into issues with the latest once I ran it against my codebase but I haven't found time to investigate myself. I think your use case is more complete than I had originally reported.

[hi-ogawa]

Thanks for testing it out. The use case sounds valid (namely calling vi.spy(obj, "foo") multiple times with the intention of stacking up many spies). The original issue #6419 (which brought the change #6464) is technically not about vi.spyOn itself, but about the behavior in relate to vi.restoreAllMocks, so maybe we should solve this from a different angle.

[NateRadebaugh]

Thanks, the new PR looks even more targeted solution of the original bug and should resolve the new issues we're seeing.

[hi-ogawa]

@dskloetd @NateRadebaugh Hey folks, if anyone interested, please try it out #7499 (comment). This brings back Vitest v2 behavior of vi.spyOn and fixes only vi.restore/reset/clearAllMocks() API.

[NateRadebaugh]

Thanks yeah I've already reviewed the other PR and it seems to fit my needs, effectively reverting the breaking change and restoring my tests to passing.

In the meantime I've used patch-package to unblock my team's upgrade, looking forward to an official release with it, thanks!

[hi-ogawa]

Superseded by #7499