Lint: enforce publy/arrow-function-components (off → error) - #1247
Merged
Conversation
radandevist
added a commit
that referenced
this pull request
Aug 22, 2026
…(useRender, createElement, jsx, jsxs)
Round-5: the rule missed PascalCase function declarations that return a
call to a known renderer (useRender, createElement, jsx, jsxs) instead of
JSX directly. Base UI components like Badge delegate to useRender({...})
and were invisible to the detector.
Changes:
- Add KNOWN_RENDERERS set and isKnownRendererCallee helper
- Track returnsRendererCall in analyseBody return values
- Update bodyLooksLikeComponent to flag renderer-call returns
- Convert badge.tsx from function declaration to arrow component
- Add 4 invalid + 2 valid Round-5 test cases (red-first)
- Fix detection description and counts in lint-rules.md
Closes the gap identified in PR #1247 review r1.
Convert all function-declaration React components in apps/front/src/components/ to arrow-function expressions (const Foo = () => ...) to satisfy the publy/arrow-function-components lint rule. Co-Authored-By: mimo-v2.5 <noreply@anthropic.com>
Convert all function-declaration React components in apps/front/src/routes/ to arrow-function expressions (const Foo = () => ...) to satisfy the publy/arrow-function-components lint rule. Components referenced before declaration in createFileRoute() are moved above the route export to avoid const hoisting issues (function declarations are hoisted; const expressions are not). Co-Authored-By: mimo-v2.5 <noreply@anthropic.com>
Flip the publy/arrow-function-components rule from off to error in .oxlintrc.json and update docs/guides/lint-rules.md with the new severity and enforcement PR reference (#1210). All 137 offending function-declaration components across 69 files in apps/front/src have been converted to arrow-function expressions in the preceding commits. Co-Authored-By: mimo-v2.5 <noreply@anthropic.com>
…(useRender, createElement, jsx, jsxs)
Round-5: the rule missed PascalCase function declarations that return a
call to a known renderer (useRender, createElement, jsx, jsxs) instead of
JSX directly. Base UI components like Badge delegate to useRender({...})
and were invisible to the detector.
Changes:
- Add KNOWN_RENDERERS set and isKnownRendererCallee helper
- Track returnsRendererCall in analyseBody return values
- Update bodyLooksLikeComponent to flag renderer-call returns
- Convert badge.tsx from function declaration to arrow component
- Add 4 invalid + 2 valid Round-5 test cases (red-first)
- Fix detection description and counts in lint-rules.md
Closes the gap identified in PR #1247 review r1.
…efinite local values The #1210 conversion moved components from FunctionDeclaration (a DEFINITE_LOCAL_DECLARATION_KINDS member, verdict null without body inspection) to VariableDeclaration initializers. resolveValueIdentity then fell through to extractComponentBody, which cannot see through useRender(...) delegations (ui/badge.tsx), and classified those components UNVERIFIABLE - reddening every file rendering them and adding a phantom entry to the inventory. An inline arrow/function-expression initializer is a freshly created local value: it can never be identical to the drawer module's exported symbol, whatever its body renders. Identity is decided here without body extraction; the walk still expands definitions for geometry.
CI's react-doctor gate (0.9.12, --scope files --blocking warning) flags 104 findings on this branch; pairwise comparison vs origin/develop shows all pre-existing debt surfaced by the gate introduced in #1195 - zero new findings come from this branch. Fix the actionable ones. Real fixes (17 warnings): - drawer.tsx: remove permanent will-change utilities - __root.tsx: ref-write moved into useEffect; beforeLoad ordered above head - profiles/audit-logs/tenants/users: ref-writes to useEffect syncs, flatMap replacing filter+push loops, restore missing ref declaration - $tenantId-edit/$userId-edit: effect deps narrowed to stable sources. $tenantId-edit additionally memoizes toStaffTenantDetails on data identity: its fresh-object-per-render return invalidated the form values memo each render, re-triggering reset() in a loop until OOM (caught by a file-by-file suite sweep; 29/29 green after fix) Narrow inline disables for false positives: - profiles.tsx navigate-in-render: handler only fires from callbacks - tenant.tsx hydration-branch/no-event-handler: CSR-only route (ssr:false) using the TanStack post-load redirect pattern doctor.config.json disables three structural rules that mis-fit ratified repo conventions (justification required by docs/guides/react-doctor.md): - no-multi-component-file / only-export-components: route co-location of subcomponents and helper exports is the established TanStack Start layout across apps/front routes - no-giant-component: threshold flags long-standing route shells; decomposition is its own effort, out of Round 3 scope Gates: react-doctor CI-exact exit 0; front typecheck pass; file-by-file unit-suite sweep green; drawer-contrast source guard green in its e2e lane (test:drawer-contrast).
The Round-5 renderer-call detection lived in arrow-function-components.js, which #1207 replaced with a TypeScript rewrite - rebasing dropped the .js file, so this ports the logic into the .ts rule: - KNOWN_RENDERERS set (useRender, createElement, jsx, jsxs) - isKnownRendererCallee: bare identifier or React namespace member form - BodyAnalysis.returnsRendererCall set when a return statement returns a known renderer call instead of JSX - bodyLooksLikeComponent treats renderer-call returns as component-shaped All 490 lint-ts vitest tests pass, including the six Round-5 RuleTester cases added earlier on this branch.
- drafts.tsx: drop the duplicated Route block left by the merge resolution; keep develop's staticData config in the single trailing block, matching the branch's component-first ordering - \$postId/edit.tsx: convert TenantPostEditPage from function declaration to arrow expression (new develop file caught by the now- enforced publy/arrow-function-components) and move its Route block below the component to respect const TDZ Posts suite 19/19, front typecheck and repo lint green.
radandevist
force-pushed
the
chore/1210-arrow-function-components
branch
from
August 23, 2026 07:32
404f18f to
52f63c4
Compare
Round-4 fixes for PR #1247 (verdict .dump/verdict-r3.md): - AGENTS.md: replace the stale "off / not lint-enforced" statement with the enforced status (`error`, #1210) and the owner sentence verbatim: "arrow components; class methods stay methods — `this` binding". - docs/guides/lint-rules.md: add Scope entry for publy/arrow-function-components — the rule never targets class members (methods, getters, static members); it visits only FunctionDeclaration/FunctionExpression, preserving `this` binding. Pins the negative RuleTester case "Class declaration — out of scope for this rule". - Follow-up on the renderer-allowlist blind spot filed as #1283.
This was referenced Aug 23, 2026
radandevist
added a commit
that referenced
this pull request
Aug 23, 2026
…components - Convert the nine page/drawer components to arrow consts; develop flipped publy/arrow-function-components to error (#1247) after our merge base. - Hoist each page component above its createFileRoute block so the Route registration no longer references the const before declaration. - $userId-general: replace hasSavedRef with state; useBlocker's shouldBlockFn runs during render and react-doctor forbids ref reads there. - organizations drawer: useWatch instead of methods.watch for level. - organizations bulk bar: single-pass selection collection replaces chained filter+map (react-doctor/js-combine-iterations). Verified locally against CI-pinned tooling: pnpm lint, tsc --noEmit, pnpm dlx react-doctor@0.9.12 --scope files --base origin/develop --blocking warning (exit 0), guard suites 147/147. Model: ox-alpha
radandevist
added a commit
that referenced
this pull request
Aug 23, 2026
… (#1267) React Compiler follow-ups from #1206: full compiler-skip inventory from a real build in docs/guides/front/react-compiler.md with a per-file decision (9 components rewritten compiler-friendly, 20 documented skips, 49→36 diagnostics); an artifact guard asserting on the BUILT client bundle that the compiler runtime chunk is present and compiled components ≥ 80 % of the measured baseline (90 → floor 72), wired into the front test script, `just ci-front` and the front-ci supply-chain job (ci-gate manifest reconciled); conventions.md corrected (React Doctor is its own oxlint workflow); the 5 SOURCEMAP_BROKEN warnings explained (skipped modules return no map under full-compilation mode). Also pins an i18n-key-coverage guard regression with a red-proof test. Implemented by Ox Alpha via OpenCode Zen (max, one long lane rebased across #1247/#1253). Reviewed adversarially by hy3:free via Nous (APPROVED_WITH_FOLLOW_UPS at 64eb845; annotation-mode switch → guard red; counts reconciled against the PR's own TSVs). Follow-up: one doc path typo (issue opened). Unverified by hand: the 9 rewritten components visually (no behaviour change expected). Closes #1234 Part of #1190
radandevist
added a commit
that referenced
this pull request
Aug 23, 2026
…ntory after rebase (PR 2) - convert the seven migrated page components to arrow consts and move the Route declarations below them, per publy/arrow-function-components (enforced on develop by #1247 while this branch was in flight) - convert the two Probe test components in query-display.test.tsx likewise - register the relocated data-honesty-ignore site in tenants/$tenantId/users/$userId.tsx in suppression-inventory.json (the guard test counts sites per file/reason pair) Part of #1250.
radandevist
added a commit
that referenced
this pull request
Aug 24, 2026
…ntory after rebase (PR 2) - convert the seven migrated page components to arrow consts and move the Route declarations below them, per publy/arrow-function-components (enforced on develop by #1247 while this branch was in flight) - convert the two Probe test components in query-display.test.tsx likewise - register the relocated data-honesty-ignore site in tenants/$tenantId/users/$userId.tsx in suppression-inventory.json (the guard test counts sites per file/reason pair) Part of #1250.
radandevist
added a commit
that referenced
this pull request
Aug 24, 2026
…ntory after rebase (PR 2) - convert the seven migrated page components to arrow consts and move the Route declarations below them, per publy/arrow-function-components (enforced on develop by #1247 while this branch was in flight) - convert the two Probe test components in query-display.test.tsx likewise - register the relocated data-honesty-ignore site in tenants/$tenantId/users/$userId.tsx in suppression-inventory.json (the guard test counts sites per file/reason pair) Part of #1250.
radandevist
added a commit
that referenced
this pull request
Aug 25, 2026
# react-doctor rung 1: re-enable `no-giant-component` Part of #1291 · Closes #1313 Model: Ox Alpha (stealth/ox-alpha via Nous Portal, max effort, jcode) ## What `apps/front/doctor.config.json` has carried three global rule kill-switches since #1247. This rung removes **exactly one line** — `"react-doctor/no-giant-component": "off"` — re-enabling the rule tree-wide at its default severity. No other config change (verified minimal diff). ## Why the config change is justified (per the guard-config review rule) The rule was turned off globally as a shortcut in #1247 because the `--scope files` gate inherits pre-existing debt in every touched file. Every offender is now split below the 300-line budget, so flipping the rule back on produces **zero findings tree-wide**: nothing pre-existing gets inherited by later PRs, which is what made the kill-switch necessary in the first place. ## Before / after | Measurement | Count | |---|---| | `no-giant-component` findings, rung-0 baseline (2026-08-24, react-doctor@0.9.12, full tree, default severity) | **16 / 16 files** | | Findings after this branch | **0** | | Local CI replica: `--scope files --base origin/develop --blocking warning --verbose` | green | ## Proof the rule actually runs Before the flip, a 315-line canary (`src/routes/authed/staff/_giant-canary.tsx`) was planted on the tree: the pinned 0.9.12 scanner flagged it immediately (`no-giant-component`, components must stay ≤300 lines). The canary was removed right after; nothing of it remains in this branch. ## The splits (11 commits, rebased onto current develop) - `data-table.tsx` → toolbar/pagination/empty-state parts - staff details trio (`profiles/$profileId`, `staff-users/$userId`, tenant `users/$userId`) → shared detail shells/views/lifecycle helpers - `tenants-new.tsx` / `$tenantId-edit.tsx` / `$tenantId.tsx` → form sections, stat rows, danger zone - `invitations.tsx` → route-search module (+test), filter-state, table wrapper, drawer hosts; **re-split after #1306 rewrote the page wholesale** (447 → 286 lines), dropping `_invitation-status-label.ts` as a byte-identical duplicate of the helper `_invitation-columns.tsx` already exports - `profiles.tsx` / `profiles/$profileId` tab pages → permission matrix, members card, identity header - `_assign-members-drawer.tsx`, `_profile-permissions-tab.tsx` → selection/list parts - `tenant/settings/general.tsx` → form field groups During conflict resolution, develop's parallel refactors (#1306, #1295, #1292) were preferred wherever they superseded ours; our extractions were re-applied only where develop's version still exceeded the budget. Bulk-action logic in `tenants/$tenantId/users.tsx` verified identical to develop's inline version before extraction. ## Round 1 fixes — what failed in e2e and why (d315142) An earlier DONE claim on `db5f815e2` was wrong: `front-e2e` shards 2–4 were RED with five failing specs: | Spec | Symptom | |---|---| | `parity-happy-path.spec.ts:221` | after clearing the staff-users search, only the filtered row remained; URL no longer had `q=` | | `staff-profiles.spec.ts:122` | same symptom — "Staff Owner" row not restored after clear | | `staff-tenants.spec.ts:201` | resetting the status filter did not restore "Acme Corporation" | | `staff-tenant-details.spec.ts:867` | level filter expected `disabled` during row selection, received `enabled` | | `table.spec.ts:150` | pager "Previous" did not return to page 1's row | **Root cause — one class for all five:** the DataTable split extracted grid/header children that read the TanStack table instance during render. That instance is mutable and identity-stable across renders, so under the React Compiler (active in production builds via `viteReact({ compiler: true })`, which is how e2e serves the app) `DataTable`'s JSX children were memoized: a parent-only re-render no longer re-executed their bodies. After a search clear, filter reset, or pagination move the page state and toolbar were fresh while the rows stayed frozen on the previous query's data. Plain vitest never reproduces this because unit transforms do not run the compiler pass — the repro required a compiler-enabled vitest config over the real components (fails exactly like CI without the directive, passes 43/43 with it). **Fix:** `'use no memo'` on `DataTable`, `DataTableGrid` and `DataTableHeaderRow`, each with a justification comment pointing at the full rationale in `data-table.tsx`. No test edits, no skips/retries/timeout changes; every component stays ≤300 lines. ## Resume/rebase (2026-08-25, merge commit 1ab3f51) The previous session died on a provider error mid-flight, leaving ~110 files staged from an **unfinished `git merge origin/develop`**. Resumed from disk instead of restarting: - Re-read every staged hunk: the staged tree was exactly the in-flight merge resolution porting develop's anti-slop rungs 4+5 (#1329), the `no-iife` conditional/logical callee peeling (#1332) and the real-route `<Trans>` render guard coverage (#1312) into this lane's split modules. Completed the merge commit `1ab3f5145` keeping BOTH parents (85e6037 × 301f2b8); the two loose files still in the worktree belonged to the same port and were folded in. Nothing half-finished was kept. - `.oxlintrc.json`: **tightening only** — `anti-slop/no-chained-type-assertions` and `anti-slop/no-unknown-returns` go `off` → `error`. No rule disabled or loosened anywhere. - The tightened rules surfaced 11 leftovers in modules only this lane has (split files with no develop counterpart). All fixed at root cause, no suppressions: mutation contracts retyped to named domain results (`GetStaffUserByIdResult`, the suspend/reactivate/remove result union) instead of `Promise<unknown>`; chained `as unknown as` casts replaced by single named widening points (`widenFake`, `componentOf`) or fully typed literals; the matchMedia test mock now builds a real `MediaQueryList` shape without an assertion chain. Verification, all re-run on the merge result: - `pnpm lint`: 0 errors with rungs 4+5 at error - `pnpm --filter front typecheck`: green - Full front unit/component suite: **235 files / 2530 tests green** (includes the design-system, z-index and React Compiler artifact guards) - `just react-doctor`: no findings tree-wide, `no-giant-component` blocking - `pnpm format`: clean; `just ci-drift`: green - e2e intentionally not run locally (verification policy); CI runs front-e2e 4/4 on this PR and that is the acceptance evidence ## Verification - `pnpm --filter front typecheck`: green - Full front unit/component suite (`pnpm --filter front test` under `heavy.sh` serialization): green - Full-tree react-doctor scan + local CI replica gate (`--scope files --base origin/develop --blocking warning`): green, exit 0 - Compiler-parity repro (temporary uncommitted vitest config with `compiler: true`): A/B proves the directive is load-bearing — 1 stale-row failure without it, 43/43 green with it - e2e intentionally not run locally (verification policy); CI runs front-e2e 4/4 on this PR and that is the acceptance evidence - develop moved repeatedly mid-flight (#1298, #1307, then #1305/#1306/#1310/#1314/#1328): merged forward each time. The last round did overlap (QueryDisplay contract, profiles dialog props, the `react/refs` override file, `no-iife`) and was absorbed by migrating our split pages to develop's patterns and re-porting develop's fixes into the split modules (`a36e04d4a`, `d9ed413ce`), keeping every component ≤300 lines - 2026-08-25: merged forward again over #1312/#1329/#1331/#1332 (see Resume/rebase above); all gates re-run green on `1ab3f5145` Two findings the scoped gate raised against our own extracted modules were fixed at root cause in this branch (no suppressions): the staff-user submit handler returned an explicit outcome instead of calling an injected `navigate()` outside a component, and `EditAccessSection`'s six boolean props collapsed into grouped query/pagination objects. ## Round 2 review fixes — suppression debt eliminated at root cause (2026-08-25) Round-1 review required: rebase onto current develop, zero net-added disable directives vs origin/develop, root-cause fixes only. Three gaps were real and are fixed: 1. **Five lane-introduced `eslint-disable` lines.** The earlier splits had *relocated* suppressions from develop files into the new modules (2× `exhaustive-deps` from `data-table.tsx` → `use-matched-breakpoints.ts`, 1× `no-unused-vars` from `data-table.tsx` → `column-display-meta.ts`, 1× `exhaustive-deps` from `tenants-new.tsx` → `_tenants-new-form.tsx`, 1× from the old giant `users.tsx` → `_assign-members-state-hook.ts`, 1× from `_profile-permissions-tab.tsx` → `_profile-permissions-save-hook.ts`). All five removed without any suppression: - `use-matched-breakpoints.ts`: both callbacks re-derive the breakpoint array from `key` (`key.split(',').map(Number)`), so `[key]` is a complete dep array. - `_tenants-new-form.tsx`: resolver memo deps completed to `[t, i18n.language]`. - `_assign-members-state-hook.ts` / `_profile-permissions-save-hook.ts`: effects already guard on their own bookkeeping refs, so deriving the id/key sets from the stable key string inside the effect (`rowAccountIdsKey.split(',')`, `grantedSignature.split(',')`) makes the arrays complete with identical behavior. 2. **Five navigate-in-render disables** (from round 1): URL writers wrapped in `useCallback`, which is exactly the lexical contract the rule checks; directives deleted. 3. **Stale base**: rebased again onto `d26a1f17b` (#1358, lint-ts/docs only, no overlap), 26 commits replayed cleanly. ### Directive parity proof (A/B vs origin/develop, `git diff origin/develop...HEAD -- apps/front/src`) | Direction | Lines matching `eslint-disable|react-doctor-disable` | |---|---| | Added | **0** | | Removed | **9** | ### Full-tree react-doctor A/B (pinned 0.9.12, dedicated pristine-develop worktree `/tmp/wt-develop`, both under `heavy.sh`) | Measurement | origin/develop | this branch | |---|---|---| | Files scanned | 655 | 749 | | Score | 66 / 100 | 73 / 100 | | Findings total | 48 | 41 | | Finding families | 20 | 17 | | `no-giant-component` | n/a (rule off in develop's config) | **0** (rule enabled by this PR) | Every family count on the branch is ≤ develop's; six families that exist on develop (boolean-prop-combos, pure-function-per-render, loading-flag-outside-finally, prop-callback-in-effect ×2, second parent-sync family, one unused-file/export pair) are gone on the branch. Remaining 41 findings across 17 families are pre-existing develop debt in files this PR does not touch (e.g. `_invite-user-drawer.tsx`, `_change-email-dialog.tsx`, `app-shell.tsx`, `marketing-header.tsx`, `use-hydrated.ts`, `broadcast-sync.ts`, `confirm-dialog.tsx`, `login.tsx`, `floating-selection-bar.tsx`, `offset-pagination.ts`, `field-checkbox-group.tsx`) plus the known untouched `tenants-new-helpers.ts` iterations family and `_profile-edit-details-drawer.tsx` (#1335, exists identically on develop). ### Gates, all re-run at the pushed tip - `just ci-front` under `heavy.sh`: **green** — 238 test files / 2563 tests, design-system/z-index/compiler-artifact guards included - Local CI replica `pnpm dlx react-doctor@0.9.12 --scope files --base origin/develop --blocking warning` under `heavy.sh`: **Scanned 126 files · Score 100/100 · No issues found**, exit 0 - `pnpm --filter front typecheck`: green - Targeted suites for every touched area during development: 168/168 (data-table, tenants-new, assign-members trio, permission matrix, profile details) - e2e intentionally not run locally (verification policy); CI runs front-e2e 4/4 on this PR and that is the acceptance evidence ### Post-push CI status (honest transcript, 2026-08-25 ~06:00Z) The `front e2e` **build job** fails on this PR's tip with an infrastructure error, not a diff error: after all four images build successfully, the step "Push stack images to GHCR" is denied with verbatim `denied: permission_denied: The requested installation does not exist.` — the runner's credential no longer has access to the personal namespace `ghcr.io/radandevist/publyapp-e2e`. A `--failed` re-run reproduced the identical denial. Two neighboring PRs hit the same step failure in the same window (05:32Z and 05:40Z), while one run at 05:34Z succeeded — the breakage started between 05:34Z and 05:46Z today and is shared across PRs. Nothing in this branch touches `.github/workflows` or image definitions; the fix belongs to the registry credential owner (restore the GitHub App/PAT installation for `ghcr.io/radandevist/publyapp-e2e`). Everything else is green: `just ci-front` (238 files / 2563 tests) re-run locally under `heavy.sh` at the pushed tip, `just ci-drift` green, typecheck green, scoped react-doctor CI replica 100/100. --- Implementer: Ox Alpha (Nous Portal, jcode, max effort). Reviews: r1 CHANGES_REQUIRED (suppression debt, false tree-wide claim), r2 APPROVED by hy3 (Go, last rung after the free chain) at ebbe48d: 0 suppressions added / 9 removed, react-doctor 0 no-giant-component tree-wide; tip 32d98c9 = ebbe48d + develop merge (update-branch after #1396); full CI green incl. front-e2e 4/4.
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
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.
Round 4
Round-3 blocker fixes (owner item 7)
publy/no-opandpubly/arrow-function-componentsare off …" statement with the enforced status (error, Lint: enforce publy/arrow-function-components (convert the offenders, off → error) #1210) and the owner sentence verbatim: "arrow components; class methods stay methods —thisbinding".publy/arrow-function-components): added a Scope entry — the rule never targets class members (methods, getters, static members) because it visits onlyFunctionDeclaration/FunctionExpression, so class bodies are untouched and methodthisbinding is preserved; pinned by the negative RuleTester case "Class declaration — out of scope for this rule" inpackages/lint-ts/src/rules/arrow-function-components.test.ts.functiondeclaration rendering through a non-allowlisted renderer such ascustomRenderis invisible): arrow-function-components: PascalCase function declarations rendered through a non-allowlisted renderer (e.g. customRender) are invisible; document the renderer allowlist as exhaustive or widen it #1283 (Part of Lint: enforce publy/arrow-function-components (convert the offenders, off → error) #1210).Verification at tip
pnpm lint— green (exit 0, Node v24.19.0)just ci-drift— green (AGENTS/docs guards, 18 checks pass)Closes #1210
Model: rounds 1-4 Ox Alpha via OpenCode Zen (max); reviewers hy3 (r1-r2) and hy3:free via Nous (r3).