Skip to content
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

fix: don't resolve builtin Node modules #2538

Merged
merged 6 commits into from Dec 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/browser/rollup.config.js
Expand Up @@ -10,6 +10,7 @@ const external = [
...Object.keys(pkg.dependencies),
...Object.keys(pkg.peerDependencies || {}),
'worker_threads',
'node:worker_threads',
]

const plugins = [
Expand Down
1 change: 1 addition & 0 deletions packages/ui/rollup.config.js
Expand Up @@ -12,6 +12,7 @@ const external = [
...Object.keys(pkg.dependencies),
...Object.keys(pkg.peerDependencies || {}),
'worker_threads',
'node:worker_threads',
'vitest/node',
'vitest',
]
Expand Down
20 changes: 12 additions & 8 deletions packages/vite-node/rollup.config.js
Expand Up @@ -26,16 +26,11 @@ const external = [
'pathe',
'birpc',
'vite',
'url',
'events',
'node:url',
'node:events',
]

const plugins = [
alias({
entries: [
{ find: /^node:(.+)$/, replacement: '$1' },
],
}),
resolve({
preferBuiltins: true,
}),
Expand Down Expand Up @@ -68,7 +63,16 @@ export default defineConfig([
chunkFileNames: 'chunk-[name].cjs',
},
external,
plugins,
plugins: [
alias({
entries: [
// cjs in Node 14 doesn't support node: prefix
// can be dropped, when we drop support for Node 14
{ find: /^node:(.+)$/, replacement: '$1' },
],
}),
...plugins,
],
onwarn,
},
{
Expand Down
16 changes: 10 additions & 6 deletions packages/vite-node/src/client.ts
@@ -1,9 +1,9 @@
import { createRequire } from 'module'
import { createRequire } from 'node:module'
// we need native dirname, because windows __dirname has \\
// eslint-disable-next-line no-restricted-imports
import { dirname } from 'path'
import { fileURLToPath, pathToFileURL } from 'url'
import vm from 'vm'
import { dirname } from 'node:path'
import { fileURLToPath, pathToFileURL } from 'node:url'
import vm from 'node:vm'
import { isNodeBuiltin } from 'mlly'
import { resolve } from 'pathe'
import createDebug from 'debug'
import { VALID_ID_PREFIX, cleanUrl, isInternalRequest, isPrimitive, normalizeModuleId, normalizeRequestId, slash, toFilePath } from './utils'
Expand Down Expand Up @@ -190,8 +190,12 @@ export class ViteNodeRunner {
}
}

shouldResolveId(id: string, _importee?: string) {
return !isInternalRequest(id) && !isNodeBuiltin(id)
}

async resolveUrl(id: string, importee?: string): Promise<[url: string, fsPath: string]> {
if (isInternalRequest(id))
if (!this.shouldResolveId(id))
return [id, id]
// we don't pass down importee here, because otherwise Vite doesn't resolve it correctly
if (importee && id.startsWith(VALID_ID_PREFIX))
Expand Down
2 changes: 1 addition & 1 deletion packages/vite-node/src/debug.ts
@@ -1,5 +1,5 @@
/* eslint-disable no-console */
import { existsSync, promises as fs } from 'fs'
import { existsSync, promises as fs } from 'node:fs'
import { join, resolve } from 'pathe'
import type { TransformResult } from 'vite'
import c from 'picocolors'
Expand Down
2 changes: 1 addition & 1 deletion packages/vite-node/src/externalize.ts
@@ -1,4 +1,4 @@
import { existsSync } from 'fs'
import { existsSync } from 'node:fs'
import { isNodeBuiltin, isValidNodeImport } from 'mlly'
import type { DepsHandlingOptions } from './types'
import { slash } from './utils'
Expand Down
2 changes: 1 addition & 1 deletion packages/vite-node/src/hmr/emitter.ts
@@ -1,4 +1,4 @@
import { EventEmitter } from 'events'
import { EventEmitter } from 'node:events'
import type { HMRPayload, Plugin } from 'vite'

export type EventType = string | symbol
Expand Down
2 changes: 1 addition & 1 deletion packages/vite-node/src/server.ts
@@ -1,4 +1,4 @@
import { performance } from 'perf_hooks'
import { performance } from 'node:perf_hooks'
import { resolve } from 'pathe'
import type { TransformResult, ViteDevServer } from 'vite'
import createDebug from 'debug'
Expand Down
7 changes: 4 additions & 3 deletions packages/vite-node/src/utils.ts
@@ -1,5 +1,5 @@
import { fileURLToPath, pathToFileURL } from 'url'
import { existsSync } from 'fs'
import { fileURLToPath, pathToFileURL } from 'node:url'
import { existsSync } from 'node:fs'
import { resolve } from 'pathe'
import type { Arrayable, Nullable } from './types'

Expand All @@ -23,7 +23,7 @@ export function normalizeRequestId(id: string, base?: string): string {
.replace(/^\/@id\/__x00__/, '\0') // virtual modules start with `\0`
.replace(/^\/@id\//, '')
.replace(/^__vite-browser-external:/, '')
.replace(/^(node|file):/, '')
.replace(/^file:/, '')
.replace(/^\/+/, '/') // remove duplicate leading slashes
.replace(/\?v=\w+/, '?') // remove ?v= query
.replace(/&v=\w+/, '') // remove &v= query
Expand All @@ -50,6 +50,7 @@ export function normalizeModuleId(id: string) {
.replace(/\\/g, '/')
.replace(/^\/@fs\//, isWindows ? '' : '/')
.replace(/^file:\//, '/')
.replace(/^node:/, '')
.replace(/^\/+/, '/')
}

Expand Down
11 changes: 2 additions & 9 deletions packages/vitest/rollup.config.js
Expand Up @@ -6,7 +6,6 @@ import dts from 'rollup-plugin-dts'
import nodeResolve from '@rollup/plugin-node-resolve'
import commonjs from '@rollup/plugin-commonjs'
import json from '@rollup/plugin-json'
import alias from '@rollup/plugin-alias'
import license from 'rollup-plugin-license'
import c from 'picocolors'
import fg from 'fast-glob'
Expand Down Expand Up @@ -41,6 +40,8 @@ const external = [
...Object.keys(pkg.dependencies),
...Object.keys(pkg.peerDependencies),
'worker_threads',
'node:worker_threads',
'node:fs',
'inspector',
'vite-node/source-map',
'vite-node/client',
Expand All @@ -49,14 +50,6 @@ const external = [
]

const plugins = [
alias({
entries: [
{ find: /^node:(.+)$/, replacement: '$1' },
{ find: 'vite-node/server', replacement: resolve(__dirname, '../vite-node/src/server.ts') },
{ find: 'vite-node/client', replacement: resolve(__dirname, '../vite-node/src/client.ts') },
{ find: 'vite-node/utils', replacement: resolve(__dirname, '../vite-node/src/utils.ts') },
],
}),
nodeResolve({
preferBuiltins: true,
}),
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/api/setup.ts
@@ -1,4 +1,4 @@
import { promises as fs } from 'fs'
import { promises as fs } from 'node:fs'
import type { BirpcReturn } from 'birpc'
import { createBirpc } from 'birpc'
import { parse, stringify } from 'flatted'
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/constants.ts
@@ -1,4 +1,4 @@
import url from 'url'
import url from 'node:url'
import { resolve } from 'pathe'
import { isNode } from './utils/env'

Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/integrations/env/node.ts
@@ -1,4 +1,4 @@
import { Console } from 'console'
import { Console } from 'node:console'
import type { Environment } from '../../types'

export default <Environment>({
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/integrations/snapshot/port/state.ts
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import fs from 'fs'
import fs from 'node:fs'
import type { OptionsReceived as PrettyFormatOptions } from 'pretty-format'
import type { ParsedStack, SnapshotData, SnapshotMatchOptions, SnapshotResult, SnapshotStateOptions, SnapshotUpdateState } from '../../../types'
import { slash } from '../../../utils'
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/integrations/snapshot/port/utils.ts
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import fs from 'fs'
import fs from 'node:fs'
import { dirname, join } from 'pathe'
import naturalCompare from 'natural-compare'
import type { OptionsReceived as PrettyFormatOptions } from 'pretty-format'
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/node/cache/files.ts
@@ -1,5 +1,5 @@
import fs from 'fs'
import type { Stats } from 'fs'
import fs from 'node:fs'
import type { Stats } from 'node:fs'

type FileStatsCache = Pick<Stats, 'size'>

Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/node/cache/index.ts
@@ -1,4 +1,4 @@
import fs from 'fs'
import fs from 'node:fs'
import { findUp } from 'find-up'
import { resolve } from 'pathe'
import { loadConfigFromFile } from 'vite'
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/node/cache/results.ts
@@ -1,4 +1,4 @@
import fs from 'fs'
import fs from 'node:fs'
import { dirname, resolve } from 'pathe'
import type { File, ResolvedConfig } from '../../types'
import { version } from '../../../package.json'
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/node/core.ts
@@ -1,4 +1,4 @@
import { existsSync, promises as fs } from 'fs'
import { existsSync, promises as fs } from 'node:fs'
import type { ViteDevServer } from 'vite'
import { normalizePath } from 'vite'
import { relative, toNamespacedPath } from 'pathe'
Expand Down
6 changes: 3 additions & 3 deletions packages/vitest/src/node/pool.ts
@@ -1,6 +1,6 @@
import { MessageChannel } from 'worker_threads'
import _url from 'url'
import { cpus } from 'os'
import { MessageChannel } from 'node:worker_threads'
import _url from 'node:url'
import { cpus } from 'node:os'
import { resolve } from 'pathe'
import type { Options as TinypoolOptions } from 'tinypool'
import { Tinypool } from 'tinypool'
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/node/reporters/benchmark/json.ts
@@ -1,4 +1,4 @@
import { existsSync, promises as fs } from 'fs'
import { existsSync, promises as fs } from 'node:fs'
import { dirname, resolve } from 'pathe'
import type { Vitest } from '../../../node'
import type { BenchTaskResult, File, Reporter } from '../../../types'
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/node/reporters/json.ts
@@ -1,4 +1,4 @@
import { existsSync, promises as fs } from 'fs'
import { existsSync, promises as fs } from 'node:fs'
import { dirname, resolve } from 'pathe'
import type { Vitest } from '../../node'
import type { File, Reporter, Suite, Task, TaskState } from '../../types'
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/node/reporters/junit.ts
@@ -1,5 +1,5 @@
import { existsSync, promises as fs } from 'fs'
import { hostname } from 'os'
import { existsSync, promises as fs } from 'node:fs'
import { hostname } from 'node:os'
import { dirname, relative, resolve } from 'pathe'

import type { Vitest } from '../../node'
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/runtime/entry.ts
@@ -1,4 +1,4 @@
import { promises as fs } from 'fs'
import { promises as fs } from 'node:fs'
import type { EnvironmentOptions, ResolvedConfig, VitestEnvironment } from '../types'
import { getWorkerState, resetModules } from '../utils'
import { vi } from '../integrations/vi'
Expand Down
11 changes: 11 additions & 0 deletions packages/vitest/src/runtime/execute.ts
@@ -1,6 +1,8 @@
import { ViteNodeRunner } from 'vite-node/client'
import { isInternalRequest } from 'vite-node/utils'
import type { ViteNodeRunnerOptions } from 'vite-node'
import { normalizePath } from 'vite'
import { isNodeBuiltin } from 'mlly'
import type { MockMap } from '../types/mocker'
import { getCurrentEnvironment, getWorkerState } from '../utils'
import { VitestMocker } from './mocker'
Expand Down Expand Up @@ -31,6 +33,15 @@ export class VitestRunner extends ViteNodeRunner {
this.mocker = new VitestMocker(this)
}

shouldResolveId(id: string, _importee?: string | undefined): boolean {
if (isInternalRequest(id))
return false
const environment = getCurrentEnvironment()
// do not try and resolve node builtins in Node
// import('url') returns Node internal even if 'url' package is installed
return environment === 'node' ? !isNodeBuiltin(id) : true
}

async resolveUrl(id: string, importee?: string) {
if (importee && importee.startsWith('mock:'))
importee = importee.slice(5)
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/runtime/loader.ts
@@ -1,5 +1,5 @@
import { pathToFileURL } from 'url'
import { readFile } from 'fs/promises'
import { pathToFileURL } from 'node:url'
import { readFile } from 'node:fs/promises'
import { hasCJSSyntax, isNodeBuiltin } from 'mlly'
import { normalizeModuleId } from 'vite-node/utils'
import { getWorkerState } from '../utils'
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/runtime/mocker.ts
@@ -1,4 +1,4 @@
import { existsSync, readdirSync } from 'fs'
import { existsSync, readdirSync } from 'node:fs'
import { isNodeBuiltin } from 'mlly'
import { basename, dirname, extname, isAbsolute, join, resolve } from 'pathe'
import c from 'picocolors'
Expand Down
4 changes: 2 additions & 2 deletions packages/vitest/src/runtime/setup.ts
Expand Up @@ -50,8 +50,8 @@ export async function setupConsoleLogSpy() {
const timers = new Map<string, { stdoutTime: number; stderrTime: number; timer: any }>()
const unknownTestId = '__vitest__unknown_test__'

const { Writable } = await import('stream')
const { Console } = await import('console')
const { Writable } = await import('node:stream')
const { Console } = await import('node:console')

// group sync console.log calls with macro task
function schedule(taskId: string) {
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/typecheck/typechecker.ts
@@ -1,4 +1,4 @@
import { rm } from 'fs/promises'
import { rm } from 'node:fs/promises'
import type { ExecaChildProcess } from 'execa'
import { execa } from 'execa'
import { resolve } from 'pathe'
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/types/worker.ts
@@ -1,4 +1,4 @@
import type { MessagePort } from 'worker_threads'
import type { MessagePort } from 'node:worker_threads'
import type { FetchFunction, ModuleCacheMap, RawSourceMap, ViteNodeResolveId } from 'vite-node'
import type { BirpcReturn } from 'birpc'
import type { MockMap } from './mocker'
Expand Down
3 changes: 1 addition & 2 deletions packages/vitest/src/utils/index.ts
@@ -1,5 +1,4 @@
// eslint-disable-next-line no-restricted-imports
import { relative as relativeBrowser } from 'path'
import { relative as relativeBrowser } from 'node:path'
import c from 'picocolors'
import { isPackageExists } from 'local-pkg'
import { relative as relativeNode } from 'pathe'
Expand Down
2 changes: 1 addition & 1 deletion packages/web-worker/src/shared-worker.ts
@@ -1,4 +1,4 @@
import { MessageChannel, type MessagePort as NodeMessagePort } from 'worker_threads'
import { MessageChannel, type MessagePort as NodeMessagePort } from 'node:worker_threads'
import type { InlineWorkerContext, Procedure } from './types'
import { InlineWorkerRunner } from './runner'
import { debug, getRunnerOptions } from './utils'
Expand Down
2 changes: 2 additions & 0 deletions packages/ws-client/rollup.config.js
Expand Up @@ -13,7 +13,9 @@ const external = [
'ws',
'birpc',
'worker_threads',
'node:worker_threads',
'fs',
'node:fs',
'vitest',
'inspector',
]
Expand Down
2 changes: 2 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion scripts/update-contributors.ts
@@ -1,4 +1,4 @@
import { promises as fs } from 'fs'
import { promises as fs } from 'node:fs'
import { $fetch } from 'ohmyfetch'

interface Contributor {
Expand Down
2 changes: 1 addition & 1 deletion test/benchmark/test.mjs
@@ -1,4 +1,4 @@
import { readFile } from 'fs/promises'
import { readFile } from 'node:fs/promises'
import { execa } from 'execa'

let error
Expand Down
1 change: 1 addition & 0 deletions test/core/package.json
Expand Up @@ -7,6 +7,7 @@
},
"devDependencies": {
"tinyspy": "^1.0.2",
"url": "^0.11.0",
"vitest": "workspace:*"
}
}
18 changes: 18 additions & 0 deletions test/core/test/node-protocol-jsdom.spec.ts
@@ -0,0 +1,18 @@
// @vitest-environment jsdom

// outdated url package, which Vite will resolve to, if "url" import is used
// this should help catch bugs in source code
import packageUrl from 'url'
import nodeUrl from 'node:url'
import { expect, it } from 'vitest'

it('vitest resolves url to installed url package, but node:url to internal Node module', () => {
expect(packageUrl).not.toHaveProperty('URL')
expect(packageUrl).not.toHaveProperty('URLSearchParams')
expect(packageUrl).not.toHaveProperty('fileURLToPath')
expect(nodeUrl).toHaveProperty('URL')
expect(nodeUrl).toHaveProperty('URLSearchParams')
expect(nodeUrl).toHaveProperty('fileURLToPath')
// eslint-disable-next-line n/no-deprecated-api
expect(packageUrl.parse !== nodeUrl.parse).toBe(true)
})