fix: don't resolve builtin Node modules (#2538)

sheremet-va · web-flow · commit 1cbc24dca6de · 2022-12-20T12:57:48.000+03:00
* fix: use "node:" prefix in Vitest imports

* chore: add test for prefix

* chore: cleanup

* chore: use more node protocol

* chore: cleanup

* chore: cleanup
diff --git a/packages/browser/rollup.config.js b/packages/browser/rollup.config.js
@@ -10,6 +10,7 @@ const external = [
   ...Object.keys(pkg.dependencies),
   ...Object.keys(pkg.peerDependencies || {}),
   'worker_threads',
+  'node:worker_threads',
 ]
 
 const plugins = [
diff --git a/packages/ui/rollup.config.js b/packages/ui/rollup.config.js
@@ -12,6 +12,7 @@ const external = [
   ...Object.keys(pkg.dependencies),
   ...Object.keys(pkg.peerDependencies || {}),
   'worker_threads',
+  'node:worker_threads',
   'vitest/node',
   'vitest',
 ]
diff --git a/packages/vite-node/rollup.config.js b/packages/vite-node/rollup.config.js
@@ -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,
   }),
@@ -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,
   },
   {
diff --git a/packages/vite-node/src/client.ts b/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'
@@ -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))
diff --git a/packages/vite-node/src/debug.ts b/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'
diff --git a/packages/vite-node/src/externalize.ts b/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'
diff --git a/packages/vite-node/src/hmr/emitter.ts b/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
diff --git a/packages/vite-node/src/server.ts b/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'
diff --git a/packages/vite-node/src/utils.ts b/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'
 
@@ -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
@@ -50,6 +50,7 @@ export function normalizeModuleId(id: string) {
     .replace(/\\/g, '/')
     .replace(/^\/@fs\//, isWindows ? '' : '/')
     .replace(/^file:\//, '/')
+    .replace(/^node:/, '')
     .replace(/^\/+/, '/')
 }
 
diff --git a/packages/vitest/rollup.config.js b/packages/vitest/rollup.config.js
@@ -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'
@@ -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',
@@ -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,
   }),
diff --git a/packages/vitest/src/api/setup.ts b/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'
diff --git a/packages/vitest/src/constants.ts b/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'
 
diff --git a/packages/vitest/src/integrations/env/node.ts b/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>({
diff --git a/packages/vitest/src/integrations/snapshot/port/state.ts b/packages/vitest/src/integrations/snapshot/port/state.ts
@@ -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'
diff --git a/packages/vitest/src/integrations/snapshot/port/utils.ts b/packages/vitest/src/integrations/snapshot/port/utils.ts
@@ -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'
diff --git a/packages/vitest/src/node/cache/files.ts b/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'>
 
diff --git a/packages/vitest/src/node/cache/index.ts b/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'
diff --git a/packages/vitest/src/node/cache/results.ts b/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'
diff --git a/packages/vitest/src/node/core.ts b/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'
diff --git a/packages/vitest/src/node/pool.ts b/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'
diff --git a/packages/vitest/src/node/reporters/benchmark/json.ts b/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'
diff --git a/packages/vitest/src/node/reporters/json.ts b/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'
diff --git a/packages/vitest/src/node/reporters/junit.ts b/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'
diff --git a/packages/vitest/src/runtime/entry.ts b/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'
diff --git a/packages/vitest/src/runtime/execute.ts b/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'
@@ -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)
diff --git a/packages/vitest/src/runtime/loader.ts b/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'
diff --git a/packages/vitest/src/runtime/mocker.ts b/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'
diff --git a/packages/vitest/src/runtime/setup.ts b/packages/vitest/src/runtime/setup.ts
@@ -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) {
diff --git a/packages/vitest/src/typecheck/typechecker.ts b/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'
diff --git a/packages/vitest/src/types/worker.ts b/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'
diff --git a/packages/vitest/src/utils/index.ts b/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'
diff --git a/packages/web-worker/src/shared-worker.ts b/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'
diff --git a/packages/ws-client/rollup.config.js b/packages/ws-client/rollup.config.js
@@ -13,7 +13,9 @@ const external = [
   'ws',
   'birpc',
   'worker_threads',
+  'node:worker_threads',
   'fs',
+  'node:fs',
   'vitest',
   'inspector',
 ]
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/scripts/update-contributors.ts b/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 {
diff --git a/test/benchmark/test.mjs b/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
diff --git a/test/core/package.json b/test/core/package.json
@@ -7,6 +7,7 @@
   },
   "devDependencies": {
     "tinyspy": "^1.0.2",
+    "url": "^0.11.0",
     "vitest": "workspace:*"
   }
 }
diff --git a/test/core/test/node-protocol-jsdom.spec.ts b/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)
+})
diff --git a/test/core/test/node-protocol-node.spec.ts b/test/core/test/node-protocol-node.spec.ts

Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,7 @@ const external = [`
`10`	`10`	`...Object.keys(pkg.dependencies),`
`11`	`11`	`...Object.keys(pkg.peerDependencies \|\| {}),`
`12`	`12`	`'worker_threads',`
	`13`	`+ 'node:worker_threads',`
`13`	`14`	`]`
`14`	`15`
`15`	`16`	`const plugins = [`
Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,7 @@ const external = [`
`12`	`12`	`...Object.keys(pkg.dependencies),`
`13`	`13`	`...Object.keys(pkg.peerDependencies \|\| {}),`
`14`	`14`	`'worker_threads',`
	`15`	`+ 'node:worker_threads',`
`15`	`16`	`'vitest/node',`
`16`	`17`	`'vitest',`
`17`	`18`	`]`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-import { existsSync } from 'fs'`
	`1`	`+import { existsSync } from 'node:fs'`
`2`	`2`	`import { isNodeBuiltin, isValidNodeImport } from 'mlly'`
`3`	`3`	`import type { DepsHandlingOptions } from './types'`
`4`	`4`	`import { slash } from './utils'`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-import { EventEmitter } from 'events'`
	`1`	`+import { EventEmitter } from 'node:events'`
`2`	`2`	`import type { HMRPayload, Plugin } from 'vite'`
`3`	`3`
`4`	`4`	`export type EventType = string \| symbol`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-import { performance } from 'perf_hooks'`
	`1`	`+import { performance } from 'node:perf_hooks'`
`2`	`2`	`import { resolve } from 'pathe'`
`3`	`3`	`import type { TransformResult, ViteDevServer } from 'vite'`
`4`	`4`	`import createDebug from 'debug'`