-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
feat: add the builtins environment resolve
#18584
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
Changes from 7 commits
bc680b8
298e0c1
63866c9
4712b5f
98723a4
fbdadfc
8c0bc0a
0861cb2
d2a05d5
e39c3a2
ad579bf
d1f1ca0
ae1ed5d
5666f67
8181eeb
1baf5c8
0b7d83f
2699eba
debad44
664273d
09ff292
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,17 +25,19 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isDataUrl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isExternalUrl, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isInNodeModules, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isNodeLikeBuiltin, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isNonDriveRelativeAbsolutePath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isObject, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isOptimizable, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isTsRequest, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| nodeLikeBuiltins, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| normalizePath, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| safeRealpathSync, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tryStatSync, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } 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 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -100,6 +102,10 @@ export interface EnvironmentResolveOptions { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * @internal | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| enableBuiltinNoExternalCheck?: boolean | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Array of strings or regular expressions that indicate what modules are builtin for the environment. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| builtins?: (string | RegExp)[] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface ResolveOptions extends EnvironmentResolveOptions { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -320,7 +326,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,59 +422,69 @@ export function resolvePlugin( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| importer, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| false, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.environment.config, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| external, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| )) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return res | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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(this.environment.config.resolve.builtins, id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dario-piotrowicz marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
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 isProduction | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? browserExternalId | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : `${browserExternalId}:${id}` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return options.idOnly | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ? id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| : { id, external: true, moduleSideEffects: false } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| currentEnvironmentOptions.consumer === 'client' && | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| isNodeLikeBuiltin(id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else if ( | |
| currentEnvironmentOptions.consumer === 'client' && | |
| isNodeLikeBuiltin(id) | |
| ) { | |
| } 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) | |
| ) { |
How about moving the "if options.enableBuiltinNoExternalCheck block" here from above and removing options.enableBuiltinNoExternalCheck && and setting builtin: [] by default for ssr.target: 'webworker'?
By setting builtin: [], I mean updating this line to (consumer === 'server' && !isSsrTargetWebworkerEnvironment.
https://github.com/vitejs/vite/pull/18584/files#diff-11e17761d4ecfee8f8fde15c6d79b7bc0260176396a30dfd8e6f6bbaf5af4745R888
This way if an environment sets builtin and have some node builtin modules not included in builtin, Vite will show a more friendly error (Cannot bundle built-in module) when noExternal is set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made the changes in regards to enableBuiltinNoExternalCheck that you suggested, it sounds sensible to me 🙂👍
There was a problem hiding this comment.
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 throwsCould not resolve "cloudflare:*"error. I guess it needs to bereturn { path: resolved, external: true }.But I wonder if we should set
external: truefor anything that was externalized by rollup plugins orresolve.externalinstead of checkingisBuiltinhere.