• test(oxfmt): Add oxfmt --lsp test #16866
• test(oxfmt): Update oxfmt --init|migrate tests #16865
• feat(oxfmt): Support oxfmt --migrate prettier (JS side) #16773 👈 (View in Graphite)
• feat(oxfmt): Support oxfmt --migrate [prettier] (Rust side) #16771
• main

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

This stack of pull requests is managed by Graphite. Learn more about stacking.

Review for PR #16773

Overall impressions

Solid foundation for oxfmt --migrate prettier with good attention to CLI entrypoints and shared utilities. The implementation is clear and concise, but there are a few correctness and safety issues in the migration logic that are worth addressing to make migrations robust, predictable, and user-informative.

Major Feedback

1. Mismatched Prettier config resolution
   In migrate-prettier.ts, resolveConfigFile() operates on dummy.js, but resolveConfig() is called with cwd (the directory), not the file path, so different configs (including overrides) might be resolved. Align both calls to use the same target (dummy.js) to ensure correct config migration.

2. Over-broad Prettier option mapping
   All Prettier config keys are copied to .oxfmtrc.json, including unsupported/unknown options. This makes for future migration headaches and could mislead users. Prefer an explicit allowlist of supported options, and warn on unknowns to keep the migrated config clean and future-proof.

3. Error swallowing in .prettierignore migration
   All errors while reading .prettierignore—not just file not found—are ignored without feedback, which could make ignore-related issues much harder to debug. Only quietly ignore "not found"; warn for anything else.

4. Inconsistent exit behavior
   exitWithError sets process.exitCode but continues program flow. This risks accidental execution after a blocking error, especially with mixed return exitWithError(...) and just exitWithError(...) patterns. Prefer making this function either consistently control-flow breaking (e.g., throw) or explicit (e.g., always return and require a top-level return).

Minor suggestions

• Consider logging, tabling, or exporting the list of skipped config keys for users who want to manually check what wasn't migrated.
• Add unit tests for option translation and error edge cases in the next refactor, as stated in your PR notes.

Summary

Great progress and well-factored utilities with just a few migration correctness/safety changes advised before releasing widely. Please reference detailed suggestions in the inline bots' comments for practical code-level examples.

Inline feedback (see PR for context and details):

1. Align Prettier config resolution to always use the same filename target.
2. Replace blanket option copying with an allowlist, warning on unsupported keys.
3. Log reading errors from .prettierignore except for ENOENT.
4. Make exitWithError always halt control flow (e.g., throw).

Would recommend addressing the above for more predictable and robust migrations.

Merge activity

• Dec 15, 2:48 AM UTC: Dunqing added this pull request to the Graphite merge queue.
• Dec 15, 2:56 AM UTC: Merged by the Graphite merge queue.