refactor(oxfmt): Rewrite --init mode in JS#16769
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
The biggest concern is Mode::Init => unreachable!(...) in the NAPI path, which can hard-crash the process if --init reaches Rust due to packaging/regression or direct invocation. The JS --init implementation is mostly fine, but $schema detection should match the previous Rust behavior (is_file) to avoid false positives. Consider tightening --init UX by rejecting extra arguments to prevent ambiguous behavior. Lint suppression for console is broader than necessary and should be scoped down.
Additional notes (2)
-
Maintainability |
apps/oxfmt/src-js/migration/init.ts:1-2
The file disablesno-consoleglobally, which broadens the blast radius and makes it easy to accidentally introduce unrelated console usage later. Since this module has only a couple of logging lines, prefer narrowing the suppression to the specific lines or using a small logger wrapper that can be mocked in tests. (This is especially important for CLI code where stdout/stderr shape is part of the API.) -
Readability |
apps/oxfmt/src/main_napi.rs:69-73
Mode::Init => unreachable!(...)will panic if JS ever fails to intercept--init(e.g., someone uses the NAPI entrypoint directly, or a packaging/regression causes the JS CLI to not run this path). Panicking turns a user-facing CLI error into a crash.
Given this mode still exists for help output, prefer returning a controlled non-zero result with a clear message. That preserves stability and makes failures diagnosable.
Summary of changes
Summary
JavaScript CLI
- Wrapped
apps/oxfmt/src-js/cli.tsin an async IIFE and added a JS-side fast path for--initviarunInit(). - Added
apps/oxfmt/src-js/migration/init.tsimplementing--initscaffolding for.oxfmtrc.jsonand optional$schemainjection.
Rust CLI/NAPI surface
- Kept
Mode::Initfor help/arg parsing but changed behavior so Rust/NAPI treats it as unreachable (apps/oxfmt/src/main_napi.rs). - Updated help text from
.oxfmtrc.jsoncto.oxfmtrc.json(apps/oxfmt/src/cli/command.rs). - Removed Rust init module (
apps/oxfmt/src/init/mod.rs) and its exposure fromapps/oxfmt/src/lib.rs. - Simplified CLI exit code mapping by removing init-related variants (
apps/oxfmt/src/cli/result.rs).
Tests
- Added
apps/oxfmt/test/init.test.tscovering creation,$schemainsertion, and abort behavior. - Exported
runCliandRunResultfromapps/oxfmt/test/utils.tsfor reuse.
86a1ca1 to
fb899cf
Compare
--init mode in JS
Merge activity
|
ca7f3b3 to
b6f4a3e
Compare
After giving it some thought, I decided to lean these features toward JS after all. The reasons are: - I want Rust side to focus on complex formatting tasks, while moving infrequently used commands elsewhere - Code defining `napi` callbacks tends to become verbose While `--init` itself worked on the Rust side, it will share some lines with upcoming `--migrate`, so I'm extracting this beforehand too. Also, We were generating `.jsonc`, but I've switched to creating `.json` for now. This is because running `oxfmt` immediately after generating `.jsonc` causes trailing comma diffs. 😨 I was hesitant to introduce a jsonc writer, so `.json` will enough for now.
fb899cf to
15067ce
Compare
b6f4a3e to
d719988
Compare
After giving it some thought, I decided to lean these features toward JS after all. The reasons are: - I want Rust side to focus on complex formatting tasks, while moving infrequently used commands elsewhere - Code defining `napi` callbacks tends to become verbose While `--init` itself worked on the Rust side, it will share some lines with upcoming `--migrate`, so I'm extracting this beforehand too. Also, We were generating `.jsonc`, but I've switched to creating `.json` for now. This is because running `oxfmt` immediately after generating `.jsonc` causes trailing comma diffs. 😨 I was hesitant to introduce a jsonc writer, so `.json` will enough for now.
After giving it some thought, I decided to lean these features toward JS after all. The reasons are: - I want Rust side to focus on complex formatting tasks, while moving infrequently used commands elsewhere - Code defining `napi` callbacks tends to become verbose While `--init` itself worked on the Rust side, it will share some lines with upcoming `--migrate`, so I'm extracting this beforehand too. Also, We were generating `.jsonc`, but I've switched to creating `.json` for now. This is because running `oxfmt` immediately after generating `.jsonc` causes trailing comma diffs. 😨 I was hesitant to introduce a jsonc writer, so `.json` will enough for now.
15067ce to
8f9b5c2
Compare
8f9b5c2 to
bbec437
Compare

After giving it some thought, I decided to lean these features toward JS after all.
The reasons are:
napicallbacks tends to become verboseWhile
--inititself worked on the Rust side, it will share some lines with upcoming--migrate, so I'm extracting this beforehand too.Also, We were generating
.jsonc, but I've switched to creating.jsonfor now.This is because running
oxfmtimmediately after generating.jsonccauses trailing comma diffs. 😨I was hesitant to introduce a jsonc writer, so
.jsonwill enough for now.