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

feat: add the builtins environment resolve #18584

Merged
merged 21 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 132 additions & 1 deletion packages/vite/src/node/__tests__/resolve.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -116,6 +116,137 @@ describe('file url', () => {
expect(mod4.default).toBe(mod)
})

describe('environment builtins', () => {
function getConfig(
targetEnv: 'client' | 'ssr' | string,
builtins: NonNullable<EnvironmentOptions['resolve']>['builtins'],
): InlineConfig {
return {
configFile: false,
root: join(import.meta.dirname, 'fixtures/file-url'),
logLevel: 'error',
server: {
middlewareMode: true,
},
environments: {
[targetEnv]: {
resolve: {
builtins,
},
},
},
}
}

async function run({
builtins,
targetEnv = 'custom',
testEnv = 'custom',
idToResolve,
}: {
builtins?: NonNullable<EnvironmentOptions['resolve']>['builtins']
targetEnv?: 'client' | 'ssr' | string
testEnv?: 'client' | 'ssr' | string
idToResolve: string
}) {
const server = await createServer(getConfig(targetEnv, builtins))
onTestFinished(() => server.close())

return server.environments[testEnv]?.pluginContainer.resolveId(
idToResolve,
)
}

test('declared builtin string', async () => {
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({
builtins: [/^my-env:\w/],
idToResolve: 'my-env:custom-builtin',
})
expect(resolved?.external).toBe(true)
})

test('non declared builtin', async () => {
const resolved = await run({
builtins: [
/* empty */
],
idToResolve: 'my-env:custom-builtin',
})
expect(resolved).toBeNull()
})

test('non declared node builtin', async () => {
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({
idToResolve: 'node:fs',
})
expect(resolved?.external).toBe(true)
})

test('default to node-like builtins for ssr environment', async () => {
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({
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({
builtins: [/^node:/],
idToResolve: 'node:fs',
})
expect(resolved?.external).toBe(true)
})

test('declared builtin string in different environment', async () => {
const resolved = await run({
builtins: ['my-env:custom-builtin'],
idToResolve: 'my-env:custom-builtin',
targetEnv: 'custom',
testEnv: 'ssr',
})
expect(resolved).toBe(null)
})
})

test('build', async () => {
await build({
...getConfig(),
Expand Down
12 changes: 9 additions & 3 deletions packages/vite/src/node/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,17 @@ import {
asyncFlatten,
createDebugger,
createFilter,
isBuiltin,
isExternalUrl,
isFilePathESM,
isInNodeModules,
isNodeBuiltin,
isNodeLikeBuiltin,
isObject,
isParentDirectory,
mergeAlias,
mergeConfig,
mergeWithDefaults,
nodeLikeBuiltins,
normalizeAlias,
normalizePath,
} from './utils'
Expand Down Expand Up @@ -881,7 +882,11 @@ function resolveEnvironmentResolveOptions(
consumer === 'client' || isSsrTargetWebworkerEnvironment
? DEFAULT_CLIENT_CONDITIONS
: DEFAULT_SERVER_CONDITIONS.filter((c) => c !== 'browser'),
enableBuiltinNoExternalCheck: !!isSsrTargetWebworkerEnvironment,
builtins:
resolve?.builtins ??
(consumer === 'server' && !isSsrTargetWebworkerEnvironment
? nodeLikeBuiltins
: []),
},
resolve ?? {},
)
Expand Down Expand Up @@ -1753,6 +1758,7 @@ async function bundleConfigFile(
preserveSymlinks: false,
packageCache,
isRequire,
builtins: nodeLikeBuiltins,
})?.id
}

Expand All @@ -1771,7 +1777,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 }
}

Expand Down
4 changes: 3 additions & 1 deletion packages/vite/src/node/external.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ function createIsExternal(
}
let isExternal = false
if (id[0] !== '.' && !path.isAbsolute(id)) {
isExternal = isBuiltin(id) || isConfiguredAsExternal(id, importer)
isExternal =
isBuiltin(environment.config.resolve.builtins, id) ||
isConfiguredAsExternal(id, importer)
}
processedIds.set(id, isExternal)
return isExternal
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/optimizer/esbuildDepPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export function esbuildDepPlugin(
namespace: 'optional-peer-dep',
}
}
if (environment.config.consumer === 'server' && isBuiltin(resolved)) {
if (isBuiltin(environment.config.resolve.builtins, resolved)) {
return
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this does not work. cloudflare:* will be processed by esbuild and IIRC esbuild does not externalize them automatically and then it throws Could not resolve "cloudflare:*" error. I guess it needs to be return { path: resolved, external: true }.
But I wonder if we should set external: true for anything that was externalized by rollup plugins or resolve.external instead of checking isBuiltin here.

if (isExternalUrl(resolved)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/vite/src/node/plugins/importAnalysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ export function importAnalysisPlugin(config: ResolvedConfig): Plugin {
if (shouldExternalize(environment, specifier, importer)) {
return
}
if (isBuiltin(specifier)) {
if (isBuiltin(environment.config.resolve.builtins, specifier)) {
return
}
}
Expand Down
112 changes: 66 additions & 46 deletions packages/vite/src/node/plugins/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
isDataUrl,
isExternalUrl,
isInNodeModules,
isNodeLikeBuiltin,
isNonDriveRelativeAbsolutePath,
isObject,
isOptimizable,
Expand Down Expand Up @@ -97,9 +98,9 @@ export interface EnvironmentResolveOptions {
*/
external?: string[] | true
/**
* @internal
* Array of strings or regular expressions that indicate what modules are builtin for the environment.
*/
enableBuiltinNoExternalCheck?: boolean
builtins?: (string | RegExp)[]
}

export interface ResolveOptions extends EnvironmentResolveOptions {
Expand Down Expand Up @@ -173,11 +174,8 @@ interface ResolvePluginOptions {
}

export interface InternalResolveOptions
extends Required<Omit<ResolveOptions, 'enableBuiltinNoExternalCheck'>>,
ResolvePluginOptions {
/** @internal this is always optional for backward compat */
enableBuiltinNoExternalCheck?: boolean
}
extends Required<ResolveOptions>,
ResolvePluginOptions {}

// Defined ResolveOptions are used to overwrite the values for all environments
// It is used when creating custom resolvers (for CSS, scanning, etc)
Expand Down Expand Up @@ -422,47 +420,67 @@ export function resolvePlugin(
return res
}

// node built-ins.
// externalize if building for a node compatible environment, otherwise redirect to empty module
if (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 Node.js built-in "${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)
// built-ins
// externalize if building for a server environment, otherwise redirect to an empty module
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.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)
}
dario-piotrowicz marked this conversation as resolved.
Show resolved Hide resolved

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 options.idOnly
? id
: { id, external: true, moduleSideEffects: false }
} else if (
currentEnvironmentOptions.consumer === 'client' &&
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 built-in module "${id}"`
if (importer) {
message += ` imported from "${path.relative(
process.cwd(),
importer,
)}"`
}
return isProduction
? browserExternalId
: `${browserExternalId}:${id}`
message += `. Consider disabling environments.${this.environment.name}.noExternal or remove the built-in dependency.`
this.error(message)
}

sapphi-red marked this conversation as resolved.
Show resolved Hide resolved
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}`
}
}

Expand Down Expand Up @@ -720,8 +738,10 @@ export function tryNodeResolve(
basedir = root
}

const isModuleBuiltin = (id: string) => isBuiltin(options.builtins, 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 =
Expand All @@ -738,7 +758,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)
) {
Expand Down
Loading
Loading