feat(devtools): only enable for selected build environments#22290
feat(devtools): only enable for selected build environments#22290webfansplz wants to merge 1 commit into
Conversation
| const debug = createDebugger('vite:config', { depth: 10 }) | ||
| const promisifiedRealpath = promisify(fs.realpath) | ||
| const SYMBOL_RESOLVED_CONFIG: unique symbol = Symbol('vite:resolved-config') | ||
| type DevToolsConfigWithEnvironments = { environments?: string[] } |
There was a problem hiding this comment.
Pull request overview
Adds support for enabling Vite DevTools only for selected build environments (linked to vitejs/devtools#292), integrating that selection into both config resolution and builder DevTools startup.
Changes:
- Introduces
getDevToolsEnvironments()to derive the target environment names fromdevtools.config.environments(defaulting to all environments). - Enables
build.rolldownOptions.devtoolsonly for the selected environment(s) duringresolveConfig(). - Starts DevTools in the builder only if at least one selected environment was actually built.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/vite/src/node/config.ts | Adds environment selection helper and applies devtools enablement per environment during config resolution. |
| packages/vite/src/node/build.ts | Uses the selection helper to decide whether to start DevTools based on which environments were built. |
| return ( | ||
| (devtoolsConfig.config as DevToolsConfigWithEnvironments).environments ?? | ||
| Object.keys(environments) |
There was a problem hiding this comment.
getDevToolsEnvironments returns devtoolsConfig.config.environments verbatim. Because these names can come directly from user config, it should be validated/filtered against the provided environments map using Object.hasOwn(...) (and ideally deduped) before callers index into resolved.environments[name]. Without this, values like __proto__/toString (or simply unknown environment names) can accidentally hit the prototype chain or return undefined, leading to incorrect behavior or potential runtime errors downstream.
| return ( | |
| (devtoolsConfig.config as DevToolsConfigWithEnvironments).environments ?? | |
| Object.keys(environments) | |
| const configuredEnvironments = ( | |
| devtoolsConfig.config as DevToolsConfigWithEnvironments | |
| ).environments | |
| if (!configuredEnvironments) { | |
| return Object.keys(environments) | |
| } | |
| return [...new Set(configuredEnvironments)].filter((environment) => | |
| Object.hasOwn(environments, environment), |
| // Enable `rolldownOptions.devtools` if devtools is enabled and the environment is selected, or for all environments by default. | ||
| if (resolved.devtools.enabled) { | ||
| resolved.build.rolldownOptions.devtools ??= {} | ||
| for (const environmentName of getDevToolsEnvironments( | ||
| resolved.devtools, | ||
| resolved.environments, | ||
| )) { |
There was a problem hiding this comment.
This change introduces new behavior (enabling rolldownOptions.devtools only for selected environments, and defaulting to all environments when not specified), but there doesn't appear to be a unit test covering it. Consider adding a focused test in packages/vite/src/node/__tests__/config.spec.ts (which already exercises per-environment build option resolution) to assert that only the configured environment(s) get build.rolldownOptions.devtools set, and that the default applies to all environments when devtools.config.environments is omitted.
Be related to vitejs/devtools#292