Repository navigation
Conversation
7eaa161 to
1c7d01d
Compare
renkun-ken
left a comment
There was a problem hiding this comment.
Reviewed 1c7d01d7865b2ca97e7c1971e400cbdd4a7409cd. No blocking correctness issues found; approving with one minor documentation clarification below.
Validation:
- Verified all 179 changed file blobs against GitHub and reproduced the 173 TypeScript changes with Oxfmt 0.73.0, apart from the documented lint-suppression relocation. Compared normalized emitted JavaScript and inspected the remaining redundant-parenthesis difference; no behavior changes found.
- Frozen-lockfile dependency installation (
--ignore-scripts),pnpm run format:check,pnpm run lint, andpnpm run typecheckpassed locally. - CLI smoke tests covered root/nested
.ts,.tsx,.mts, and.ctsfiles, excluded non-TypeScript files, and ignored generated/dependency directories. Direct language-server checks confirmed TypeScript formatting and non-TypeScript exclusion, with the.gitignorecaveat below. - All eight current GitHub checks pass, including tests on macOS, Linux, and Windows. I did not rerun the full extension-host/R integration suites locally.
| `package.json` sorting are disabled. Oxfmt respects the repository's `.gitignore` | ||
| files, so generated bundles, compiled tests, and dependencies are excluded. |
There was a problem hiding this comment.
Non-blocking documentation clarification: please qualify this as CLI behavior. With Oxfmt 0.73.0 and this PR's .oxfmtrc.jsonc/.gitignore in a fresh Git fixture, oxfmt --check . skips out/generated.ts and dist/generated.ts, but oxfmt --lsp returns formatting edits for textDocument/formatting requests on those same files. Sharing the config therefore does not give editor formatting the same generated-file exclusion. Saying “The CLI also respects the repository's .gitignore…” would match the tested behavior; if exclusion in the editor is intended too, add explicit generated-directory exclusions after the TypeScript re-inclusion patterns.
Summary
Follow up on #1818 and the toolchain review in #1732 by introducing a shared formatter and checking formatting in CI. After the ESLint migration, contributors had no formatting command or CI check; this PR provides both and formats the existing TypeScript sources consistently.
To keep the diff reviewable, formatting is limited to TypeScript files (
.ts,.tsx,.mts,.cts) for now. Other file types can be enabled in follow-up PRs.pnpm run format/pnpm run format:check..oxfmtrc.jsoncbetween the CLI and the already recommended Oxc editor extension. Restrict the scope to TypeScript viaignorePatterns, use four-space indentation and single quotes, and disable import and package metadata sorting. Generated output (out/,dist/) is skipped through the existing Git ignore rules.src/, 4 underhtml/) in a dedicated mechanical commit. Move oneno-control-regexsuppression comment next to the regular expression after line wrapping moved it to another line.formatCI job with Node.js 24, frozen-lockfile installation, andpnpm run format:check. Enable format on save for TypeScript and document the contributor commands.