[john-okeefe]

Problem

vp migrate rewrites the imports vite-plus owns but leaves config semantics untouched (per the scoped-rewrite policy in #2004). One leftover is not a silent behavioral difference but a guaranteed build failure: Vite 8 no longer bundles esbuild, so a project migrating from Vite ≤ 7 with

export default defineConfig({
  build: { minify: 'esbuild' },
});

fails on its very next vp build with:

[plugin vite:esbuild-transpile]
Error: Failed to load `transformWithEsbuild`. It is deprecated and it now requires esbuild to be installed separately. If you are a package author, please migrate to `transformWithOxc` instead.

The error names neither minify nor the config file, and nothing in the migration output flags it. Repro + full details in #3176.

What this does (updated after review)

Per @fengmk2's review, this now performs a targeted config migration rather than only warning:

• Static case rewritten: build.minify: 'esbuild' inside the build object of a default-exported config is rewritten in place to minify: true via span replacement over the serialized oxc AST (analyzeMigrationSource). Comments and unrelated plugin options are preserved, quoted keys ('minify') are handled, and the switch is reported through the success log plus a new esbuildMinifyMigrationCount in the migration summary. Rewriting (rather than removing the property) keeps minification enabled for SSR builds.
• Everything else keeps its code and gets a diagnostic presenting both valid paths — install esbuild separately to keep esbuild behavior, or migrate to oxc — for: dynamic minify values, minify options outside the build object, and transformWithEsbuild usage (which may need argument changes). Vite 8 still supports esbuild-specific config when esbuild is installed separately, so the diagnostics no longer imply support is gone.
• AST-driven, regex only as a guarded fallback: detection walks the oxc AST, covering defineConfig({...}), defineConfig([{...}, ...]) env-config arrays, TS wrappers (as/satisfies), and quoted keys. Raw-text scanning exists only to feed diagnostics for shapes the static walk can't see (spread configs, helper-built objects), and it skips comment ranges.

Where it hooks in

Called everywhere migrate finalizes a project's vite config, mirroring migrateTaskCacheConfigInViteConfig:

• rewriteStandaloneProject
• rewriteMonorepo (workspace root + each package)
• rewriteMonorepoProject
• finalizeCoreMigrationForExistingVitePlus (already-on-vite+ projects running the finalize pass)

Test plan

New tests in migrator.spec.ts cover: the static rewrite (including comment preservation), quoted keys, unrelated minify options left alone with a diagnostic, dynamic values diagnosed without rewriting, transformWithEsbuild diagnostics, comment-only mentions ignored, oxc-compatible configs untouched, and the missing-config no-op.

• pnpm exec vitest run src/migration/__tests__/migrator.spec.ts — 370 passed (362 pre-existing + 8 new).

Closes #3176

[fengmk2]

Could we extend this into a targeted config migration? For a static build.minify: 'esbuild' with no esbuild-specific behavior to preserve, we could change it to minify: true and report the switch to Oxc. This would complete the migration for the case in #3176.

#2004 scopes import rewrites to symbols that vite-plus owns; it does not prohibit config migrations. We already migrate task-cache and pack configuration.

I’d use the existing AST analysis to identify the config property and preserve comments. The regex can match comments or unrelated plugin options, and it misses quoted property names. For dynamic configs, esbuild-specific options, or transformWithEsbuild calls that need argument changes, we should preserve the code and provide a diagnostic.

We should also adjust the warning: Vite 8 still supports build.minify: 'esbuild' when users install esbuild. Users who need that behavior should have the option to retain it. For the Oxc migration, prefer minify: true over removing the property, since removal can disable minification for SSR builds.

[john-okeefe]

Thanks @fengmk2 — all four points are addressed in f9be853:

• Targeted migration: static build.minify: 'esbuild' inside a default-exported config is now rewritten in place to minify: true (span replacement over the serialized oxc AST from analyzeMigrationSource), and the switch is reported via the success log plus a new esbuildMinifyMigrationCount in the migration summary. Rewriting rather than removing, as you noted, keeps minification enabled for SSR builds.
• AST-driven: detection walks the oxc AST, so comments are preserved (spans replace only the value node), unrelated plugin options are never touched, and quoted keys ('minify') resolve correctly. Raw-text scanning is now only a fallback for diagnostics and skips comment ranges.
• Diagnostics for the non-static cases (dynamic values, minify outside build, transformWithEsbuild calls): code is preserved and the diagnostic presents both valid paths — install esbuild to keep the behavior, or migrate to oxc.
• Wording adjusted: nothing implies esbuild support is gone under Vite 8.

Tests cover the rewrite (incl. comment preservation), quoted keys, unrelated options, dynamic values, transformWithEsbuild, comment-only mentions, and the no-op paths — 370 passed, and vp check on the touched module is clean.

[john-okeefe]

Follow-up (7d41849): the migration is now also tracked in CoreMigrationFinalizationResult (esbuildMinifyConfig), like the other finalize-path migrations, with the result-shape expectations in migrator.spec.ts updated to match. Style/convention pass only — nothing in the behavior described above changed.

[fengmk2]

@codex review

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-23T13:39:19.187236Z	7d41849	Manual request

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7d418492dc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[john-okeefe]

All three review findings are addressed:

• "Include the esbuild result when marking a migration complete" — coreMigrationResult.esbuildMinifyConfig is now part of the didMigrate predicate, so the minify switch reaches the summary and formatting pass instead of being swallowed by the "already using Vite+" fast exit. Regression-pinned by a new CLI snapshot fixture (migration_existing_vite_plus_esbuild_minify) that reintroduces minify: 'esbuild' on a converged project — verified it fails without the fix — plus a unit test asserting the flag and its convergence. (e7ed78e)

• "Inspect esbuild values returned by config callbacks" — staticConfigObjects now unwraps config factories: expression bodies directly, block bodies via their direct return statements. Literal 'esbuild' values there are rewritten; dynamic selections (e.g. mode === 'legacy' ? 'esbuild' : true) now emit the diagnostic instead of staying silent. Returns nested in conditionals keep the raw-occurrence fallback behavior (diagnosed, never rewritten blind). Four new unit tests. (86d871f)

• "Exclude computed identifiers from config-key matching" — propertyName no longer matches computed identifier keys, so [build]: { minify: 'esbuild' } with const build = 'plugin' is left untouched (and still diagnosed via the raw-occurrence fallback); computed string literals (['build']) still match, since they are the same key. Two new unit tests. (86d871f)

Validation: 377/377 unit tests, vp check clean, the snapshot fixture passes in compare mode, and it was confirmed to fail without the didMigrate fix.

[chatgpt-codex-connector]

To use Codex here, create a Codex account and connect to github.

[john-okeefe]

@fengmk2 Just wondering if you could check on this again. Thanks!

[fengmk2]

[P1] Preserve esbuild-specific behavior before switching minifiers

The rewrite checks the minify literal without checking the surrounding esbuild options. A project that explicitly uses esbuild features is therefore switched to Oxc while those features silently stop working, even when esbuild is installed:

export default {
  esbuild: { drop: ['console'], mangleProps: /_$/ },
  build: { minify: 'esbuild' },
};

The current helper changes this to minify: true, records one migration, and emits no warning. A real Vite 8.3.5 build confirms the effect: with 'esbuild', the output removes console.log and renames field_; changing only the minifier to true retains both the console call and field_. The Vite migration guide requires a separate migration for drop and states that Oxc does not support property mangling.

Only apply the automatic switch when the relevant esbuild behavior can be preserved. If options are unsupported or cannot be resolved safely, retain the original minifier and issue the manual-migration diagnostic.

[fengmk2]

[P2] Restrict automatic edits to helpers with known config semantics

staticConfigObjects treats arguments of every function call and constructor as Vite config objects. An unknown helper can use its argument before returning a config, so changing that argument from a string to a boolean can break config evaluation:

function createConfig(options) {
  if (options.build.minify.toLowerCase() !== 'esbuild') {
    throw new Error('unexpected minifier');
  }
  return options;
}
export default createConfig({ build: { minify: 'esbuild' } });

This evaluates successfully before migration. After migration it throws TypeError: options.build.minify.toLowerCase is not a function, with no migration warning. Resolve the callee binding and limit automatic edits to recognized config helpers with compatible semantics. Preserve arguments to unknown functions and constructors and diagnose the esbuild setting instead.

[fengmk2]

[P2] Identify unhandled esbuild settings by AST location, not match counts

The fallback compares the number of regex matches with the number of AST edits, but those sets do not contain the same occurrences. For example:

export default ({ mode }) => {
  if (mode === 'legacy') {
    return { build: { minify: 'esbuild' } };
  }
  return { build: { 'minify': 'esbuild' } };
};

The direct return is rewritten, while the nested return remains unchanged. The regex matches only the nested return's bare key, while the edit count includes the direct return's quoted key. Both counts equal one, so migration emits no warning. A subsequent build in legacy mode still selects esbuild and fails if it is absent.

The regex also misses quoted keys and comments before the value in unsupported shapes, such as const base = { build: { 'minify': 'esbuild' } }; export default { ...base };. Use parsed property nodes for diagnostics and exclude only the exact value spans already rewritten. This covers unresolved shapes without changing their code.

[fengmk2]

[P2] Do not report boolean-only minify values as esbuild usage

Every non-literal value sets hasDynamicMinify, including values that cannot select esbuild:

export default {
  build: { minify: process.env.MINIFY === 'true' },
};

This expression only returns a boolean, but migration warns that minify: "esbuild" requires a separate installation and suggests minify: true. The same warning occurs for true as const and an identifier bound to true. Following the replacement advice would remove the conditional behavior. Each subsequent migration emits the warning again; on an existing Vite+ project it also prevents the normal no-op return because that condition requires no report warnings.

Require evidence that the value can select esbuild before issuing this esbuild-specific diagnostic. Recognize boolean expressions and unwrap TypeScript wrappers; distinguish unresolved values from known esbuild usage.