From 1ad63b0c61865addf857ce2846496c7db47177fc Mon Sep 17 00:00:00 2001 From: Vladimir Date: Tue, 6 Jun 2023 18:59:55 +0200 Subject: [PATCH] feat!: throw an error, if module cannot be resolved (#3307) --- docs/.vitepress/config.ts | 4 +++ docs/guide/common-errors.md | 40 +++++++++++++++++++++++ packages/vite-node/src/client.ts | 20 ++++++++---- packages/vitest/src/runtime/execute.ts | 23 +++++++++++-- packages/vitest/src/runtime/mocker.ts | 27 +++++++++++---- packages/web-worker/src/shared-worker.ts | 22 ++++++------- packages/web-worker/src/worker.ts | 22 ++++++------- test/core/test/imports.test.ts | 4 +-- test/core/test/unmock-import.test.ts | 2 +- test/web-worker/test/init.test.ts | 2 +- test/web-worker/test/sharedWorker.spec.ts | 4 +-- test/web-worker/vitest.config.ts | 2 +- 12 files changed, 128 insertions(+), 44 deletions(-) create mode 100644 docs/guide/common-errors.md diff --git a/docs/.vitepress/config.ts b/docs/.vitepress/config.ts index 3c9312dffed0..35da71da1b53 100644 --- a/docs/.vitepress/config.ts +++ b/docs/.vitepress/config.ts @@ -219,6 +219,10 @@ export default withPwa(defineConfig({ text: 'Migration Guide', link: '/guide/migration', }, + { + text: 'Common Errors', + link: '/guide/common-errors', + }, ], }, { diff --git a/docs/guide/common-errors.md b/docs/guide/common-errors.md new file mode 100644 index 000000000000..a8075e4ba6fb --- /dev/null +++ b/docs/guide/common-errors.md @@ -0,0 +1,40 @@ +# Common Errors + +## Cannot find module './relative-path' + +If you receive an error that module cannot be found, it might mean several different things: + +- 1. You misspelled the path. Make sure the path is correct. + +- 2. It's possible that your rely on `baseUrl` in your `tsconfig.json`. Vite doesn't take into account `tsconfig.json` by default, so you might need to install [`vite-tsconfig-paths`](https://www.npmjs.com/package/vite-tsconfig-paths) yourself, if you rely on this behaviour. + +```ts +import { defineConfig } from 'vitest/config' +import tsconfigPaths from 'vite-tsconfig-paths' + +export default defineConfig({ + plugins: [tsconfigPaths()] +}) +``` + +Or rewrite your path to not be relative to root: + +```diff +- import helpers from 'src/helpers' ++ import helpers from '../src/helpers' +``` + +- 3. Make sure you don't have relative [aliases](/config/#alias). Vite treats them as relative to the file where the import is instead of the root. + +```diff +import { defineConfig } from 'vitest/config' + +export default defineConfig({ + test: { + alias: { +- '@/': './src/', ++ '@/': new URL('./src/', import.meta.url).pathname, + } + } +}) +``` \ No newline at end of file diff --git a/packages/vite-node/src/client.ts b/packages/vite-node/src/client.ts index c6ba7ebc0da7..04cb46575837 100644 --- a/packages/vite-node/src/client.ts +++ b/packages/vite-node/src/client.ts @@ -228,7 +228,7 @@ export class ViteNodeRunner { } shouldResolveId(id: string, _importee?: string) { - return !isInternalRequest(id) && !isNodeBuiltin(id) + return !isInternalRequest(id) && !isNodeBuiltin(id) && !id.startsWith('data:') } private async _resolveUrl(id: string, importer?: string): Promise<[url: string, fsPath: string]> { @@ -243,12 +243,18 @@ export class ViteNodeRunner { if (!this.options.resolveId || exists) return [id, path] const resolved = await this.options.resolveId(id, importer) - const resolvedId = resolved - ? normalizeRequestId(resolved.id, this.options.base) - : id - // to be compatible with dependencies that do not resolve id - const fsPath = resolved ? resolvedId : path - return [resolvedId, fsPath] + if (!resolved) { + const error = new Error( + `Cannot find module '${id}'${importer ? ` imported from '${importer}'` : ''}.` + + '\n\n- If you rely on tsconfig.json to resolve modules, please install "vite-tsconfig-paths" plugin to handle module resolution.' + + '\n - Make sure you don\'t have relative aliases in your Vitest config. Use absolute paths instead. Read more: https://vitest.dev/guide/common-errors', + ) + Object.defineProperty(error, 'code', { value: 'ERR_MODULE_NOT_FOUND', enumerable: true }) + Object.defineProperty(error, Symbol.for('vitest.error.not_found.data'), { value: { id, importer }, enumerable: false }) + throw error + } + const resolvedId = normalizeRequestId(resolved.id, this.options.base) + return [resolvedId, resolvedId] } async resolveUrl(id: string, importee?: string) { diff --git a/packages/vitest/src/runtime/execute.ts b/packages/vitest/src/runtime/execute.ts index 3cc62ad2aa79..de7cf1550c07 100644 --- a/packages/vitest/src/runtime/execute.ts +++ b/packages/vitest/src/runtime/execute.ts @@ -99,7 +99,7 @@ export class VitestExecutor extends ViteNodeRunner { } shouldResolveId(id: string, _importee?: string | undefined): boolean { - if (isInternalRequest(id)) + if (isInternalRequest(id) || id.startsWith('data:')) return false const environment = getCurrentEnvironment() // do not try and resolve node builtins in Node @@ -107,10 +107,29 @@ export class VitestExecutor extends ViteNodeRunner { return environment === 'node' ? !isNodeBuiltin(id) : !id.startsWith('node:') } + async originalResolveUrl(id: string, importer?: string) { + return super.resolveUrl(id, importer) + } + async resolveUrl(id: string, importer?: string) { + if (VitestMocker.pendingIds.length) + await this.mocker.resolveMocks() + if (importer && importer.startsWith('mock:')) importer = importer.slice(5) - return super.resolveUrl(id, importer) + try { + return await super.resolveUrl(id, importer) + } + catch (error: any) { + if (error.code === 'ERR_MODULE_NOT_FOUND') { + const { id } = error[Symbol.for('vitest.error.not_found.data')] + const path = this.mocker.normalizePath(id) + const mock = this.mocker.getDependencyMock(path) + if (mock !== undefined) + return [id, id] as [string, string] + } + throw error + } } async dependencyRequest(id: string, fsPath: string, callstack: string[]): Promise { diff --git a/packages/vitest/src/runtime/mocker.ts b/packages/vitest/src/runtime/mocker.ts index 8b818bd8b399..dfcbc0a189ac 100644 --- a/packages/vitest/src/runtime/mocker.ts +++ b/packages/vitest/src/runtime/mocker.ts @@ -39,7 +39,7 @@ function isSpecialProp(prop: Key, parentType: string) { } export class VitestMocker { - private static pendingIds: PendingSuiteMock[] = [] + public static pendingIds: PendingSuiteMock[] = [] private resolveCache = new Map>() constructor( @@ -88,7 +88,22 @@ export class VitestMocker { } private async resolvePath(rawId: string, importer: string) { - const [id, fsPath] = await this.executor.resolveUrl(rawId, importer) + let id: string + let fsPath: string + try { + [id, fsPath] = await this.executor.originalResolveUrl(rawId, importer) + } + catch (error: any) { + // it's allowed to mock unresolved modules + if (error.code === 'ERR_MODULE_NOT_FOUND') { + const { id: unresolvedId } = error[Symbol.for('vitest.error.not_found.data')] + id = unresolvedId + fsPath = unresolvedId + } + else { + throw error + } + } // external is node_module or unresolved module // for example, some people mock "vscode" and don't have it installed const external = (!isAbsolute(fsPath) || this.isAModuleDirectory(fsPath)) ? rawId : null @@ -100,7 +115,10 @@ export class VitestMocker { } } - private async resolveMocks() { + public async resolveMocks() { + if (!VitestMocker.pendingIds.length) + return + await Promise.all(VitestMocker.pendingIds.map(async (mock) => { const { fsPath, external } = await this.resolvePath(mock.id, mock.importer) if (mock.type === 'unmock') @@ -353,9 +371,6 @@ export class VitestMocker { } public async requestWithMock(url: string, callstack: string[]) { - if (VitestMocker.pendingIds.length) - await this.resolveMocks() - const id = this.normalizePath(url) const mock = this.getDependencyMock(id) diff --git a/packages/web-worker/src/shared-worker.ts b/packages/web-worker/src/shared-worker.ts index 4f7ad91195e6..7a06c85b0160 100644 --- a/packages/web-worker/src/shared-worker.ts +++ b/packages/web-worker/src/shared-worker.ts @@ -110,7 +110,7 @@ export function createSharedWorkerConstructor(): typeof SharedWorker { debug('initialize shared worker %s', this._vw_name) - runner.executeFile(fsPath).then(() => { + return runner.executeFile(fsPath).then(() => { // worker should be new every time, invalidate its sub dependency runnerOptions.moduleCache.invalidateSubDepTree([fsPath, runner.mocker.getMockPath(fsPath)]) this._vw_workerTarget.dispatchEvent( @@ -119,17 +119,17 @@ export function createSharedWorkerConstructor(): typeof SharedWorker { }), ) debug('shared worker %s successfully initialized', this._vw_name) - }).catch((e) => { - debug('shared worker %s failed to initialize: %o', this._vw_name, e) - const EventConstructor = globalThis.ErrorEvent || globalThis.Event - const error = new EventConstructor('error', { - error: e, - message: e.message, - }) - this.dispatchEvent(error) - this.onerror?.(error) - console.error(e) }) + }).catch((e) => { + debug('shared worker %s failed to initialize: %o', this._vw_name, e) + const EventConstructor = globalThis.ErrorEvent || globalThis.Event + const error = new EventConstructor('error', { + error: e, + message: e.message, + }) + this.dispatchEvent(error) + this.onerror?.(error) + console.error(e) }) } } diff --git a/packages/web-worker/src/worker.ts b/packages/web-worker/src/worker.ts index 73f0a1e983ba..805cda8508df 100644 --- a/packages/web-worker/src/worker.ts +++ b/packages/web-worker/src/worker.ts @@ -75,7 +75,7 @@ export function createWorkerConstructor(options?: DefineWorkerOptions): typeof W debug('initialize worker %s', this._vw_name) - runner.executeFile(fsPath).then(() => { + return runner.executeFile(fsPath).then(() => { // worker should be new every time, invalidate its sub dependency runnerOptions.moduleCache.invalidateSubDepTree([fsPath, runner.mocker.getMockPath(fsPath)]) const q = this._vw_messageQueue @@ -83,17 +83,17 @@ export function createWorkerConstructor(options?: DefineWorkerOptions): typeof W if (q) q.forEach(([data, transfer]) => this.postMessage(data, transfer), this) debug('worker %s successfully initialized', this._vw_name) - }).catch((e) => { - debug('worker %s failed to initialize: %o', this._vw_name, e) - const EventConstructor = globalThis.ErrorEvent || globalThis.Event - const error = new EventConstructor('error', { - error: e, - message: e.message, - }) - this.dispatchEvent(error) - this.onerror?.(error) - console.error(e) }) + }).catch((e) => { + debug('worker %s failed to initialize: %o', this._vw_name, e) + const EventConstructor = globalThis.ErrorEvent || globalThis.Event + const error = new EventConstructor('error', { + error: e, + message: e.message, + }) + this.dispatchEvent(error) + this.onerror?.(error) + console.error(e) }) } diff --git a/test/core/test/imports.test.ts b/test/core/test/imports.test.ts index 646bd9ec3945..ba2b241d673b 100644 --- a/test/core/test/imports.test.ts +++ b/test/core/test/imports.test.ts @@ -74,9 +74,9 @@ test('dynamic import has null prototype', async () => { test('dynamic import throws an error', async () => { const path = './some-unknown-path' const imported = import(path) - await expect(imported).rejects.toThrowError(/Failed to load/) + await expect(imported).rejects.toThrowError(/Cannot find module '\.\/some-unknown-path'/) // @ts-expect-error path does not exist - await expect(() => import('./some-unknown-path')).rejects.toThrowError(/Failed to load/) + await expect(() => import('./some-unknown-path')).rejects.toThrowError(/Cannot find module/) }) test('can import @vite/client', async () => { diff --git a/test/core/test/unmock-import.test.ts b/test/core/test/unmock-import.test.ts index d7123feb237d..1fc99e0d2848 100644 --- a/test/core/test/unmock-import.test.ts +++ b/test/core/test/unmock-import.test.ts @@ -20,7 +20,7 @@ test('first import', async () => { expect(data.state).toBe('STOPPED') }) -test('second import should had been re-mock', async () => { +test('second import should have been re-mocked', async () => { // @ts-expect-error I know this const { data } = await import('/data') expect(data.state).toBe('STARTED') diff --git a/test/web-worker/test/init.test.ts b/test/web-worker/test/init.test.ts index 27f7434e5167..043b47679265 100644 --- a/test/web-worker/test/init.test.ts +++ b/test/web-worker/test/init.test.ts @@ -66,7 +66,7 @@ it('worker with invalid url throws an error', async () => { }) expect(event).toBeInstanceOf(ErrorEvent) expect(event.error).toBeInstanceOf(Error) - expect(event.error.message).toContain('Failed to load') + expect(event.error.message).toContain('Cannot find module') }) it('self injected into worker and its deps should be equal', async () => { diff --git a/test/web-worker/test/sharedWorker.spec.ts b/test/web-worker/test/sharedWorker.spec.ts index 0fa4649e5bcc..aa59db39812b 100644 --- a/test/web-worker/test/sharedWorker.spec.ts +++ b/test/web-worker/test/sharedWorker.spec.ts @@ -1,5 +1,5 @@ import { expect, it } from 'vitest' -import MySharedWorker from './src/sharedWorker?sharedworker' +import MySharedWorker from '../src/sharedWorker?sharedworker' function sendEventMessage(worker: SharedWorker, msg: any) { worker.port.postMessage(msg) @@ -50,7 +50,7 @@ it('throws an error on invalid path', async () => { }) expect(event).toBeInstanceOf(ErrorEvent) expect(event.error).toBeInstanceOf(Error) - expect(event.error.message).toContain('Failed to load') + expect(event.error.message).toContain('Cannot find module') }) it('doesn\'t trigger events, if closed', async () => { diff --git a/test/web-worker/vitest.config.ts b/test/web-worker/vitest.config.ts index 69f70c696d4d..a73a97fc9727 100644 --- a/test/web-worker/vitest.config.ts +++ b/test/web-worker/vitest.config.ts @@ -12,7 +12,7 @@ export default defineConfig({ ], }, onConsoleLog(log) { - if (log.includes('Failed to load')) + if (log.includes('Cannot find module')) return false }, },