diff --git a/packages/mocker/src/node/esmWalker.ts b/packages/mocker/src/node/esmWalker.ts index 41420995a03c..279c070e6d9e 100644 --- a/packages/mocker/src/node/esmWalker.ts +++ b/packages/mocker/src/node/esmWalker.ts @@ -5,6 +5,7 @@ import type { Identifier, ImportExpression, Literal, + MetaProperty, Pattern, Property, VariableDeclaration, @@ -43,7 +44,7 @@ interface Visitors { info: IdentifierInfo, parentStack: Node[], ) => void - onImportMeta?: (node: Node) => void + onImportMeta?: (node: Positioned) => void onDynamicImport?: (node: Positioned) => void onCallExpression?: (node: Positioned) => void } @@ -142,7 +143,7 @@ export function esmWalker( } if (node.type === 'MetaProperty' && node.meta.name === 'import') { - onImportMeta?.(node as Node) + onImportMeta?.(node as Positioned) } else if (node.type === 'ImportExpression') { onDynamicImport?.(node as Positioned) diff --git a/packages/mocker/src/node/hoistMocksPlugin.ts b/packages/mocker/src/node/hoistMocksPlugin.ts index e62f69a9172e..ab77f92e604a 100644 --- a/packages/mocker/src/node/hoistMocksPlugin.ts +++ b/packages/mocker/src/node/hoistMocksPlugin.ts @@ -237,9 +237,9 @@ export function hoistMocks( } const declaredConst = new Set() - const hoistedNodes: Positioned< + const hoistedNodes: Set[] = [] + >> = new Set() function createSyntaxError(node: Positioned, message: string) { const _error = new SyntaxError(message) @@ -304,8 +304,15 @@ export function hoistMocks( } const usedUtilityExports = new Set() + let hasImportMetaVitest = false esmWalker(ast, { + onImportMeta(node) { + const property = code.slice(node.end, node.end + 7) // '.vitest'.length + if (property === '.vitest') { + hasImportMetaVitest = true + } + }, onIdentifier(id, info, parentStack) { const binding = idToImportMap.get(id.name) if (!binding) { @@ -382,7 +389,7 @@ export function hoistMocks( ) } } - hoistedNodes.push(node) + hoistedNodes.add(node) } // vi.doMock(import('./path')) -> vi.doMock('./path') // vi.doMock(await import('./path')) -> vi.doMock('./path') @@ -420,7 +427,7 @@ export function hoistMocks( 'Cannot export hoisted variable. You can control hoisting behavior by placing the import from this file first.', ) // hoist "const variable = vi.hoisted(() => {})" - hoistedNodes.push(declarationNode) + hoistedNodes.add(declarationNode) } else { const awaitedExpression = findNodeAround( @@ -430,7 +437,7 @@ export function hoistMocks( )?.node as Positioned | undefined // hoist "await vi.hoisted(async () => {})" or "vi.hoisted(() => {})" const moveNode = awaitedExpression?.argument === node ? awaitedExpression : node - hoistedNodes.push(moveNode) + hoistedNodes.add(moveNode) } } } @@ -444,7 +451,11 @@ export function hoistMocks( && isIdentifier(callee.property) && isIdentifier(callee.object) ) { - return `${callee.object.name}.${callee.property.name}()` + const argument = node.arguments[0] as Positioned + const argStr = argument.type === 'Literal' || argument.type === 'ImportExpression' + ? code.slice(argument.start, argument.end) + : '' + return `${callee.object.name}.${callee.property.name}(${argStr})` } return '"hoisted method"' } @@ -481,10 +492,11 @@ export function hoistMocks( } // validate hoistedNodes doesn't have nodes inside other nodes - for (let i = 0; i < hoistedNodes.length; i++) { - const node = hoistedNodes[i] - for (let j = i + 1; j < hoistedNodes.length; j++) { - const otherNode = hoistedNodes[j] + const arrayNodes = Array.from(hoistedNodes) + for (let i = 0; i < arrayNodes.length; i++) { + const node = arrayNodes[i] + for (let j = i + 1; j < arrayNodes.length; j++) { + const otherNode = arrayNodes[j] if (node.start >= otherNode.start && node.end <= otherNode.end) { throw createError(otherNode, node) @@ -495,8 +507,29 @@ export function hoistMocks( } } + // validate that hoisted nodes are defined on the top level + // ignore `import.meta.vitest` because it needs to be inside an IfStatement + // and it can be used anywhere in the code (inside methods too) + if (!hasImportMetaVitest) { + for (const node of ast.body as Node[]) { + hoistedNodes.delete(node as any) + if (node.type === 'ExpressionStatement') { + hoistedNodes.delete(node.expression as any) + } + } + + for (const invalidNode of hoistedNodes) { + console.warn( + `Warning: A ${getNodeName(getNodeCall(invalidNode))} call in "${id}" is not at the top level of the module. ` + + `Although it appears nested, it will be hoisted and executed before any tests run. ` + + `Move it to the top level to reflect its actual execution order. This will become an error in a future version.\n` + + `See: https://vitest.dev/guide/mocking/modules#how-it-works`, + ) + } + } + // hoist vi.mock/vi.hoisted - for (const node of hoistedNodes) { + for (const node of arrayNodes) { const end = getNodeTail(code, node) // don't hoist into itself if it's already at the top if (hoistIndex === end || hoistIndex === node.start) { @@ -530,7 +563,7 @@ export function hoistMocks( } } - if (!hoistedModuleImported && hoistedNodes.length) { + if (!hoistedModuleImported && arrayNodes.length > 0) { const utilityImports = [...usedUtilityExports] // "vi" or "vitest" is imported from a module other than "vitest" if (utilityImports.some(name => idToImportMap.has(name))) { diff --git a/test/core/test/__snapshots__/injector-mock.test.ts.snap b/test/core/test/__snapshots__/injector-mock.test.ts.snap index fb1477b26ad5..55ff2a1b4ddd 100644 --- a/test/core/test/__snapshots__/injector-mock.test.ts.snap +++ b/test/core/test/__snapshots__/injector-mock.test.ts.snap @@ -1,6 +1,6 @@ // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html -exports[`throws an error when nodes are incompatible > correctly throws an error if awaited assigned vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock(): both methods are hoisted to the top of the file and not actually called inside each other."`; +exports[`throws an error when nodes are incompatible > correctly throws an error if awaited assigned vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock('./mocked'): both methods are hoisted to the top of the file and not actually called inside each other."`; exports[`throws an error when nodes are incompatible > correctly throws an error if awaited assigned vi.hoisted is called inside vi.mock 2`] = ` " 2| @@ -11,7 +11,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error 6| " `; -exports[`throws an error when nodes are incompatible > correctly throws an error if awaited vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock(): both methods are hoisted to the top of the file and not actually called inside each other."`; +exports[`throws an error when nodes are incompatible > correctly throws an error if awaited vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock('./mocked'): both methods are hoisted to the top of the file and not actually called inside each other."`; exports[`throws an error when nodes are incompatible > correctly throws an error if awaited vi.hoisted is called inside vi.mock 2`] = ` " 2| @@ -44,7 +44,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error 5| })" `; -exports[`throws an error when nodes are incompatible > correctly throws an error if vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock(): both methods are hoisted to the top of the file and not actually called inside each other."`; +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.hoisted is called inside vi.mock 1`] = `"Cannot call vi.hoisted() inside vi.mock('./mocked'): both methods are hoisted to the top of the file and not actually called inside each other."`; exports[`throws an error when nodes are incompatible > correctly throws an error if vi.hoisted is called inside vi.mock 2`] = ` " 2| @@ -77,7 +77,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error 5| })" `; -exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock inside vi.hoisted 1`] = `"Cannot call vi.mock() inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`; +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock inside vi.hoisted 1`] = `"Cannot call vi.mock('./mocked') inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`; exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock inside vi.hoisted 2`] = ` " 2| @@ -88,7 +88,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error 6| " `; -exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned awaited vi.hoisted 1`] = `"Cannot call vi.mock() inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`; +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned awaited vi.hoisted 1`] = `"Cannot call vi.mock('./mocked') inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`; exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned awaited vi.hoisted 2`] = ` " 2| @@ -99,7 +99,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error 6| " `; -exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned vi.hoisted 1`] = `"Cannot call vi.mock() inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`; +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned vi.hoisted 1`] = `"Cannot call vi.mock('./mocked') inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`; exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside assigned vi.hoisted 2`] = ` " 2| @@ -110,7 +110,7 @@ exports[`throws an error when nodes are incompatible > correctly throws an error 6| " `; -exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside awaited vi.hoisted 1`] = `"Cannot call vi.mock() inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`; +exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside awaited vi.hoisted 1`] = `"Cannot call vi.mock('./mocked') inside vi.hoisted(): both methods are hoisted to the top of the file and not actually called inside each other."`; exports[`throws an error when nodes are incompatible > correctly throws an error if vi.mock is called inside awaited vi.hoisted 2`] = ` " 2| diff --git a/test/core/test/injector-mock.test.ts b/test/core/test/injector-mock.test.ts index 8e0f93831b20..aab1197b3c59 100644 --- a/test/core/test/injector-mock.test.ts +++ b/test/core/test/injector-mock.test.ts @@ -1,7 +1,7 @@ import type { HoistMocksPluginOptions } from '../../../packages/mocker/src/node/hoistMocksPlugin' import { stripVTControlCharacters } from 'node:util' import { parseAst } from 'vite' -import { describe, expect, it, test } from 'vitest' +import { describe, expect, it, test, vi } from 'vitest' import { hoistMocks } from '../../../packages/mocker/src/node/hoistMocksPlugin' import { generateCodeFrame } from '../../../packages/vitest/src/node/printError.js' @@ -1508,4 +1508,147 @@ export const mocked = vi.unmock('./mocked') expect(error.message).toMatchSnapshot() expect(stripVTControlCharacters(error.frame)).toMatchSnapshot() }) + + it('shows an error when hoisted methods are used outside the top level scope', ({ onTestFinished }) => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + onTestFinished(() => warn.mockRestore()) + const result = hoistSimpleCode(` +// correct +vi.mock('./hello-world-1') + +if (condition) { + vi.mock('./hello-world-2') +} + +test('some test', () => { + vi.mock('./hello-world-3') +}) + +test('some test', () => { + if (condition) { + vi.mock('./hello-world-4') + vi.hoisted(() => {}) + vi.mock(import('./hello-world-5')) + const variable = vi.hoisted(() => {}) + } +}) + +describe('some suite', () => { + if (condition) { + vi.mock('./hello-world-6') + } +}) + `) + expect(result).toMatchInlineSnapshot(` + "import { vi } from "vitest" + vi.mock('./hello-world-1') + vi.mock('./hello-world-2') + vi.mock('./hello-world-3') + vi.mock('./hello-world-4') + vi.hoisted(() => {}) + vi.mock('./hello-world-5') + const variable = vi.hoisted(() => {}) + vi.mock('./hello-world-6') + + // correct + + if (condition) { + } + + test('some test', () => { + }) + + test('some test', () => { + if (condition) { + } + }) + + describe('some suite', () => { + if (condition) { + } + })" + `) + expect(warn).toMatchInlineSnapshot(` + [MockFunction warn] { + "calls": [ + [ + "Warning: A vi.mock('./hello-world-2') call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version. + See: https://vitest.dev/guide/mocking/modules#how-it-works", + ], + [ + "Warning: A vi.mock('./hello-world-3') call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version. + See: https://vitest.dev/guide/mocking/modules#how-it-works", + ], + [ + "Warning: A vi.mock('./hello-world-4') call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version. + See: https://vitest.dev/guide/mocking/modules#how-it-works", + ], + [ + "Warning: A vi.hoisted() call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version. + See: https://vitest.dev/guide/mocking/modules#how-it-works", + ], + [ + "Warning: A vi.mock(import('./hello-world-5')) call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version. + See: https://vitest.dev/guide/mocking/modules#how-it-works", + ], + [ + "Warning: A vi.hoisted() call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version. + See: https://vitest.dev/guide/mocking/modules#how-it-works", + ], + [ + "Warning: A vi.mock('./hello-world-6') call in "/test.js" is not at the top level of the module. Although it appears nested, it will be hoisted and executed before any tests run. Move it to the top level to reflect its actual execution order. This will become an error in a future version. + See: https://vitest.dev/guide/mocking/modules#how-it-works", + ], + ], + "results": [ + { + "type": "return", + "value": undefined, + }, + { + "type": "return", + "value": undefined, + }, + { + "type": "return", + "value": undefined, + }, + { + "type": "return", + "value": undefined, + }, + { + "type": "return", + "value": undefined, + }, + { + "type": "return", + "value": undefined, + }, + { + "type": "return", + "value": undefined, + }, + ], + } + `) + }) + + it('ignores vi.mock position if import.meta.vitest is present', ({ onTestFinished }) => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + onTestFinished(() => warn.mockRestore()) + const result = hoistSimpleCode(` +if (import.meta.vitest) { + vi.mock('./hello-world-1') +} + `) + expect(result).toMatchInlineSnapshot(` + "import { vi } from "vitest" + vi.mock('./hello-world-1') + + if (import.meta.vitest) { + }" + `) + expect(warn).not.toHaveBeenCalled() + }) })