From bc680b820c6bd2a8bd045b2d495d2fa74d95c1b4 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 5 Nov 2024 19:27:01 +0000 Subject: [PATCH 01/21] feat: add the new environment config option `isBuiltin` --- packages/vite/src/node/config.ts | 7 ++++ packages/vite/src/node/external.ts | 5 ++- .../src/node/optimizer/esbuildDepPlugin.ts | 5 ++- .../vite/src/node/plugins/importAnalysis.ts | 2 +- packages/vite/src/node/plugins/resolve.ts | 39 ++++++++++++++----- packages/vite/src/node/ssr/fetchModule.ts | 3 +- packages/vite/src/node/utils.ts | 16 ++++++-- 7 files changed, 60 insertions(+), 17 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index c2c3bda7a16147..aa63d6f3edb2d2 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -260,6 +260,11 @@ export interface SharedEnvironmentOptions { * Optimize deps config */ optimizeDeps?: DepOptimizationOptions + /** + * Function to specify when modules should be considered built-in for the environment. + * (If not provided the node built-in modules are the only ones assumed as such) + */ + isBuiltin?: (id: string) => boolean } export interface EnvironmentOptions extends SharedEnvironmentOptions { @@ -283,6 +288,7 @@ export type ResolvedEnvironmentOptions = { optimizeDeps: DepOptimizationOptions dev: ResolvedDevEnvironmentOptions build: ResolvedBuildEnvironmentOptions + isBuiltin?: (id: string) => boolean } export type DefaultEnvironmentOptions = Omit< @@ -772,6 +778,7 @@ function resolveEnvironmentOptions( resolve.preserveSymlinks, consumer, ), + isBuiltin: options.isBuiltin, dev: resolveDevEnvironmentOptions( options.dev, environmentName, diff --git a/packages/vite/src/node/external.ts b/packages/vite/src/node/external.ts index 58d30cfc096371..57c8df6d37670d 100644 --- a/packages/vite/src/node/external.ts +++ b/packages/vite/src/node/external.ts @@ -88,6 +88,7 @@ export function createIsConfiguredAsExternal( resolveOptions, undefined, false, + environment.config, ) if (!resolved) { return false @@ -155,7 +156,9 @@ function createIsExternal( } let isExternal = false if (id[0] !== '.' && !path.isAbsolute(id)) { - isExternal = isBuiltin(id) || isConfiguredAsExternal(id, importer) + isExternal = + isBuiltin(id, environment.config) || + isConfiguredAsExternal(id, importer) } processedIds.set(id, isExternal) return isExternal diff --git a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts index 7afbb7fbdea82f..c1fde0fb1bdbe7 100644 --- a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts +++ b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts @@ -115,7 +115,10 @@ export function esbuildDepPlugin( namespace: 'optional-peer-dep', } } - if (environment.config.consumer === 'server' && isBuiltin(resolved)) { + if ( + environment.config.consumer === 'server' && + isBuiltin(resolved, environment.config) + ) { return } if (isExternalUrl(resolved)) { diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts index f13d920fb83647..5f37c54b48d35b 100644 --- a/packages/vite/src/node/plugins/importAnalysis.ts +++ b/packages/vite/src/node/plugins/importAnalysis.ts @@ -519,7 +519,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin { if (shouldExternalize(environment, specifier, importer)) { return } - if (isBuiltin(specifier)) { + if (isBuiltin(specifier, environment.config)) { return } } diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index f75798b84abe8e..30588ffe3bef2c 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -35,7 +35,7 @@ import { } from '../utils' import { optimizedDepInfoFromFile, optimizedDepInfoFromId } from '../optimizer' import type { DepsOptimizer } from '../optimizer' -import type { SSROptions } from '..' +import type { EnvironmentOptions, SSROptions } from '..' import type { PackageCache, PackageData } from '../packages' import { canExternalizeFile, shouldExternalize } from '../external' import { @@ -320,7 +320,13 @@ export function resolvePlugin( if ( options.mainFields.includes('browser') && - (res = tryResolveBrowserMapping(fsPath, importer, options, true)) + (res = tryResolveBrowserMapping( + fsPath, + importer, + options, + true, + this.environment.config, + )) ) { return res } @@ -410,6 +416,7 @@ export function resolvePlugin( importer, options, false, + this.environment.config, external, )) ) { @@ -417,14 +424,21 @@ export function resolvePlugin( } if ( - (res = tryNodeResolve(id, importer, options, depsOptimizer, external)) + (res = tryNodeResolve( + id, + importer, + options, + depsOptimizer, + external, + this.environment.config, + )) ) { return res } - // node built-ins. - // externalize if building for a node compatible environment, otherwise redirect to empty module - if (isBuiltin(id)) { + // built-ins + // externalize if building for a server environment, otherwise redirect to an empty module + if (isBuiltin(id, this.environment.config)) { if (currentEnvironmentOptions.consumer === 'server') { if ( options.enableBuiltinNoExternalCheck && @@ -433,7 +447,7 @@ export function resolvePlugin( // only if the id is explicitly listed in external, we will externalize it and skip this error. (options.external === true || !options.external.includes(id)) ) { - let message = `Cannot bundle Node.js built-in "${id}"` + let message = `Cannot bundle built-in module "${id}"` if (importer) { message += ` imported from "${path.relative( process.cwd(), @@ -697,6 +711,7 @@ export function tryNodeResolve( options: InternalResolveOptions, depsOptimizer?: DepsOptimizer, externalize?: boolean, + environmentOptions?: EnvironmentOptions, ): PartialResolvedId | undefined { const { root, dedupe, isBuild, preserveSymlinks, packageCache } = options @@ -721,7 +736,11 @@ export function tryNodeResolve( } let selfPkg = null - if (!isBuiltin(id) && !id.includes('\0') && bareImportRE.test(id)) { + if ( + !isBuiltin(id, environmentOptions) && + !id.includes('\0') && + bareImportRE.test(id) + ) { // check if it's a self reference dep. const selfPackageData = findNearestPackageData(basedir, packageCache) selfPkg = @@ -738,7 +757,7 @@ export function tryNodeResolve( // if so, we can resolve to a special id that errors only when imported. if ( basedir !== root && // root has no peer dep - !isBuiltin(id) && + !isBuiltin(id, environmentOptions) && !id.includes('\0') && bareImportRE.test(id) ) { @@ -1082,6 +1101,7 @@ function tryResolveBrowserMapping( importer: string | undefined, options: InternalResolveOptions, isFilePath: boolean, + environmentOptions: EnvironmentOptions, externalize?: boolean, ) { let res: string | undefined @@ -1100,6 +1120,7 @@ function tryResolveBrowserMapping( options, undefined, undefined, + environmentOptions, )?.id : tryFsResolve(path.join(pkg.dir, browserMappedPath), options)) ) { diff --git a/packages/vite/src/node/ssr/fetchModule.ts b/packages/vite/src/node/ssr/fetchModule.ts index d3851b37e7053c..a54997d725bceb 100644 --- a/packages/vite/src/node/ssr/fetchModule.ts +++ b/packages/vite/src/node/ssr/fetchModule.ts @@ -28,8 +28,7 @@ export async function fetchModule( importer?: string, options: FetchModuleOptions = {}, ): Promise { - // builtins should always be externalized - if (url.startsWith('data:') || isBuiltin(url)) { + if (url.startsWith('data:') || isBuiltin(url, environment.config)) { return { externalize: url, type: 'builtin' } } diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index 794a333276782f..adfc8cacae384f 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -46,7 +46,7 @@ import { findNearestPackageData, resolvePackageData, } from './packages' -import type { CommonServerOptions } from '.' +import type { CommonServerOptions, EnvironmentOptions } from '.' /** * Inlined to keep `@rollup/pluginutils` in devDependencies @@ -102,8 +102,18 @@ const BUN_BUILTIN_NAMESPACE = 'bun:' // Some runtimes like Bun injects namespaced modules here, which is not a node builtin const nodeBuiltins = builtinModules.filter((id) => !id.includes(':')) -// TODO: Use `isBuiltin` from `node:module`, but Deno doesn't support it -export function isBuiltin(id: string): boolean { +// TODO: For the default node implementation use `isBuiltin` from `node:module`. +// Note that Deno doesn't support it. +export function isBuiltin( + id: string, + environmentOptions?: EnvironmentOptions, +): boolean { + if (environmentOptions?.isBuiltin) { + return environmentOptions.isBuiltin(id) + } + + // TODO: Deprecate/remove the following ifs and let the environment + // tell vite what builtins it has (via the `isBuiltin` option) if (process.versions.deno && id.startsWith(NPM_BUILTIN_NAMESPACE)) return true if (process.versions.bun && id.startsWith(BUN_BUILTIN_NAMESPACE)) return true return isNodeBuiltin(id) From 298e0c12855f14bdb7453de8721a7de45c321773 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 6 Nov 2024 00:23:10 +0000 Subject: [PATCH 02/21] improve code as suggested in PR --- packages/vite/src/node/config.ts | 16 ++++++++-------- packages/vite/src/node/external.ts | 3 +-- .../src/node/optimizer/esbuildDepPlugin.ts | 3 +-- .../vite/src/node/plugins/importAnalysis.ts | 3 +-- packages/vite/src/node/plugins/resolve.ts | 19 +++++++++++-------- packages/vite/src/node/ssr/fetchModule.ts | 5 +++-- packages/vite/src/node/utils.ts | 13 ++----------- 7 files changed, 27 insertions(+), 35 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index aa63d6f3edb2d2..017ea6bcd1b55b 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -61,11 +61,11 @@ import { asyncFlatten, createDebugger, createFilter, - isBuiltin, isExternalUrl, isFilePathESM, isInNodeModules, isNodeBuiltin, + isNodeLikeBuiltin, isObject, isParentDirectory, mergeAlias, @@ -260,11 +260,6 @@ export interface SharedEnvironmentOptions { * Optimize deps config */ optimizeDeps?: DepOptimizationOptions - /** - * Function to specify when modules should be considered built-in for the environment. - * (If not provided the node built-in modules are the only ones assumed as such) - */ - isBuiltin?: (id: string) => boolean } export interface EnvironmentOptions extends SharedEnvironmentOptions { @@ -778,7 +773,7 @@ function resolveEnvironmentOptions( resolve.preserveSymlinks, consumer, ), - isBuiltin: options.isBuiltin, + isBuiltin: resolve.isBuiltin, dev: resolveDevEnvironmentOptions( options.dev, environmentName, @@ -889,6 +884,10 @@ function resolveEnvironmentResolveOptions( ? DEFAULT_CLIENT_CONDITIONS : DEFAULT_SERVER_CONDITIONS.filter((c) => c !== 'browser'), enableBuiltinNoExternalCheck: !!isSsrTargetWebworkerEnvironment, + isBuiltin: + // Note: even client environments get the node-like isBuiltin + // utility since that is necessary for prerendering + resolve?.isBuiltin ?? isNodeLikeBuiltin, }, resolve ?? {}, ) @@ -1760,6 +1759,7 @@ async function bundleConfigFile( preserveSymlinks: false, packageCache, isRequire, + isBuiltin: isNodeLikeBuiltin, })?.id } @@ -1778,7 +1778,7 @@ async function bundleConfigFile( // With the `isNodeBuiltin` check above, this check captures if the builtin is a // non-node built-in, which esbuild doesn't know how to handle. In that case, we // externalize it so the non-node runtime handles it instead. - if (isBuiltin(id)) { + if (isNodeLikeBuiltin(id)) { return { external: true } } diff --git a/packages/vite/src/node/external.ts b/packages/vite/src/node/external.ts index 57c8df6d37670d..18257ad1a613cb 100644 --- a/packages/vite/src/node/external.ts +++ b/packages/vite/src/node/external.ts @@ -6,7 +6,6 @@ import { createDebugger, createFilter, getNpmPackageName, - isBuiltin, isInNodeModules, } from './utils' import type { Environment } from './environment' @@ -157,7 +156,7 @@ function createIsExternal( let isExternal = false if (id[0] !== '.' && !path.isAbsolute(id)) { isExternal = - isBuiltin(id, environment.config) || + environment.config.resolve.isBuiltin(id) || isConfiguredAsExternal(id, importer) } processedIds.set(id, isExternal) diff --git a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts index c1fde0fb1bdbe7..f4e5294f208217 100644 --- a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts +++ b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts @@ -5,7 +5,6 @@ import type { PackageCache } from '../packages' import { escapeRegex, flattenId, - isBuiltin, isExternalUrl, moduleListContains, normalizePath, @@ -117,7 +116,7 @@ export function esbuildDepPlugin( } if ( environment.config.consumer === 'server' && - isBuiltin(resolved, environment.config) + environment.config.resolve.isBuiltin(resolved) ) { return } diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts index 5f37c54b48d35b..04240b2d183892 100644 --- a/packages/vite/src/node/plugins/importAnalysis.ts +++ b/packages/vite/src/node/plugins/importAnalysis.ts @@ -35,7 +35,6 @@ import { generateCodeFrame, getHash, injectQuery, - isBuiltin, isDataUrl, isDefined, isExternalUrl, @@ -519,7 +518,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin { if (shouldExternalize(environment, specifier, importer)) { return } - if (isBuiltin(specifier, environment.config)) { + if (environment.config.resolve.isBuiltin(specifier)) { return } } diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 30588ffe3bef2c..966a7cb17e8a49 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -21,10 +21,10 @@ import { fsPathFromId, getNpmPackageName, injectQuery, - isBuiltin, isDataUrl, isExternalUrl, isInNodeModules, + isNodeLikeBuiltin, isNonDriveRelativeAbsolutePath, isObject, isOptimizable, @@ -100,6 +100,11 @@ export interface EnvironmentResolveOptions { * @internal */ enableBuiltinNoExternalCheck?: boolean + /** + * Function to specify when modules should be considered built-in for the environment. + * (If not provided the node built-in modules are the only ones assumed as such) + */ + isBuiltin?: (id: string) => boolean } export interface ResolveOptions extends EnvironmentResolveOptions { @@ -438,7 +443,7 @@ export function resolvePlugin( // built-ins // externalize if building for a server environment, otherwise redirect to an empty module - if (isBuiltin(id, this.environment.config)) { + if (this.environment.config.resolve.isBuiltin(id)) { if (currentEnvironmentOptions.consumer === 'server') { if ( options.enableBuiltinNoExternalCheck && @@ -735,12 +740,10 @@ export function tryNodeResolve( basedir = root } + const isBuiltin = environmentOptions?.resolve?.isBuiltin ?? isNodeLikeBuiltin + let selfPkg = null - if ( - !isBuiltin(id, environmentOptions) && - !id.includes('\0') && - bareImportRE.test(id) - ) { + if (!isBuiltin(id) && !id.includes('\0') && bareImportRE.test(id)) { // check if it's a self reference dep. const selfPackageData = findNearestPackageData(basedir, packageCache) selfPkg = @@ -757,7 +760,7 @@ export function tryNodeResolve( // if so, we can resolve to a special id that errors only when imported. if ( basedir !== root && // root has no peer dep - !isBuiltin(id, environmentOptions) && + !isBuiltin(id) && !id.includes('\0') && bareImportRE.test(id) ) { diff --git a/packages/vite/src/node/ssr/fetchModule.ts b/packages/vite/src/node/ssr/fetchModule.ts index a54997d725bceb..c31e7bd9f4ab09 100644 --- a/packages/vite/src/node/ssr/fetchModule.ts +++ b/packages/vite/src/node/ssr/fetchModule.ts @@ -2,7 +2,7 @@ import { pathToFileURL } from 'node:url' import type { FetchResult } from 'vite/module-runner' import type { EnvironmentModuleNode, TransformResult } from '..' import { tryNodeResolve } from '../plugins/resolve' -import { isBuiltin, isExternalUrl, isFilePathESM } from '../utils' +import { isExternalUrl, isFilePathESM } from '../utils' import { unwrapId } from '../../shared/utils' import { MODULE_RUNNER_SOURCEMAPPING_SOURCE, @@ -28,7 +28,7 @@ export async function fetchModule( importer?: string, options: FetchModuleOptions = {}, ): Promise { - if (url.startsWith('data:') || isBuiltin(url, environment.config)) { + if (url.startsWith('data:') || environment.config.resolve.isBuiltin(url)) { return { externalize: url, type: 'builtin' } } @@ -56,6 +56,7 @@ export async function fetchModule( isProduction, root, packageCache: environment.config.packageCache, + isBuiltin: environment.config.resolve.isBuiltin, }) if (!resolved) { const err: any = new Error( diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index adfc8cacae384f..b390f340de6768 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -46,7 +46,7 @@ import { findNearestPackageData, resolvePackageData, } from './packages' -import type { CommonServerOptions, EnvironmentOptions } from '.' +import type { CommonServerOptions } from '.' /** * Inlined to keep `@rollup/pluginutils` in devDependencies @@ -104,16 +104,7 @@ const nodeBuiltins = builtinModules.filter((id) => !id.includes(':')) // TODO: For the default node implementation use `isBuiltin` from `node:module`. // Note that Deno doesn't support it. -export function isBuiltin( - id: string, - environmentOptions?: EnvironmentOptions, -): boolean { - if (environmentOptions?.isBuiltin) { - return environmentOptions.isBuiltin(id) - } - - // TODO: Deprecate/remove the following ifs and let the environment - // tell vite what builtins it has (via the `isBuiltin` option) +export function isNodeLikeBuiltin(id: string): boolean { if (process.versions.deno && id.startsWith(NPM_BUILTIN_NAMESPACE)) return true if (process.versions.bun && id.startsWith(BUN_BUILTIN_NAMESPACE)) return true return isNodeBuiltin(id) From 63866c998dd75eebc3b6de631a1489a59b9d5048 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 6 Nov 2024 00:44:30 +0000 Subject: [PATCH 03/21] polish code further --- packages/vite/src/node/config.ts | 8 +- .../src/node/optimizer/esbuildDepPlugin.ts | 5 +- packages/vite/src/node/plugins/resolve.ts | 74 ++++++++++--------- 3 files changed, 44 insertions(+), 43 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 017ea6bcd1b55b..36f433a2b9b44d 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -885,9 +885,11 @@ function resolveEnvironmentResolveOptions( : DEFAULT_SERVER_CONDITIONS.filter((c) => c !== 'browser'), enableBuiltinNoExternalCheck: !!isSsrTargetWebworkerEnvironment, isBuiltin: - // Note: even client environments get the node-like isBuiltin - // utility since that is necessary for prerendering - resolve?.isBuiltin ?? isNodeLikeBuiltin, + resolve?.isBuiltin ?? + (consumer === 'server' + ? isNodeLikeBuiltin + : // there are not built-in modules in the browser + () => false), }, resolve ?? {}, ) diff --git a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts index f4e5294f208217..767d8822866ebe 100644 --- a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts +++ b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts @@ -114,10 +114,7 @@ export function esbuildDepPlugin( namespace: 'optional-peer-dep', } } - if ( - environment.config.consumer === 'server' && - environment.config.resolve.isBuiltin(resolved) - ) { + if (environment.config.resolve.isBuiltin(resolved)) { return } if (isExternalUrl(resolved)) { diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 966a7cb17e8a49..7eea1fab6039a4 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -443,45 +443,47 @@ export function resolvePlugin( // built-ins // externalize if building for a server environment, otherwise redirect to an empty module - if (this.environment.config.resolve.isBuiltin(id)) { - if (currentEnvironmentOptions.consumer === 'server') { - if ( - options.enableBuiltinNoExternalCheck && - options.noExternal === true && - // if both noExternal and external are true, noExternal will take the higher priority and bundle it. - // only if the id is explicitly listed in external, we will externalize it and skip this error. - (options.external === true || !options.external.includes(id)) - ) { - let message = `Cannot bundle built-in module "${id}"` - if (importer) { - message += ` imported from "${path.relative( - process.cwd(), - importer, - )}"` - } - message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.` - this.error(message) + if ( + currentEnvironmentOptions.consumer === 'server' && + this.environment.config.resolve.isBuiltin(id) + ) { + if ( + options.enableBuiltinNoExternalCheck && + options.noExternal === true && + // if both noExternal and external are true, noExternal will take the higher priority and bundle it. + // only if the id is explicitly listed in external, we will externalize it and skip this error. + (options.external === true || !options.external.includes(id)) + ) { + let message = `Cannot bundle built-in module "${id}"` + if (importer) { + message += ` imported from "${path.relative( + process.cwd(), + importer, + )}"` } + message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.` + this.error(message) + } - return options.idOnly - ? id - : { id, external: true, moduleSideEffects: false } - } else { - if (!asSrc) { - debug?.( - `externalized node built-in "${id}" to empty module. ` + - `(imported by: ${colors.white(colors.dim(importer))})`, - ) - } else if (isProduction) { - this.warn( - `Module "${id}" has been externalized for browser compatibility, imported by "${importer}". ` + - `See https://vite.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.`, - ) - } - return isProduction - ? browserExternalId - : `${browserExternalId}:${id}` + return options.idOnly + ? id + : { id, external: true, moduleSideEffects: false } + } else if ( + currentEnvironmentOptions.consumer === 'client' && + isNodeLikeBuiltin(id) + ) { + if (!asSrc) { + debug?.( + `externalized node built-in "${id}" to empty module. ` + + `(imported by: ${colors.white(colors.dim(importer))})`, + ) + } else if (isProduction) { + this.warn( + `Module "${id}" has been externalized for browser compatibility, imported by "${importer}". ` + + `See https://vite.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.`, + ) } + return isProduction ? browserExternalId : `${browserExternalId}:${id}` } } From 4712b5fe466cf8512a07fb6b3e84dd8efb525922 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 6 Nov 2024 00:47:15 +0000 Subject: [PATCH 04/21] improve tsdoc comment --- packages/vite/src/node/plugins/resolve.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 7eea1fab6039a4..786edf66830cc9 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -101,7 +101,7 @@ export interface EnvironmentResolveOptions { */ enableBuiltinNoExternalCheck?: boolean /** - * Function to specify when modules should be considered built-in for the environment. + * Function to discern when a module should be considered as built-in for the environment. * (If not provided the node built-in modules are the only ones assumed as such) */ isBuiltin?: (id: string) => boolean From 98723a4de8bd60ee53f26379cdcf075e5e119d51 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 6 Nov 2024 00:49:07 +0000 Subject: [PATCH 05/21] revert comment change --- packages/vite/src/node/utils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index b390f340de6768..214a01f89bd1c3 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -102,8 +102,7 @@ const BUN_BUILTIN_NAMESPACE = 'bun:' // Some runtimes like Bun injects namespaced modules here, which is not a node builtin const nodeBuiltins = builtinModules.filter((id) => !id.includes(':')) -// TODO: For the default node implementation use `isBuiltin` from `node:module`. -// Note that Deno doesn't support it. +// TODO: Use `isBuiltin` from `node:module`, but Deno doesn't support it export function isNodeLikeBuiltin(id: string): boolean { if (process.versions.deno && id.startsWith(NPM_BUILTIN_NAMESPACE)) return true if (process.versions.bun && id.startsWith(BUN_BUILTIN_NAMESPACE)) return true From fbdadfce1b1687c301882579525b6551f1ee2ee1 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 6 Nov 2024 22:56:23 +0000 Subject: [PATCH 06/21] update `resolve.isBuiltin` to `resolve.builtins` --- packages/vite/src/node/config.ts | 16 ++++---- packages/vite/src/node/external.ts | 3 +- .../src/node/optimizer/esbuildDepPlugin.ts | 3 +- .../vite/src/node/plugins/importAnalysis.ts | 3 +- packages/vite/src/node/plugins/resolve.ts | 16 ++++---- packages/vite/src/node/ssr/fetchModule.ts | 9 +++-- packages/vite/src/node/utils.ts | 40 +++++++++++++++++-- 7 files changed, 65 insertions(+), 25 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index 36f433a2b9b44d..d14569e0fcd36f 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -71,6 +71,7 @@ import { mergeAlias, mergeConfig, mergeWithDefaults, + nodeLikeBuiltins, normalizeAlias, normalizePath, } from './utils' @@ -283,7 +284,6 @@ export type ResolvedEnvironmentOptions = { optimizeDeps: DepOptimizationOptions dev: ResolvedDevEnvironmentOptions build: ResolvedBuildEnvironmentOptions - isBuiltin?: (id: string) => boolean } export type DefaultEnvironmentOptions = Omit< @@ -773,7 +773,6 @@ function resolveEnvironmentOptions( resolve.preserveSymlinks, consumer, ), - isBuiltin: resolve.isBuiltin, dev: resolveDevEnvironmentOptions( options.dev, environmentName, @@ -884,12 +883,13 @@ function resolveEnvironmentResolveOptions( ? DEFAULT_CLIENT_CONDITIONS : DEFAULT_SERVER_CONDITIONS.filter((c) => c !== 'browser'), enableBuiltinNoExternalCheck: !!isSsrTargetWebworkerEnvironment, - isBuiltin: - resolve?.isBuiltin ?? + builtins: + resolve?.builtins ?? (consumer === 'server' - ? isNodeLikeBuiltin - : // there are not built-in modules in the browser - () => false), + ? nodeLikeBuiltins + : [ + // there are not built-in modules in the browser + ]), }, resolve ?? {}, ) @@ -1761,7 +1761,7 @@ async function bundleConfigFile( preserveSymlinks: false, packageCache, isRequire, - isBuiltin: isNodeLikeBuiltin, + builtins: nodeLikeBuiltins, })?.id } diff --git a/packages/vite/src/node/external.ts b/packages/vite/src/node/external.ts index 18257ad1a613cb..e7b5247e2af400 100644 --- a/packages/vite/src/node/external.ts +++ b/packages/vite/src/node/external.ts @@ -6,6 +6,7 @@ import { createDebugger, createFilter, getNpmPackageName, + isBuiltin, isInNodeModules, } from './utils' import type { Environment } from './environment' @@ -156,7 +157,7 @@ function createIsExternal( let isExternal = false if (id[0] !== '.' && !path.isAbsolute(id)) { isExternal = - environment.config.resolve.isBuiltin(id) || + isBuiltin(environment.config.resolve.builtins, id) || isConfiguredAsExternal(id, importer) } processedIds.set(id, isExternal) diff --git a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts index 767d8822866ebe..a84c99f122b4ec 100644 --- a/packages/vite/src/node/optimizer/esbuildDepPlugin.ts +++ b/packages/vite/src/node/optimizer/esbuildDepPlugin.ts @@ -5,6 +5,7 @@ import type { PackageCache } from '../packages' import { escapeRegex, flattenId, + isBuiltin, isExternalUrl, moduleListContains, normalizePath, @@ -114,7 +115,7 @@ export function esbuildDepPlugin( namespace: 'optional-peer-dep', } } - if (environment.config.resolve.isBuiltin(resolved)) { + if (isBuiltin(environment.config.resolve.builtins, resolved)) { return } if (isExternalUrl(resolved)) { diff --git a/packages/vite/src/node/plugins/importAnalysis.ts b/packages/vite/src/node/plugins/importAnalysis.ts index 04240b2d183892..b74b4fd4d288d2 100644 --- a/packages/vite/src/node/plugins/importAnalysis.ts +++ b/packages/vite/src/node/plugins/importAnalysis.ts @@ -35,6 +35,7 @@ import { generateCodeFrame, getHash, injectQuery, + isBuiltin, isDataUrl, isDefined, isExternalUrl, @@ -518,7 +519,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin { if (shouldExternalize(environment, specifier, importer)) { return } - if (environment.config.resolve.isBuiltin(specifier)) { + if (isBuiltin(environment.config.resolve.builtins, specifier)) { return } } diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 786edf66830cc9..2894f22f8a01ab 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -21,6 +21,7 @@ import { fsPathFromId, getNpmPackageName, injectQuery, + isBuiltin, isDataUrl, isExternalUrl, isInNodeModules, @@ -29,6 +30,7 @@ import { isObject, isOptimizable, isTsRequest, + nodeLikeBuiltins, normalizePath, safeRealpathSync, tryStatSync, @@ -101,10 +103,9 @@ export interface EnvironmentResolveOptions { */ enableBuiltinNoExternalCheck?: boolean /** - * Function to discern when a module should be considered as built-in for the environment. - * (If not provided the node built-in modules are the only ones assumed as such) + * Array of strings or regular expressions that indicate what modules are builtin for the environment. */ - isBuiltin?: (id: string) => boolean + builtins?: (string | RegExp)[] } export interface ResolveOptions extends EnvironmentResolveOptions { @@ -445,7 +446,7 @@ export function resolvePlugin( // externalize if building for a server environment, otherwise redirect to an empty module if ( currentEnvironmentOptions.consumer === 'server' && - this.environment.config.resolve.isBuiltin(id) + isBuiltin(this.environment.config.resolve.builtins, id) ) { if ( options.enableBuiltinNoExternalCheck && @@ -742,10 +743,11 @@ export function tryNodeResolve( basedir = root } - const isBuiltin = environmentOptions?.resolve?.isBuiltin ?? isNodeLikeBuiltin + const isModuleBuiltin = (id: string) => + isBuiltin(environmentOptions?.resolve?.builtins ?? nodeLikeBuiltins, id) let selfPkg = null - if (!isBuiltin(id) && !id.includes('\0') && bareImportRE.test(id)) { + if (!isModuleBuiltin(id) && !id.includes('\0') && bareImportRE.test(id)) { // check if it's a self reference dep. const selfPackageData = findNearestPackageData(basedir, packageCache) selfPkg = @@ -762,7 +764,7 @@ export function tryNodeResolve( // if so, we can resolve to a special id that errors only when imported. if ( basedir !== root && // root has no peer dep - !isBuiltin(id) && + !isModuleBuiltin(id) && !id.includes('\0') && bareImportRE.test(id) ) { diff --git a/packages/vite/src/node/ssr/fetchModule.ts b/packages/vite/src/node/ssr/fetchModule.ts index c31e7bd9f4ab09..e5d40fe2934220 100644 --- a/packages/vite/src/node/ssr/fetchModule.ts +++ b/packages/vite/src/node/ssr/fetchModule.ts @@ -2,7 +2,7 @@ import { pathToFileURL } from 'node:url' import type { FetchResult } from 'vite/module-runner' import type { EnvironmentModuleNode, TransformResult } from '..' import { tryNodeResolve } from '../plugins/resolve' -import { isExternalUrl, isFilePathESM } from '../utils' +import { isBuiltin, isExternalUrl, isFilePathESM } from '../utils' import { unwrapId } from '../../shared/utils' import { MODULE_RUNNER_SOURCEMAPPING_SOURCE, @@ -28,7 +28,10 @@ export async function fetchModule( importer?: string, options: FetchModuleOptions = {}, ): Promise { - if (url.startsWith('data:') || environment.config.resolve.isBuiltin(url)) { + if ( + url.startsWith('data:') || + isBuiltin(environment.config.resolve.builtins, url) + ) { return { externalize: url, type: 'builtin' } } @@ -56,7 +59,7 @@ export async function fetchModule( isProduction, root, packageCache: environment.config.packageCache, - isBuiltin: environment.config.resolve.isBuiltin, + builtins: environment.config.resolve.builtins, }) if (!resolved) { const err: any = new Error( diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index 214a01f89bd1c3..52fa12fd4c1e51 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -102,11 +102,43 @@ const BUN_BUILTIN_NAMESPACE = 'bun:' // Some runtimes like Bun injects namespaced modules here, which is not a node builtin const nodeBuiltins = builtinModules.filter((id) => !id.includes(':')) -// TODO: Use `isBuiltin` from `node:module`, but Deno doesn't support it +const isConfiguredAsExternalCache = new WeakMap< + (string | RegExp)[], + (id: string, importer?: string) => boolean +>() + +export function isBuiltin(builtins: (string | RegExp)[], id: string): boolean { + let isBuiltin = isConfiguredAsExternalCache.get(builtins) + if (!isBuiltin) { + isBuiltin = createIsBuiltin(builtins) + isConfiguredAsExternalCache.set(builtins, isBuiltin) + } + return isBuiltin(id) +} + +export function createIsBuiltin( + builtins: (string | RegExp)[], +): (id: string) => boolean { + const plainBuiltinsSet = new Set( + builtins.filter((builtin) => typeof builtin === 'string'), + ) + const regexBuiltins = builtins.filter( + (builtin) => typeof builtin !== 'string', + ) + + return (id) => + plainBuiltinsSet.has(id) || regexBuiltins.some((regexp) => regexp.test(id)) +} + +export const nodeLikeBuiltins = [ + ...nodeBuiltins, + new RegExp(`^${NODE_BUILTIN_NAMESPACE}`), + new RegExp(`^${NPM_BUILTIN_NAMESPACE}`), + new RegExp(`^${BUN_BUILTIN_NAMESPACE}`), +] + export function isNodeLikeBuiltin(id: string): boolean { - if (process.versions.deno && id.startsWith(NPM_BUILTIN_NAMESPACE)) return true - if (process.versions.bun && id.startsWith(BUN_BUILTIN_NAMESPACE)) return true - return isNodeBuiltin(id) + return isBuiltin(nodeLikeBuiltins, id) } export function isNodeBuiltin(id: string): boolean { From 8c0bc0a0624148b9d99924c6410f07a3ad25b5dd Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 6 Nov 2024 23:01:08 +0000 Subject: [PATCH 07/21] fix copy-pasta --- packages/vite/src/node/utils.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/vite/src/node/utils.ts b/packages/vite/src/node/utils.ts index 52fa12fd4c1e51..f2d84b2d3f5490 100644 --- a/packages/vite/src/node/utils.ts +++ b/packages/vite/src/node/utils.ts @@ -102,16 +102,16 @@ const BUN_BUILTIN_NAMESPACE = 'bun:' // Some runtimes like Bun injects namespaced modules here, which is not a node builtin const nodeBuiltins = builtinModules.filter((id) => !id.includes(':')) -const isConfiguredAsExternalCache = new WeakMap< +const isBuiltinCache = new WeakMap< (string | RegExp)[], (id: string, importer?: string) => boolean >() export function isBuiltin(builtins: (string | RegExp)[], id: string): boolean { - let isBuiltin = isConfiguredAsExternalCache.get(builtins) + let isBuiltin = isBuiltinCache.get(builtins) if (!isBuiltin) { isBuiltin = createIsBuiltin(builtins) - isConfiguredAsExternalCache.set(builtins, isBuiltin) + isBuiltinCache.set(builtins, isBuiltin) } return isBuiltin(id) } From 0861cb2b242f3164647208a8137b2eb2036c2c35 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 3 Dec 2024 11:16:30 +0000 Subject: [PATCH 08/21] avoid passing `this.environment.config` everywhere --- packages/vite/src/node/plugins/resolve.ts | 27 +++++------------------ 1 file changed, 5 insertions(+), 22 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 2894f22f8a01ab..e2310f4d54937a 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -37,7 +37,7 @@ import { } from '../utils' import { optimizedDepInfoFromFile, optimizedDepInfoFromId } from '../optimizer' import type { DepsOptimizer } from '../optimizer' -import type { EnvironmentOptions, SSROptions } from '..' +import type { SSROptions } from '..' import type { PackageCache, PackageData } from '../packages' import { canExternalizeFile, shouldExternalize } from '../external' import { @@ -326,13 +326,7 @@ export function resolvePlugin( if ( options.mainFields.includes('browser') && - (res = tryResolveBrowserMapping( - fsPath, - importer, - options, - true, - this.environment.config, - )) + (res = tryResolveBrowserMapping(fsPath, importer, options, true)) ) { return res } @@ -422,7 +416,6 @@ export function resolvePlugin( importer, options, false, - this.environment.config, external, )) ) { @@ -430,14 +423,7 @@ export function resolvePlugin( } if ( - (res = tryNodeResolve( - id, - importer, - options, - depsOptimizer, - external, - this.environment.config, - )) + (res = tryNodeResolve(id, importer, options, depsOptimizer, external)) ) { return res } @@ -446,7 +432,7 @@ export function resolvePlugin( // externalize if building for a server environment, otherwise redirect to an empty module if ( currentEnvironmentOptions.consumer === 'server' && - isBuiltin(this.environment.config.resolve.builtins, id) + isBuiltin(options.builtins, id) ) { if ( options.enableBuiltinNoExternalCheck && @@ -719,7 +705,6 @@ export function tryNodeResolve( options: InternalResolveOptions, depsOptimizer?: DepsOptimizer, externalize?: boolean, - environmentOptions?: EnvironmentOptions, ): PartialResolvedId | undefined { const { root, dedupe, isBuild, preserveSymlinks, packageCache } = options @@ -744,7 +729,7 @@ export function tryNodeResolve( } const isModuleBuiltin = (id: string) => - isBuiltin(environmentOptions?.resolve?.builtins ?? nodeLikeBuiltins, id) + isBuiltin(options?.builtins ?? nodeLikeBuiltins, id) let selfPkg = null if (!isModuleBuiltin(id) && !id.includes('\0') && bareImportRE.test(id)) { @@ -1108,7 +1093,6 @@ function tryResolveBrowserMapping( importer: string | undefined, options: InternalResolveOptions, isFilePath: boolean, - environmentOptions: EnvironmentOptions, externalize?: boolean, ) { let res: string | undefined @@ -1127,7 +1111,6 @@ function tryResolveBrowserMapping( options, undefined, undefined, - environmentOptions, )?.id : tryFsResolve(path.join(pkg.dir, browserMappedPath), options)) ) { From d2a05d5407dfc00f427aaf80ed0441c02dc0dd28 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 3 Dec 2024 12:03:21 +0000 Subject: [PATCH 09/21] Update packages/vite/src/node/plugins/resolve.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 翠 / green --- packages/vite/src/node/plugins/resolve.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index e2310f4d54937a..1260d545a478d1 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -456,6 +456,26 @@ export function resolvePlugin( ? id : { id, external: true, moduleSideEffects: false } } else if ( + currentEnvironmentOptions.consumer === 'server' && + isNodeLikeBuiltin(id) + ) { + if ( + options.noExternal === true && + // if both noExternal and external are true, noExternal will take the higher priority and bundle it. + // only if the id is explicitly listed in external, we will externalize it and skip this error. + (options.external === true || !options.external.includes(id)) + ) { + let message = `Cannot bundle node built-in module "${id}"` + if (importer) { + message += ` imported from "${path.relative( + process.cwd(), + importer, + )}"` + } + message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.` + this.error(message) + } + } if ( currentEnvironmentOptions.consumer === 'client' && isNodeLikeBuiltin(id) ) { From e39c3a21eebb3df72b35c126145f0e785a788b3a Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 3 Dec 2024 12:04:49 +0000 Subject: [PATCH 10/21] add missing `else` --- packages/vite/src/node/plugins/resolve.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 1260d545a478d1..93b6d6f6a3eaef 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -475,7 +475,7 @@ export function resolvePlugin( message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.` this.error(message) } - } if ( + } else if ( currentEnvironmentOptions.consumer === 'client' && isNodeLikeBuiltin(id) ) { From ad579bf4d7947e2e2b238cfc3f2368d1cbc6ee4d Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 3 Dec 2024 12:23:22 +0000 Subject: [PATCH 11/21] remove incorrect argument --- packages/vite/src/node/external.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/vite/src/node/external.ts b/packages/vite/src/node/external.ts index e7b5247e2af400..f0777188212be0 100644 --- a/packages/vite/src/node/external.ts +++ b/packages/vite/src/node/external.ts @@ -88,7 +88,6 @@ export function createIsConfiguredAsExternal( resolveOptions, undefined, false, - environment.config, ) if (!resolved) { return false From d1f1ca03e3a1ddfcd334171e83b3f85562dc79c9 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Tue, 3 Dec 2024 15:02:38 +0000 Subject: [PATCH 12/21] remove `enableBuiltinNoExternalCheck` as suggested in PR review --- packages/vite/src/node/config.ts | 7 ++--- packages/vite/src/node/plugins/resolve.ts | 34 +++++++++-------------- 2 files changed, 15 insertions(+), 26 deletions(-) diff --git a/packages/vite/src/node/config.ts b/packages/vite/src/node/config.ts index d14569e0fcd36f..744754285c9f43 100644 --- a/packages/vite/src/node/config.ts +++ b/packages/vite/src/node/config.ts @@ -882,14 +882,11 @@ function resolveEnvironmentResolveOptions( consumer === 'client' || isSsrTargetWebworkerEnvironment ? DEFAULT_CLIENT_CONDITIONS : DEFAULT_SERVER_CONDITIONS.filter((c) => c !== 'browser'), - enableBuiltinNoExternalCheck: !!isSsrTargetWebworkerEnvironment, builtins: resolve?.builtins ?? - (consumer === 'server' + (consumer === 'server' && !isSsrTargetWebworkerEnvironment ? nodeLikeBuiltins - : [ - // there are not built-in modules in the browser - ]), + : []), }, resolve ?? {}, ) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 93b6d6f6a3eaef..322c5853ac9a74 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -98,10 +98,6 @@ export interface EnvironmentResolveOptions { * @experimental */ external?: string[] | true - /** - * @internal - */ - enableBuiltinNoExternalCheck?: boolean /** * Array of strings or regular expressions that indicate what modules are builtin for the environment. */ @@ -179,11 +175,8 @@ interface ResolvePluginOptions { } export interface InternalResolveOptions - extends Required>, - ResolvePluginOptions { - /** @internal this is always optional for backward compat */ - enableBuiltinNoExternalCheck?: boolean -} + extends Required, + ResolvePluginOptions {} // Defined ResolveOptions are used to overwrite the values for all environments // It is used when creating custom resolvers (for CSS, scanning, etc) @@ -433,15 +426,21 @@ export function resolvePlugin( if ( currentEnvironmentOptions.consumer === 'server' && isBuiltin(options.builtins, id) + ) { + return options.idOnly + ? id + : { id, external: true, moduleSideEffects: false } + } else if ( + currentEnvironmentOptions.consumer === 'server' && + isNodeLikeBuiltin(id) ) { if ( - options.enableBuiltinNoExternalCheck && options.noExternal === true && // if both noExternal and external are true, noExternal will take the higher priority and bundle it. // only if the id is explicitly listed in external, we will externalize it and skip this error. (options.external === true || !options.external.includes(id)) ) { - let message = `Cannot bundle built-in module "${id}"` + let message = `Cannot bundle node built-in module "${id}"` if (importer) { message += ` imported from "${path.relative( process.cwd(), @@ -451,12 +450,8 @@ export function resolvePlugin( message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.` this.error(message) } - - return options.idOnly - ? id - : { id, external: true, moduleSideEffects: false } } else if ( - currentEnvironmentOptions.consumer === 'server' && + currentEnvironmentOptions.consumer === 'client' && isNodeLikeBuiltin(id) ) { if ( @@ -465,7 +460,7 @@ export function resolvePlugin( // only if the id is explicitly listed in external, we will externalize it and skip this error. (options.external === true || !options.external.includes(id)) ) { - let message = `Cannot bundle node built-in module "${id}"` + let message = `Cannot bundle built-in module "${id}"` if (importer) { message += ` imported from "${path.relative( process.cwd(), @@ -475,10 +470,7 @@ export function resolvePlugin( message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.` this.error(message) } - } else if ( - currentEnvironmentOptions.consumer === 'client' && - isNodeLikeBuiltin(id) - ) { + if (!asSrc) { debug?.( `externalized node built-in "${id}" to empty module. ` + From ae1ed5d672464958d5fe4d2bae8d72421e4310b4 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 4 Dec 2024 11:09:19 +0000 Subject: [PATCH 13/21] Update packages/vite/src/node/plugins/resolve.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 翠 / green --- packages/vite/src/node/plugins/resolve.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 322c5853ac9a74..3344887d426fa9 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -450,6 +450,21 @@ export function resolvePlugin( message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.` this.error(message) } + if (!(options.external === true || options.external.includes(id))) { + let message = `Automatically externalized node built-in module "${id}"` + if (importer) { + message += ` imported from "${path.relative( + process.cwd(), + importer, + )}"` + } + message += `. Consider adding it to environments.${this.environment.name}.external if it is intended.` + this.error(message) + } + + return options.idOnly + ? id + : { id, external: true, moduleSideEffects: false } } else if ( currentEnvironmentOptions.consumer === 'client' && isNodeLikeBuiltin(id) From 5666f673e596f526832bcbb6012fc0ad323f3284 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 4 Dec 2024 11:10:10 +0000 Subject: [PATCH 14/21] Update packages/vite/src/node/plugins/resolve.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: 翠 / green --- packages/vite/src/node/plugins/resolve.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 3344887d426fa9..4bf650aa7c0b34 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -756,7 +756,7 @@ export function tryNodeResolve( } const isModuleBuiltin = (id: string) => - isBuiltin(options?.builtins ?? nodeLikeBuiltins, id) + isBuiltin(options.builtins, id) let selfPkg = null if (!isModuleBuiltin(id) && !id.includes('\0') && bareImportRE.test(id)) { From 8181eeb2b4bfe4a75792eac94e78bc1c73821425 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 4 Dec 2024 12:54:44 +0000 Subject: [PATCH 15/21] add test for env builtins in dev --- .../src/node/__tests__/environment.spec.ts | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/packages/vite/src/node/__tests__/environment.spec.ts b/packages/vite/src/node/__tests__/environment.spec.ts index 75b12136c5cd2d..89f6a905daa630 100644 --- a/packages/vite/src/node/__tests__/environment.spec.ts +++ b/packages/vite/src/node/__tests__/environment.spec.ts @@ -59,6 +59,7 @@ describe('custom environment conditions', () => { noExternal, conditions: ['custom1'], externalConditions: ['custom1'], + builtins: ['my-env:custom-builtin'], }, build: { outDir: path.join( @@ -76,6 +77,7 @@ describe('custom environment conditions', () => { noExternal, conditions: ['custom1'], externalConditions: ['custom1'], + builtins: ['my-env:custom-builtin'], }, build: { outDir: path.join( @@ -143,6 +145,27 @@ describe('custom environment conditions', () => { `) }) + test('dev builtins', async () => { + const server = await createServer(getConfig({ noExternal: true })) + onTestFinished(() => server.close()) + + const results: Record = {} + for (const key of ['ssr', 'worker', 'custom1', 'custom1_2']) { + const resolved = await server.environments[key].pluginContainer.resolveId( + 'my-env:custom-builtin', + ) + results[key] = JSON.stringify(resolved) + } + expect(results).toMatchInlineSnapshot(` + { + "custom1": "{"id":"my-env:custom-builtin","external":true,"moduleSideEffects":false}", + "custom1_2": "{"id":"my-env:custom-builtin","external":true,"moduleSideEffects":false}", + "ssr": "null", + "worker": "null", + } + `) + }) + test('css', async () => { const server = await createServer(getConfig({ noExternal: true })) onTestFinished(() => server.close()) From 1baf5c8b6dd8548dc2b57104b115b8c94677672d Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 4 Dec 2024 12:57:55 +0000 Subject: [PATCH 16/21] remove unused import --- packages/vite/src/node/plugins/resolve.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index 4bf650aa7c0b34..e7ce14bc566367 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -30,7 +30,6 @@ import { isObject, isOptimizable, isTsRequest, - nodeLikeBuiltins, normalizePath, safeRealpathSync, tryStatSync, @@ -461,7 +460,7 @@ export function resolvePlugin( message += `. Consider adding it to environments.${this.environment.name}.external if it is intended.` this.error(message) } - + return options.idOnly ? id : { id, external: true, moduleSideEffects: false } @@ -755,8 +754,7 @@ export function tryNodeResolve( basedir = root } - const isModuleBuiltin = (id: string) => - isBuiltin(options.builtins, id) + const isModuleBuiltin = (id: string) => isBuiltin(options.builtins, id) let selfPkg = null if (!isModuleBuiltin(id) && !id.includes('\0') && bareImportRE.test(id)) { From 0b7d83f72d9d713302a16a59074937da39f50493 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 4 Dec 2024 13:27:47 +0000 Subject: [PATCH 17/21] improve tests --- .../src/node/__tests__/environment.spec.ts | 23 ------ .../vite/src/node/__tests__/resolve.spec.ts | 80 ++++++++++++++++++- 2 files changed, 79 insertions(+), 24 deletions(-) diff --git a/packages/vite/src/node/__tests__/environment.spec.ts b/packages/vite/src/node/__tests__/environment.spec.ts index 89f6a905daa630..75b12136c5cd2d 100644 --- a/packages/vite/src/node/__tests__/environment.spec.ts +++ b/packages/vite/src/node/__tests__/environment.spec.ts @@ -59,7 +59,6 @@ describe('custom environment conditions', () => { noExternal, conditions: ['custom1'], externalConditions: ['custom1'], - builtins: ['my-env:custom-builtin'], }, build: { outDir: path.join( @@ -77,7 +76,6 @@ describe('custom environment conditions', () => { noExternal, conditions: ['custom1'], externalConditions: ['custom1'], - builtins: ['my-env:custom-builtin'], }, build: { outDir: path.join( @@ -145,27 +143,6 @@ describe('custom environment conditions', () => { `) }) - test('dev builtins', async () => { - const server = await createServer(getConfig({ noExternal: true })) - onTestFinished(() => server.close()) - - const results: Record = {} - for (const key of ['ssr', 'worker', 'custom1', 'custom1_2']) { - const resolved = await server.environments[key].pluginContainer.resolveId( - 'my-env:custom-builtin', - ) - results[key] = JSON.stringify(resolved) - } - expect(results).toMatchInlineSnapshot(` - { - "custom1": "{"id":"my-env:custom-builtin","external":true,"moduleSideEffects":false}", - "custom1_2": "{"id":"my-env:custom-builtin","external":true,"moduleSideEffects":false}", - "ssr": "null", - "worker": "null", - } - `) - }) - test('css', async () => { const server = await createServer(getConfig({ noExternal: true })) onTestFinished(() => server.close()) diff --git a/packages/vite/src/node/__tests__/resolve.spec.ts b/packages/vite/src/node/__tests__/resolve.spec.ts index 111cdb99a6b1a0..2287e00bf8beb1 100644 --- a/packages/vite/src/node/__tests__/resolve.spec.ts +++ b/packages/vite/src/node/__tests__/resolve.spec.ts @@ -2,7 +2,7 @@ import { join } from 'node:path' import { describe, expect, onTestFinished, test } from 'vitest' import { createServer } from '../server' import { createServerModuleRunner } from '../ssr/runtime/serverModuleRunner' -import type { InlineConfig } from '../config' +import type { EnvironmentOptions, InlineConfig } from '../config' import { build } from '../build' describe('import and resolveId', () => { @@ -116,6 +116,84 @@ describe('file url', () => { expect(mod4.default).toBe(mod) }) + describe('environment builtins', () => { + function getConfig( + customEnvBuiltins: NonNullable['builtins'], + ): InlineConfig { + return { + configFile: false, + root: join(import.meta.dirname, 'fixtures/file-url'), + logLevel: 'error', + server: { + middlewareMode: true, + }, + environments: { + custom: { + resolve: { + builtins: customEnvBuiltins, + }, + }, + }, + } + } + + async function run( + customEnvBuiltins: NonNullable['builtins'], + idToResolve: string, + envName = 'custom', + ) { + const server = await createServer(getConfig(customEnvBuiltins)) + onTestFinished(() => server.close()) + + return server.environments[envName]?.pluginContainer.resolveId( + idToResolve, + ) + } + + test('declared builtin string', async () => { + const resolved = await run( + ['my-env:custom-builtin'], + 'my-env:custom-builtin', + ) + expect(resolved?.external).toBe(true) + }) + + test('declared builtin regexp', async () => { + const resolved = await run([/^my-env:\w/], 'my-env:custom-builtin') + expect(resolved?.external).toBe(true) + }) + + test('non declared builtin', async () => { + const resolved = await run([], 'my-env:custom-builtin') + expect(resolved).toBeNull() + }) + + test('non declared node builtin', async () => { + await expect(run([], 'node:fs')).rejects.toThrowError( + /Automatically externalized node built-in module "node:fs"/, + ) + }) + + test('default to node-like builtins', async () => { + const resolved = await run(undefined, 'node:fs') + expect(resolved?.external).toBe(true) + }) + + test('declared node builtin', async () => { + const resolved = await run([/^node:/], 'node:fs') + expect(resolved?.external).toBe(true) + }) + + test('declared builtin string in different environment', async () => { + const resolved = await run( + ['my-env:custom-builtin'], + 'my-env:custom-builtin', + 'ssr', + ) + expect(resolved).toBe(null) + }) + }) + test('build', async () => { await build({ ...getConfig(), From 2699ebaf150f23442e79cee14eefa5dce6963a94 Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 4 Dec 2024 13:33:29 +0000 Subject: [PATCH 18/21] remove unnecessary if block --- packages/vite/src/node/plugins/resolve.ts | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/vite/src/node/plugins/resolve.ts b/packages/vite/src/node/plugins/resolve.ts index e7ce14bc566367..121a12b3254ff3 100644 --- a/packages/vite/src/node/plugins/resolve.ts +++ b/packages/vite/src/node/plugins/resolve.ts @@ -433,22 +433,6 @@ export function resolvePlugin( currentEnvironmentOptions.consumer === 'server' && isNodeLikeBuiltin(id) ) { - if ( - options.noExternal === true && - // if both noExternal and external are true, noExternal will take the higher priority and bundle it. - // only if the id is explicitly listed in external, we will externalize it and skip this error. - (options.external === true || !options.external.includes(id)) - ) { - let message = `Cannot bundle node built-in module "${id}"` - if (importer) { - message += ` imported from "${path.relative( - process.cwd(), - importer, - )}"` - } - message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.` - this.error(message) - } if (!(options.external === true || options.external.includes(id))) { let message = `Automatically externalized node built-in module "${id}"` if (importer) { From debad440db348931561d5788f942792a03347dab Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 4 Dec 2024 13:41:14 +0000 Subject: [PATCH 19/21] add tests for defaults --- packages/vite/src/node/__tests__/resolve.spec.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/vite/src/node/__tests__/resolve.spec.ts b/packages/vite/src/node/__tests__/resolve.spec.ts index 2287e00bf8beb1..123bd6eec40a44 100644 --- a/packages/vite/src/node/__tests__/resolve.spec.ts +++ b/packages/vite/src/node/__tests__/resolve.spec.ts @@ -140,7 +140,7 @@ describe('file url', () => { async function run( customEnvBuiltins: NonNullable['builtins'], idToResolve: string, - envName = 'custom', + envName: 'client' | 'ssr' | string = 'custom', ) { const server = await createServer(getConfig(customEnvBuiltins)) onTestFinished(() => server.close()) @@ -179,6 +179,16 @@ describe('file url', () => { expect(resolved?.external).toBe(true) }) + test('default to node-like builtins for ssr environment', async () => { + const resolved = await run(undefined, 'node:fs', 'ssr') + expect(resolved?.external).toBe(true) + }) + + test('no default to node-like builtins for client environment', async () => { + const resolved = await run(undefined, 'node:fs', 'client') + expect(resolved?.id).toEqual('__vite-browser-external:node:fs') + }) + test('declared node builtin', async () => { const resolved = await run([/^node:/], 'node:fs') expect(resolved?.external).toBe(true) From 664273d667892db0a2b2ce5c69e338d49ca57ccc Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 4 Dec 2024 13:56:58 +0000 Subject: [PATCH 20/21] improve tests --- .../vite/src/node/__tests__/resolve.spec.ts | 91 ++++++++++++++----- 1 file changed, 67 insertions(+), 24 deletions(-) diff --git a/packages/vite/src/node/__tests__/resolve.spec.ts b/packages/vite/src/node/__tests__/resolve.spec.ts index 123bd6eec40a44..f9f4105a780b29 100644 --- a/packages/vite/src/node/__tests__/resolve.spec.ts +++ b/packages/vite/src/node/__tests__/resolve.spec.ts @@ -118,6 +118,7 @@ describe('file url', () => { describe('environment builtins', () => { function getConfig( + targetEnv: 'client' | 'ssr' | string, customEnvBuiltins: NonNullable['builtins'], ): InlineConfig { return { @@ -128,7 +129,7 @@ describe('file url', () => { middlewareMode: true, }, environments: { - custom: { + [targetEnv]: { resolve: { builtins: customEnvBuiltins, }, @@ -137,69 +138,111 @@ describe('file url', () => { } } - async function run( - customEnvBuiltins: NonNullable['builtins'], - idToResolve: string, - envName: 'client' | 'ssr' | string = 'custom', - ) { - const server = await createServer(getConfig(customEnvBuiltins)) + async function run({ + builtins, + targetEnv = 'custom', + testEnv = 'custom', + idToResolve, + }: { + builtins?: NonNullable['builtins'] + targetEnv?: 'client' | 'ssr' | string + testEnv?: 'client' | 'ssr' | string + idToResolve: string + }) { + const server = await createServer(getConfig(targetEnv, builtins)) onTestFinished(() => server.close()) - return server.environments[envName]?.pluginContainer.resolveId( + return server.environments[testEnv]?.pluginContainer.resolveId( idToResolve, ) } test('declared builtin string', async () => { - const resolved = await run( - ['my-env:custom-builtin'], - 'my-env:custom-builtin', - ) + const resolved = await run({ + builtins: ['my-env:custom-builtin'], + idToResolve: 'my-env:custom-builtin', + }) expect(resolved?.external).toBe(true) }) test('declared builtin regexp', async () => { - const resolved = await run([/^my-env:\w/], 'my-env:custom-builtin') + const resolved = await run({ + builtins: [/^my-env:\w/], + idToResolve: 'my-env:custom-builtin', + }) expect(resolved?.external).toBe(true) }) test('non declared builtin', async () => { - const resolved = await run([], 'my-env:custom-builtin') + const resolved = await run({ + builtins: [ + /* empty */ + ], + idToResolve: 'my-env:custom-builtin', + }) expect(resolved).toBeNull() }) test('non declared node builtin', async () => { - await expect(run([], 'node:fs')).rejects.toThrowError( + await expect( + run({ + builtins: [ + /* empty */ + ], + idToResolve: 'node:fs', + }), + ).rejects.toThrowError( /Automatically externalized node built-in module "node:fs"/, ) }) test('default to node-like builtins', async () => { - const resolved = await run(undefined, 'node:fs') + const resolved = await run({ + idToResolve: 'node:fs', + }) expect(resolved?.external).toBe(true) }) test('default to node-like builtins for ssr environment', async () => { - const resolved = await run(undefined, 'node:fs', 'ssr') + const resolved = await run({ + idToResolve: 'node:fs', + testEnv: 'ssr', + }) expect(resolved?.external).toBe(true) }) test('no default to node-like builtins for client environment', async () => { - const resolved = await run(undefined, 'node:fs', 'client') + const resolved = await run({ + idToResolve: 'node:fs', + testEnv: 'client', + }) + expect(resolved?.id).toEqual('__vite-browser-external:node:fs') + }) + + test('no builtins overriding for client environment', async () => { + const resolved = await run({ + idToResolve: 'node:fs', + testEnv: 'client', + targetEnv: 'client', + }) expect(resolved?.id).toEqual('__vite-browser-external:node:fs') }) test('declared node builtin', async () => { - const resolved = await run([/^node:/], 'node:fs') + const resolved = await run({ + builtins: [/^node:/], + idToResolve: 'node:fs', + }) expect(resolved?.external).toBe(true) }) test('declared builtin string in different environment', async () => { - const resolved = await run( - ['my-env:custom-builtin'], - 'my-env:custom-builtin', - 'ssr', - ) + const resolved = await run({ + builtins: ['my-env:custom-builtin'], + idToResolve: 'my-env:custom-builtin', + targetEnv: 'custom', + testEnv: 'ssr', + }) expect(resolved).toBe(null) }) }) From 09ff29291b98fa1602ece6f5db2d5e0fd30b06df Mon Sep 17 00:00:00 2001 From: Dario Piotrowicz Date: Wed, 4 Dec 2024 14:01:22 +0000 Subject: [PATCH 21/21] update parameter name --- packages/vite/src/node/__tests__/resolve.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vite/src/node/__tests__/resolve.spec.ts b/packages/vite/src/node/__tests__/resolve.spec.ts index f9f4105a780b29..ad15dcdd73a547 100644 --- a/packages/vite/src/node/__tests__/resolve.spec.ts +++ b/packages/vite/src/node/__tests__/resolve.spec.ts @@ -119,7 +119,7 @@ describe('file url', () => { describe('environment builtins', () => { function getConfig( targetEnv: 'client' | 'ssr' | string, - customEnvBuiltins: NonNullable['builtins'], + builtins: NonNullable['builtins'], ): InlineConfig { return { configFile: false, @@ -131,7 +131,7 @@ describe('file url', () => { environments: { [targetEnv]: { resolve: { - builtins: customEnvBuiltins, + builtins, }, }, },