Skip to content

Comments

feat: Explicit Resource Management support in mocked functions#7927

Merged
sheremet-va merged 6 commits intovitest-dev:mainfrom
EskiMojo14:dispose
May 17, 2025
Merged

feat: Explicit Resource Management support in mocked functions#7927
sheremet-va merged 6 commits intovitest-dev:mainfrom
EskiMojo14:dispose

Conversation

@EskiMojo14
Copy link
Contributor

@EskiMojo14 EskiMojo14 commented May 3, 2025

Description

Support explicit resource management with mocked functions, meaning mockRestore can be called automatically once scope is exited.

using consoleSpy = vi.spyOn(console, "spy")
// is like
const consoleSpy = vi.spyOn(console, "spy")
try {
  // ...
} finally {
  consoleSpy.mockRestore()
}

This is very convenient, but also good for Jest parity - Jest added it here jestjs/jest#14895 and released in v30.0.0-alpha.3.

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:.

@netlify
Copy link

netlify bot commented May 3, 2025 •
edited
Loading

✅ Deploy Preview for vitest-dev ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 0423d08
🔍 Latest deploy log https://app.netlify.com/sites/vitest-dev/deploys/681e2750b6d49f0008cad5f7
😎 Deploy Preview https://deploy-preview-7927--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

hi-ogawa
hi-ogawa reviewed May 4, 2025
View reviewed changes
Copy link
Contributor

@hi-ogawa hi-ogawa left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think this is nice to have. Vitest should already support using at least on typescript out of the box since esbuild transpiles it #4409, so you can probably add a test with using like

vitest/test/core/test/esnext.test.ts

Lines 15 to 24 in 30b709c

it('new "using" feature', () => {
let getResource = (): any => {
throw new Error('don\'t call me')
}
{
using resource = resourceful('foo')
getResource = () => resource.resource
}
expect(getResource()).toBe(null)
})

@hi-ogawa hi-ogawa added the p2-nice-to-have Not breaking anything but nice to have (priority) label May 4, 2025
@hi-ogawa hi-ogawa moved this to P2 - 2 in Team Board May 4, 2025
@sheremet-va
Copy link
Member

sheremet-va commented May 9, 2025

This looks good to me, but seems like tests are failing (ignore the browser ones)

@EskiMojo14
Copy link
Contributor Author

EskiMojo14 commented May 9, 2025 •
edited
Loading

I don't really understand why tests are failing - is there a way to get a more useful failure message?

never mind - I see that the core tests are failing as well

@EskiMojo14
Copy link
Contributor Author

EskiMojo14 commented May 9, 2025 •
edited
Loading

(non-browser) tests are fixed - the dispose was holding onto the function reference instead of using the key we can spy on.

@sheremet-va sheremet-va merged commit b67d307 into vitest-dev:main May 17, 2025
11 of 13 checks passed
@renovate renovate bot mentioned this pull request Sep 24, 2025
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hi-ogawa hi-ogawa hi-ogawa left review comments

@sheremet-va sheremet-va sheremet-va approved these changes

Assignees

No one assigned

Labels

p2-nice-to-have Not breaking anything but nice to have (priority)

Projects

Team Board
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@EskiMojo14 @sheremet-va @hi-ogawa