Skip to content

fix(migrate): finish leftover Oxc config migration - #2654

Merged
fengmk2 merged 5 commits into
voidzero-dev:mainfrom
SaKaNa-Y:fix/migrate-leftover-oxc-configs
Sep 11, 2026
Merged

fix(migrate): finish leftover Oxc config migration#2654
fengmk2 merged 5 commits into
voidzero-dev:mainfrom
SaKaNa-Y:fix/migrate-leftover-oxc-configs

Conversation

@SaKaNa-Y

@SaKaNa-Y SaKaNa-Y commented Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

When an interrupted migration has already added vite-plus, running vp migrate again can leave .oxfmtrc.json behind and report that the project already uses Vite+. Formatting then uses defaults instead of the unmerged options.

The existing-project finalization path skips standalone Oxc configs. Run the existing config merger for the root and workspace packages, and count completed merges as migration work. This covers Oxlint and Oxfmt JSON/JSONC configs, preserves existing inline config precedence, and retains configs that cannot be merged with a warning. A subsequent migration remains a no-op.

The branch incorporates upstream main at b1c41b2a, including the leftover tsdown config fix from #2646. Both finalization paths and result flags are preserved. A regression test covers simultaneous Oxfmt and tsdown config migration and an unchanged retry.

Verification

After resolving the upstream merge:

  • Migration unit suite: 436 passed across 11 files, including the combined-config regression. Ran the source tests with Vitest 4.1.11 and an isolated config aliasing the local prompts source, using a prebuilt native binding. The native migration implementation has not changed since the binding's base revision; the complete native CLI was not rebuilt.
  • Targeted Oxfmt check on the three changed TypeScript files, using the repository's format configuration: passed.
  • git diff --check: passed.
  • Targeted Oxlint type-aware/type check: seven prompt cancellation narrowing errors. Running the same check on unmodified upstream b1c41b2a in the same environment produced identical diagnostics.
  • The full repository suite and PTY snapshot suite were not rerun after this merge.

Before the merge, the PR's recorded validation included a passing migration_existing_oxc_configs PTY snapshot. It covers config removal, formatting with migrated options, an unchanged second migration, and no opt-out setup artifacts.

Existing inline fmt snapshot coverage

Added migration_existing_oxc_configs_inline_fmt in response to review. It checks formatting before and after migration with inline singleQuote: false overriding the standalone singleQuote: true, while semi: false verifies that formatting actually applies the inline options. It also verifies standalone config removal, unchanged inline configuration, and a no-op retry.

  • The official PTY runner generated the snapshot; both migration_existing_oxc_configs cases then passed in comparison mode (2 passed).
  • With the pre-fix migration entry, the new case failed specifically because .oxfmtrc.json still existed.
  • git diff --check passed. This update changes only test metadata and its recorded snapshot.
  • Validation rebuilt the PR's ESM JavaScript CLI and prompts and compiled the snapshot runner. It reused the existing source-built global binary, native binding, and core package; a complete native/toolchain rebuild and the full repository suite were not run.

@SaKaNa-Y
fix(migrate): finish leftover Oxc config migration
de81a37 
Existing Vite+ projects skip standalone Oxc config merging when migration is retried. Reuse the config merger during core finalization for the root and workspace packages, and report completed merges as migration work.

Add regression coverage for retry behavior, inline config precedence, merge warnings, and formatting with migrated options.
@SaKaNa-Y
SaKaNa-Y marked this pull request as draft September 10, 2026 08:06
@SaKaNa-Y
fix(migrate): reconcile Oxc and tsdown finalization
2eef2b4 
Merge upstream main while preserving both leftover-config finalization paths and their migration result flags. Update result assertions and cover simultaneous Oxfmt and tsdown config migration with an idempotent retry.

Validation: 436 migration tests passed across 11 files. Targeted formatting and diff checks passed. Targeted type checking reports the same seven prompt cancellation diagnostics as upstream main in the same environment.
@SaKaNa-Y
SaKaNa-Y marked this pull request as ready for review September 10, 2026 13:32
@fengmk2 fengmk2 self-assigned this Sep 10, 2026
@fengmk2

fengmk2 commented Sep 10, 2026

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 10, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-10T15:22:03.835963Z 2eef2b4 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

chatgpt-codex-connector Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 2eef2b4f63

ℹ️ 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".

@fengmk2 fengmk2 added test: e2e Auto run e2e tests test: install-e2e run vite install e2e test test: create-e2e Run `vp create` e2e tests labels Sep 10, 2026
@SaKaNa-Y
test(migrate): cover existing inline fmt in CLI snapshots
2311e14 
Verify that migration removes a leftover Oxfmt config while preserving conflicting inline options and formatting behavior. Check that a second migration is a no-op.

Both Oxc snapshot cases pass. The new case fails its config-removal assertion with the pre-fix migration entry.
@SaKaNa-Y
SaKaNa-Y requested a review from fengmk2 September 11, 2026 12:04
@fengmk2
fengmk2 merged commit e3bd9db into voidzero-dev:main Sep 11, 2026
107 checks passed
@SaKaNa-Y
SaKaNa-Y deleted the fix/migrate-leftover-oxc-configs branch September 11, 2026 14:39
@fengmk2 fengmk2 mentioned this pull request Sep 14, 2026
Merged
@fengmk2

fengmk2 commented Sep 14, 2026

Copy link
Copy Markdown
Member

@SaKaNa-Y I plan to revert this PR for now. Ecosystem tests found regressions in freecut, delta-comic, pdfme, and sigle: vp migrate deletes Oxc configs that imports or scripts still use, or copies JSON extends paths into incompatible inline config.

0.3.1 preserves these configs. We can reintroduce this migration step after #2695 addresses these cases and passes ecosystem validation.

@fengmk2 fengmk2 mentioned this pull request Sep 14, 2026
Merged
fengmk2 added a commit that referenced this pull request Sep 14, 2026
@fengmk2
revert(migrate): stop merging Oxc configs in existing projects (#2696)
5b30ff0 
Revert #2654. `vp migrate` again preserves standalone Oxc configs in
projects that already use Vite+.

The added merge step deletes configs that imports or package scripts
still use. It also copies JSON `extends` paths into incompatible inline
config. Ecosystem tests reproduced these failures in
[freecut](vite-plus-ecosystem-ci/freecut#9),
[delta-comic](vite-plus-ecosystem-ci/delta-comic#9),
[pdfme](vite-plus-ecosystem-ci/pdfme#7), and
[sigle](vite-plus-ecosystem-ci/sigle#8).

This revert restores an earlier limitation: interrupted migrations can
leave Oxc configs unmerged. #2695 tracks the follow-up fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fengmk2 fengmk2 fengmk2 approved these changes

Assignees

@fengmk2 fengmk2

Labels

test: create-e2e Run `vp create` e2e tests test: e2e Auto run e2e tests test: install-e2e run vite install e2e test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@SaKaNa-Y @fengmk2