vitest
vitest copied to clipboard
fix(vitest): `vi.hoisted` syntax errors on instrumented code
Description
- Fixes https://github.com/vitest-dev/vitest/issues/6057
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.yamlunless 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 docscommand.
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:, orchore:.
Current implementation:
import { expect, it, vi } from 'vitest'
let coverageCounter = 0
const nestedHoist = (coverageCounter++, vi.hoisted(() => {
return 'Nested hoist'
}))
it('nesting vi.hoisted doesnt cause syntax errors', () => {
expect(nestedHoist).toBe('Nested hoist')
})
becomes:
const { expect, it, vi } = await __vite_ssr_dynamic_import__("/@fs/x/vitest/packages/vitest/dist/index.js")
vi.hoisted(() => {
return 'Nested hoist'
})
let coverageCounter = 0;
const nestedHoist = (coverageCounter++, ); // <-- Syntax error
it("nesting vi.hoisted doesnt cause syntax errors", () => {
expect(nestedHoist).toBe('Nested hoist');
});
I think correct would be
const { expect, it, vi } = await __vite_ssr_dynamic_import__("/@fs/x/vitest/packages/vitest/dist/index.js")
const nestedHoist = vi.hoisted(() => {
return 'Nested hoist'
})
let coverageCounter = 0;
coverageCounter++;
it("nesting vi.hoisted doesnt cause syntax errors", () => {
expect(nestedHoist).toBe('Nested hoist');
});
If implementing this via AST modify becomes too difficult I think we could instead add /* istanbul ignore next */ ignore hints in mocking methods before the instrumentation.
This is getting quite complex. I think better and more maintainable solution is to:
- Prevent
@vitest/coverage-istanbulfrom instrumentingvi.mock|hoisted|etcmethod calls. This is done by adding/* istanbul ignore next */ignore hints before detected method calls. - Throw an error around here if anything else than
VariableDeclarationis found, e.g.SequenceExpression
https://github.com/vitest-dev/vitest/blob/073a50c9740f75ba25ec1fabba73b6dae73e66f4/packages/vitest/src/node/hoistMocks.ts#L330
https://github.com/vitest-dev/vitest/issues/6057#issuecomment-2813403136