-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix: fix vi.importActual() for virtual modules
#9772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
9cc0b8e
71fbf91
d4c4de5
919b4c2
1aedd3b
b621937
fc50d52
1e21b99
158c1e6
caf3289
1099aec
42539a0
4a8fb50
bba414c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import { getCachedVitestImport } from './cachedResolver' | |
| import { unwrapId, VitestModuleEvaluator } from './moduleEvaluator' | ||
| import { VitestMocker } from './moduleMocker' | ||
| import { VitestModuleRunner } from './moduleRunner' | ||
| import { removeQuery } from './utils' | ||
|
|
||
| const { readFileSync } = fs | ||
|
|
||
|
|
@@ -95,6 +96,13 @@ export function startVitestModuleRunner(options: ContextModuleRunnerOptions): Vi | |
| return vitest | ||
| } | ||
|
|
||
| // strip _vitest_original query added by importActual so that | ||
| // the plugin pipeline sees the original import id (e.g. virtual modules's load hook) | ||
| const isImportActual = id.includes('_vitest_original') | ||
| if (isImportActual) { | ||
| id = removeQuery(id, '_vitest_original') | ||
| } | ||
|
|
||
| const rawId = unwrapId(id) | ||
| resolvingModules.add(rawId) | ||
|
|
||
|
|
@@ -103,15 +111,17 @@ export function startVitestModuleRunner(options: ContextModuleRunnerOptions): Vi | |
| await moduleRunner.mocker.resolveMocks() | ||
| } | ||
|
|
||
| const resolvedMock = moduleRunner.mocker.getDependencyMock(rawId) | ||
| if (resolvedMock?.type === 'manual' || resolvedMock?.type === 'redirect') { | ||
| return { | ||
| code: '', | ||
| file: null, | ||
| id: resolvedMock.id, | ||
| url: resolvedMock.url, | ||
| invalidate: false, | ||
| mockedModule: resolvedMock, | ||
| if (!isImportActual) { | ||
| const resolvedMock = moduleRunner.mocker.getDependencyMock(rawId) | ||
| if (resolvedMock?.type === 'manual' || resolvedMock?.type === 'redirect') { | ||
| return { | ||
| code: '', | ||
| file: null, | ||
| id: resolvedMock.id, | ||
| url: resolvedMock.url, | ||
| invalidate: false, | ||
| mockedModule: resolvedMock, | ||
| } | ||
|
Comment on lines
+114
to
+124
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The issue was that previously This PR adds |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| // copied from vite/src/shared/utils.ts | ||
| const postfixRE = /[?#].*$/ | ||
|
|
||
| function cleanUrl(url: string): string { | ||
| return url.replace(postfixRE, '') | ||
| } | ||
| function splitFileAndPostfix(path: string): { file: string; postfix: string } { | ||
| const file = cleanUrl(path) | ||
| return { file, postfix: path.slice(file.length) } | ||
| } | ||
|
|
||
| export function injectQuery(url: string, queryToInject: string): string { | ||
| const { file, postfix } = splitFileAndPostfix(url) | ||
| return `${file}?${queryToInject}${postfix[0] === '?' ? `&${postfix.slice(1)}` : /* hash only */ postfix}` | ||
| } | ||
|
|
||
| export function removeQuery(url: string, queryToRemove: string): string { | ||
| return url | ||
| .replace(new RegExp(`[?&]${queryToRemove}(?=[&#]|$)`), '') | ||
| .replace(/\?$/, '') | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,16 @@ | ||
| import type { RunVitestConfig } from '../../test-utils' | ||
| import { setDefaultResultOrder } from 'node:dns' | ||
| import path from 'node:path' | ||
| import { expect, test } from 'vitest' | ||
| import { playwright } from '@vitest/browser-playwright' | ||
| import { webdriverio } from '@vitest/browser-webdriverio' | ||
| import { afterAll, expect, test } from 'vitest' | ||
| import { rolldownVersion } from 'vitest/node' | ||
| import { runInlineTests, runVitest } from '../../test-utils' | ||
|
|
||
| // webdriver@9 sets dns.setDefaultResultOrder("ipv4first") on import, | ||
| // which makes Vite resolve localhost to 127.0.0.1 and breaks other tests asserting "localhost" | ||
| afterAll(() => setDefaultResultOrder('verbatim')) | ||
|
Comment on lines
+10
to
+12
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is needed to avoid this silly |
||
|
|
||
| test('setting resetMocks works if restoreMocks is also set', async () => { | ||
| const { stderr, testTree } = await runInlineTests({ | ||
| 'vitest.config.js': { | ||
|
|
@@ -133,3 +141,139 @@ test('can mock invalid module', () => { | |
| `) | ||
| } | ||
| }) | ||
|
|
||
| function modeToConfig(mode: string): RunVitestConfig { | ||
| if (mode === 'playwright') { | ||
| return { | ||
| browser: { | ||
| enabled: true, | ||
| provider: playwright(), | ||
| instances: [{ browser: 'chromium' }], | ||
| headless: true, | ||
| }, | ||
| } | ||
| } | ||
| if (mode === 'webdriverio') { | ||
| return { | ||
| browser: { | ||
| enabled: true, | ||
| provider: webdriverio(), | ||
| instances: [{ browser: 'chrome' }], | ||
| headless: true, | ||
| }, | ||
| } | ||
| } | ||
| return {} | ||
| } | ||
|
|
||
| test.for(['node', 'playwright', 'webdriverio'])('importOriginal for virtual modules (%s)', async (mode) => { | ||
| const { stderr, errorTree, root } = await runInlineTests({ | ||
| 'vitest.config.js': ` | ||
| import { defineConfig } from 'vitest/config' | ||
| export default defineConfig({ | ||
| plugins: [{ | ||
| name: 'virtual-test', | ||
| resolveId(source) { | ||
| if (source === 'virtual:my-module') { | ||
| return "\\0" + source | ||
| } | ||
| }, | ||
| load(id) { | ||
| if (id === '\\0virtual:my-module') { | ||
| return 'export const value = "original"' | ||
| } | ||
| }, | ||
| }], | ||
| }) | ||
| `, | ||
| './basic.test.js': ` | ||
| import { test, expect, vi } from 'vitest' | ||
| import { value } from 'virtual:my-module' | ||
|
|
||
| vi.mock('virtual:my-module', async (importOriginal) => { | ||
| const original = await importOriginal() | ||
| return { value: original.value + '-modified' } | ||
| }) | ||
|
|
||
| test('importOriginal returns original virtual module exports', () => { | ||
| expect(value).toBe('original-modified') | ||
| }) | ||
| `, | ||
| }, modeToConfig(mode)) | ||
|
|
||
| // webdriverio uses a server-side interceptor plugin whose load hook | ||
| // intercepts the clean id, so importActual returns the mock instead | ||
| // of the original module. This is a known limitation. | ||
| if (mode === 'webdriverio') { | ||
| const tree = errorTree() | ||
| tree['basic.test.js'].__module_errors__ = tree['basic.test.js'].__module_errors__.map( | ||
| (e: string) => e.replace(root, '<root>'), | ||
| ) | ||
| expect(tree).toMatchInlineSnapshot(` | ||
| { | ||
| "__unhandled_errors__": [ | ||
| "[vitest] There was an error when mocking a module. If you are using "vi.mock" factory, make sure there are no top level variables inside, since this call is hoisted to top of the file. Read more: https://vitest.dev/api/vi.html#vi-mock", | ||
| ], | ||
| "basic.test.js": { | ||
| "__module_errors__": [ | ||
| "Failed to import test file <root>/basic.test.js", | ||
| ], | ||
| }, | ||
| } | ||
| `) | ||
| } | ||
| else { | ||
| expect(stderr).toBe('') | ||
| expect(errorTree()).toMatchInlineSnapshot(` | ||
| { | ||
| "basic.test.js": { | ||
| "importOriginal returns original virtual module exports": "passed", | ||
| }, | ||
| } | ||
| `) | ||
| } | ||
| }) | ||
|
|
||
| test.for(['node', 'playwright', 'webdriverio'])('mocking virtual module without importOriginal skips loading original (%s)', async (mode) => { | ||
| const { stderr, testTree } = await runInlineTests({ | ||
| 'vitest.config.js': ` | ||
| import { defineConfig } from 'vitest/config' | ||
| export default defineConfig({ | ||
| plugins: [{ | ||
| name: 'virtual-test', | ||
| resolveId(source) { | ||
| if (source === 'virtual:my-module') { | ||
| return "\\0" + source | ||
| } | ||
| }, | ||
| load(id) { | ||
| if (id === '\\0virtual:my-module') { | ||
| throw new Error('virtual module load should not be called') | ||
| } | ||
| }, | ||
| }], | ||
| }) | ||
| `, | ||
| './basic.test.js': ` | ||
| import { test, expect, vi } from 'vitest' | ||
| import { value } from 'virtual:my-module' | ||
|
|
||
| vi.mock('virtual:my-module', () => { | ||
| return { value: 'mocked' } | ||
| }) | ||
|
|
||
| test('mock works without loading original', () => { | ||
| expect(value).toBe('mocked') | ||
| }) | ||
| `, | ||
| }, modeToConfig(mode)) | ||
|
|
||
| expect(stderr).toBe('') | ||
| expect(testTree()).toMatchInlineSnapshot(` | ||
| { | ||
| "basic.test.js": { | ||
| "mock works without loading original": "passed", | ||
| }, | ||
| } | ||
| `) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally this should live in playwright provider, but don't think that's possible now, so here it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually instead of middleware level rewriting, it should be also possible to just strip
?_vitest_originalfromresolveIdlikeThis is because the
sourcecomes here is already the resolved id +?_vitest_originalsuch e.g."\0virtual:module?_vitest_original".In fact, I used a similar idea in RSC plugin to "redirect" already resolved id back as something importable https://github.com/vitejs/vite-plugin-react/blob/main/packages/plugin-rsc/src/plugins/resolved-id-proxy.ts.