Repository navigation
Conversation
Contributor
|
|
…d the compiler gate oxlint's native rule costs nothing measurable, but unlike ESLint's copy it cannot be gated on the React Compiler, so it reports 115 findings ESLint never shows: 69 that ESLint's processor drops in dual-memoized files, 26 already disabled for ESLint above the declaration line, and 20 the native port gets wrong. Those are suppressed inline, each with the reason it exists. Adds the tooling behind both numbers: compareNativeCtxValues.py diffs the native rule against ESLint's own, line by line eslint-ctx-values-rule.mjs hosts ESLint's rule so one oxlint run reports both ctxValuesSuppressionCodemod.py writes the suppressions, refusing any it cannot explain memoizedFiles.mjs batch React Compiler memoization check
…t-values rule Inserting a comment line above the reported line orphaned the eslint-disable-next-line that already sat there, so rulesdir/context-provider-split-values started firing again and the repo went from 4629 findings to 4631. Two sites, both fixed by putting the oxlint directive on the existing comment line as the repo's same-line combo instead. The codemod now refuses to guess if it meets a displaced directive it cannot combine with. Also drops react/jsx-no-constructed-context-values from the port plan, since it runs natively now. The coverage checker's actionable list goes from 7 rules to 6.
react/jsx-no-constructed-context-values and rulesdir/no-inline-useOnyx-selector were the last two
rules ESLint enforces that oxlint did not. Both are suppressed wholesale by ESLint's
react-compiler-compat processor in files that both React compilers memoize, and oxlint has no
processor, so neither could be switched on without reporting findings ESLint never shows: 69 and
238 respectively.
reactCompilerGate.mjs puts that decision where oxlint does have a hook, inside a JS plugin rule
wrapping context.report. Same didBothCompilersMemoizeFile call the processor makes, run lazily on
the first report in a file and cached, plus the processor's tests/ and node_modules/ skips, which
rh-plugin.mjs had been missing. rh-plugin.mjs now imports the shared module instead of carrying
its own copy, so all three plugins share one cache and one compile per file.
The context rule is hosted from eslint-plugin-react rather than native for two reasons: only a JS
plugin can hold the gate, and oxlint's native port diverges twice (it anchors on the JSX provider
instead of the declaration, and it does not require the value to be declared in a component).
Going native was built and measured first: it needs 115 inline suppressions plus one for every
context provider written from then on, because a comment cannot ask the compilers. Reverted.
Testing, since all these rules report zero at parity and zero proves nothing on its own:
checkReactCompilerGate.py a matched pair of probe files, memoized and 'use no memo', asserting
both tools agree on both answers. Verified green-red-green: with the
gate removed, the memoized case fails.
the two fixtures now opt out of memoization, so they still test the rule and not the
gate. port-plugin.mjs is gone; the staging area is empty.
Codecov Report✅ All modified and coverable lines are covered by tests. |
# Conflicts: # cspell.json
…he parity direction check `oxc-transform-react` 0.152.0 adds the opt-in `reportDiagnostics`, which returns the recoverable React Compiler diagnostics without forcing them fatal (oxc-project/oxc#26624). That is what `panicThreshold: 'all_errors'` was faking: making every diagnostic fatal to recover the non-fatal ones, at the cost of aborting on the first failing function per file, so findings surfaced one pass at a time. Swapping the two takes `set-state-in-effect` from 47 of ESLint's 120 to 120 at the same locations, and `TwoComponents.tsx` now reports all three components in one pass, as ESLint does. The three `blockedUpstream` fixture pins, the `NON_FATAL_IN_ISOLATION` tripwire and the repro file that documented the miss go with it. oxlint 1.86.0 and 7.0.2003 are re-measured rather than assumed: `set-state-in-effect` is exact, `refs` reports 189 against ESLint's 210 (multiplicity, plus three locations), and tsgolint infers stricter than typescript-eslint on `no-unsafe-type-assertion` (+53) and `no-unsafe-return` (+8). The baseline is regenerated for all of it, exact at 4194. Adds `oxlint-parity-direction`, which re-cuts the whole-repo report by direction at the granularity the seatbelt enforces. An oxlint-only finding only tightens the gate; an ESLint finding oxlint lacks is a check that stops running the day the gate flips, and nothing fails to say so. 533 hidden findings, all three sources accounted for: the `HeaderWithBackButton` ban `oxlint.config.mts` does not mirror (502), `refs` duplicate collapsing (25), and the 83-file write-site override whose own comment already prices its 6. The drift checker cannot see the ban because none of its probe files imports it, so its ACCEPTED entry now says so. Both `min-release-age=7` exclusions name the binding scopes as well as the packages: npm skips a missing optional dependency silently, so excluding only the package can leave a stale binding installed.
…nt's end line ESLint rejects only a multi-line `eslint-disable-line`; a multi-line `eslint-disable-next-line` is applied to the line after the comment closes (linter.js `comment should not span multiple lines` guards the `-line` spelling alone). The module skipped both spellings and anchored `-next-line` to the comment's start line, so a directive written across lines suppressed nothing for a hosted rule. No directive in the repo spans lines today, so no finding moves; `npm run oxlint-eslint-directives` goes from 21 to 22 of 22.
…ails exemption `main` renamed `src/libs/ExportOnyxState/common.ts` to `masking.ts`, and the ESLint exemption block followed while this config's copy did not. The dead pattern exempted nothing, so oxlint reported `masking.ts` and ESLint did not -- `rulesdir/no-direct-personal-details-list` 21 against ESLint's 20. The drift checker reads the rule's options on its probe files and none of them is this file, so it kept reporting 0 open. Point the pattern at the file that exists, re-run `SEATBELT_INCREASE=all`, and the rule is 20/20 with the baseline one row tighter at 4193.
…ay harness `rulesdir/no-direct-personal-details-list` was the one enabled sidecar rule with no evidence it runs: its RuleTester lives in tests/unit, which jest runs against ESLint only, and the harvester collects `ruleTester.run` cases from eslint-plugin-local-rules/tests. Mirror the cases into that directory, the way no-direct-pre-insert-fullscreen-under-rhp already does, and all 18 replay through oxlint's bridged AST with parity on (file, line). jest's testMatch takes .ts and .tsx only, so the two harnesses do not double-run them and `oxlint-sidecar-coverage` reports 193/193.
…ed counts Both remaining harness items are gone, so their TODOs come out and the rest renumber. The personal-details rule is 20/20 now that the exemption names the file that exists, which moves the whole-repo counts to 3452 against 4193, the baseline to 1895 rows exact, and the direction split to +1274 added against 533 hidden. Section 4 records why two directories hold rule tests: jest takes only .ts and .tsx, so tests/unit runs against ESLint and the harvested .test.js is what puts the same cases through oxlint's AST. The `test:bun` row said 595 pass / 0 fail, which is no longer true: 644 pass across 48 files with 3 failures in SyncVersions.test.ts. Those three fail the same way at HEAD with this branch's work stashed, twice in a row, so the row names them as pre-existing git-fixture cases instead of claiming a clean run. Timings are re-timed rather than carried over, including one process against four shards to confirm the 4193 multiset is shard-independent.
…atterns ESLint matches the ban with gitignore semantics so a bare directory name covers everything below it; oxlint matches paths exactly and needs a second /** entry. Closes the 502-finding parity delta, resyncs the oxlint seatbelt baseline to 4695, and updates the migration state doc and drift ledger to match.
The seatbelt tightens but never increases on its own, so the merge from main needs the documented SEATBELT_INCREASE=all pass (TODO 4). Upstream adds the gesture-handler 3.3 deprecations and new files; baseline goes 2390/4695 to 2427/4769 with 1 row removed for a deleted file.
Rerun every harness number at the new pins: ESLint 3528 and oxlint 4769 over 9896 files (was 3452/4695/9887). The gesture-handler 3.3 bump brings 82 Gesture deprecations, which is most of the no-deprecated move to 259/461. Direction re-reads 1822 pairs identical, +1272 added, 31 hidden -- same 31. Cold timings 375s/33s, warm 412s/92s. Re-measured the deleted-override cost at 695 exactly, and re-counted Tier B at 128 rules with 288 native. The refs gap, purity 35, coverage 483/482/475 and both baselines-exact all hold.
…o mirror production
…SLint probe with production
…r, refresh fixture coverage
44 of 45 tasks
…0, oxlint-tsgolint to 7.0.2004
# Conflicts: # cspell.json # knip.json
…deprecations tsgolint misses
… Python harness reads
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation of Change
Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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