Repository navigation
[NoQA] Oxlint linting - #101292
[NoQA] Oxlint linting#101292LukasMod wants to merge 33 commits into
Conversation
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 82f9a6d6fd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 568fc81eee
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
7e3e961 to
bd07eb1
Compare
|
Hey, I noticed you changed If you want to automatically generate translations for other locales, an Expensify employee will have to:
Alternatively, if you are an external contributor, you can run the translation script locally with your own OpenAI API key. To learn more, try running: npx bun ./scripts/generateTranslations.ts --helpTypically, you'd want to translate only what you changed by running |
bd07eb1 to
2b8377c
Compare
Main added files that trip the type-aware rules (NumericInput primitives, applyColumnWidthOverrides) and fixed the Gesture.Pan deprecations, so re-measure the baseline: 7 findings added, 8 stale entries dropped.
…rkaround comments
|
@MelvinBot review |
ReviewI found no blocking bugs. The default Worth fixing or calling out
Minor parity gaps (each verified in code)
Checked and looks correct
|
|
Working on Melvin findings |
…resolve TypeScript
…y, compared by column
…n in the file instead of ignoring it
|
This PR adds a new |
|
I marked it as a draft until all feedback is not resolved so actions will not be triggered |
…int message so the reviewer bot ignores it
|
@roryabraham Thanks for the review, I think everything is covered now. I updated config regarding latest changes on main |
Explanation of Change
Adds Oxlint as a second linter behind the existing
npm run lint, non-blocking in CI. ESLint staysthe gate. Nothing about how you lint locally changes yet.
Why
Oxlint lints the whole repo in about a tenth of ESLint's time, on half the runner. Failing runs
widen the gap: the ESLint job clears its cache and lints again from scratch to rule out stale
results, and Oxlint has no cache, so it never retries.
With a warm cache the two cost about the same wall time, at half the runner for Oxlint. Cold or
failing, Oxlint is about 17 to 19 times cheaper in vCPU-seconds.
Side by side (2 weeks, Oct 6-20, 2026)
How it fits
scripts/lint/index.tsruns aLinter -> Processor[] -> Formatterpipeline. Both linters implementthe same
Linterport and share the seatbelt, the formatter and the exit codes. Each keeps its ownbaseline:
config/eslint/eslint.seatbelt.tsvandconfig/oxlint/oxlint.seatbelt.tsv. The Oxlintjob runs on every PR with
continue-on-error. While it is a shadow job, the post-merge run onmainupdates its baseline in both directions (
SEATBELT_INCREASE=all): findings a merge adds are absorbedand fixed ones are removed, and OSBotify commits the result as "Auto-update oxlint-seatbelt baseline".
A PR is then warned only about findings it adds. Runs started by OSBotify's own commit only tighten,
so the baseline settles instead of looping. Once Oxlint is the blocking linter,
maingoes back totighten-only, as ESLint's does.
Tool versions: oxlint 1.89.0, oxc-transform-react 0.155.0, oxlint-tsgolint 7.0.2003.
Workarounds
What each unusual part of the diff is for.
OxlintLinterruns N single-threaded shards plus one type-aware processoxlint-tsgolint, which builds a multi-GB TypeScript program.OxlintLinterderives one JSON config per leg fromoxlint.config.mtsat the start of a run and deletes both at the end. The sharded run reports the same findings as a single processconfig/oxlint/plugins/eslintDirectives.mjsmakes hosted rules honour their ESLint id in disable commentsreact,react-hooks,importand@typescript-eslintprefixes, so an existingeslint-disable-next-line react/jsx-no-bindwould not suppresshosted/jsx-no-bind. It follows ESLint's placement rules too:eslint-disable/eslint-enablecount only in block comments, and a directive on the same line as a report is compared by columnoxc-transform-react, withreportDiagnostics: truereact/*compiler rules drop every diagnostic in a component once oneeslint-disablenamesreact-hooks/exhaustive-depsorrules-of-hooks(oxc#26277). Hosting eslint-plugin-react-hooks as a JS plugin works but is slow: its JS compiler took 52 s of a 121 s whole-repo run, against 5.2 s for the Rust one. WithoutreportDiagnosticsthe binding returns only fatal diagnostics (oxc#26318), so non-blocking rules likeset-state-in-effectlooked clean. With it,set-state-in-effectreports the same locations as ESLintreactCompilerGate.mjs) wraps three hosted rulesjsx-no-constructed-context-values,no-inline-useOnyx-selectorand the useCallback/useMemo advice fromexhaustive-depsare false positives in files both React compilers memoize, and the fix they suggest is banned there. Oxlint has no processor, so the gate drops them inside the plugin, which also keeps them out of the editor. In the pipelineReactCompilerFilterapplies the same rule to either linter's outputeslintSuppressionRules: []eslint-disable-next-line react-hooks/exhaustive-depscomment, and the repo has 230. Lint only: setting it in the build configs would change what shipshosted/no-deprecated-self-closing-jsxstands in for the@deprecatedcheck on<X />JSXOpeningElement). The stand-in reports under the@typescript-eslint/no-deprecatedid, so findings file onto the same per-symbol seatbelt rows, and it is enabled in the same scopes as the typed rule. EveryMenuItemusage in the repo is self-closing. Delete the stand-in when tsgolint ships the fix; a fixture already fails on that signalcategories: {correctness: 'off'}eslint.config.mjs, so Oxlint's default category cannot switch on rules ESLint does not run. Anextendsor arecommendedcategory would also pull in the nativereact/*compiler rules, without a warning (oxc#26277).tsxwork into one shardnode_modules/.bin/oxlint, nevernpx oxlintnpxpasses one string tosh -c, and a shard list exceeds Linux's per-argument cap--fixruns a single process, no shards--fixis manual-only (not used by CI, hooks orlint-changed), so the slower run costs nothing day to day--shards=auto|N,autoby default from cores andvm_staton macOSautotakes half the cores (measured fastest on 14 cores) and what memory allows. Node's free-memory reading counts free pages only and sizes the run down to one shard on an idle machine. CI passes--shards=4;0or anything invalid exits with an errorruleID: null,Parsing error: ...) and the other findings survive; a crash, which has no location, fails the runruleNames.mjsmaps Oxlint diagnostic codes back to ESLint rule idshosted(naming-convention)is@typescript-eslint/naming-convention,typescript(...)is@typescript-eslint/..., plus two renamed core rules. A bun test fails if the mapping driftsimport/resolverandimport/extensionssettings copied from ESLintno-useless-path-segmentsandno-relative-packagescannot resolve.ts/.tsxtargets and stay silentpluginsarray against Oxlint's schemaHeaderWithBackButtonimport ban has twogroupentriesno-unsafe-*andno-throw-literaloff underscripts/**,.github/**no-deprecatedoff on a list of 83 filesunicorn/prefer-atreplacesrulesdir/prefer-atx[x.length - N]and misses plainarr[0]. One of two rule downgrades we accepted. The other isboolean-conditional-rendering, which needs type informationjsPluginscannot see yetpackage.jsonoverridespinsvite-plusto^0.1.15vite-plus: "*"peer, and the latestvite-pluspulls avitest@5chain that requires@types/node >= 22while the repo pins 20. Without the pin, installs and every future oxlint bump fail with ERESOLVE. Nothing importsvite-plus. TEMPORARY until@types/nodereaches 22+.npmrc'smin-release-age-excludemin-release-age=7would block the current oxlint and oxc-transform-react releases. The native binding scopes are listed too, so npm does not keep a stale binding without telling anyone. TEMPORARY: both releases clear the hold on 2026-10-26 after 17:24 UTC, and a follow-up removes the four lines if this merges firstParity, whole repo
Both linters run through the same pipeline: ESLint 3668 findings, Oxlint 5038.
After the latest merge from
mainthe baselines hold:Read by direction, at the seatbelt's granularity (file, rule, count):
react-hooks/refsacross 10 files. Most are duplicate collapsing: the compiler emits the samediagnostic several times at one location, and the bridge keeps fewer copies. The bridge misses 3
locations (
__mocks__/react-native-safe-area-context.tsxx2,AboutPage.tsx), and onlyAboutPage.tsxends up with no
refsfinding at all.no-deprecatedread sites (getReportTransactions, 4 files undersrc/libs/actions/) inside the83-file override above, an accepted cost.
Where the 1398 extra findings come from:
no-unnecessary-type-assertion(805)and
import/no-cycle(153).no-deprecatedwrite sites (+251),no-unsafe-type-assertion(+60),no-unsafe-return(+8). Kept and grandfathered.preserve-manual-memoization(+43).react-hooks/purity(+28), allDatecalled during render. ESLint's compiler build misses these realimpure reads; we keep them to clean up over time.
import/no-named-as-defaultthe outlier at 13.
unicorn/prefer-atandno-unsafe-optional-chainingadd 4 each with no ESLintcounterpart.
rulesdir/boolean-conditional-renderingis the one rule with no Oxlint counterpart that we lose.no-invalid-thisisnot run either, because Oxlint's plugin bridge throws on the API it calls.
Plan
This PR bumps
oxc-transform-react, the React Compiler the web build and Jest use, to 0.155.0 (the bridge needsreportDiagnostics, added in 0.152.0 by oxc#26624). The.npmrcexclusion is only needed until oxlint 1.89.0 andoxc-transform-react 0.155.0 are past npm's 7-day hold, on 2026-10-26 after 17:24 UTC. If this merges before
then, a follow-up PR removes the four
min-release-age-excludelines.main, see above), watch it for a day, and compare the results between ESLint and Oxlint.SEATBELT_INCREASE,continue-on-errorand the report step fromoxlint.yml, addoxlinttoconfirmPassingBuildinpreDeploy.yml, makeoxlintthe default inscripts/lint/index.ts, and review the absorbed baseline first.hosted/*id, deleteeslintDirectives.mjs, and turn on Oxlint's unused-disable reporting.Workarounds to delete when upstream moves: the bridge when oxc#26277 is fixed, sharding when oxc#26621 is, the self-closing JSX stand-in when tsgolint ships its fix, the
vite-pluspin when@types/nodereaches 22.POC PR with migration scripts and tests: #98027
Fixed Issues
$ #104516
PROPOSAL:
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari