Skip to content

refactor(ui): stabilize useThemeColors return identity (DRC-3328) - #1549

Open
gcko wants to merge 3 commits into
mainfrom
feature/drc-3328-stabilize-usethemecolors-return-value-with-usememo-module
Open

gcko wants to merge 3 commits into
mainfrom
feature/drc-3328-stabilize-usethemecolors-return-value-with-usememo-module

Conversation

@gcko

@gcko gcko commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

PR checklist

  • Ensure you have added or ran the appropriate tests for your PR.
  • DCO signed

What type of PR is this?

refactor

What this PR does / why we need it:

useThemeColors() rebuilt its return object and all five nested sections (background, text, border, status, interactive) on every call. Any consumer that placed a section in a useMemo / useCallback dependency array would have been invalidated on every render.

This hoists the light and dark values into two module-level tables and returns a single useMemo keyed on the resolved mode and the MUI theme. Section references are now stable for as long as the mode is unchanged, and flipping light → dark → light returns the original references.

Color values are unchanged. All 20 leaves in each mode were mechanically compared against the previous ternaries.

Which issue(s) this PR fixes:

Closes DRC-3328

Special notes for your reviewer:

  • This is hygiene, not a measured perf win. No in-repo consumer currently threads a section into a dependency array, so no metric moves today. The ticket's April 28 comment recommended deferring until RUM telemetry landed; this ships it anyway as a cheap, behaviour-preserving fix to a public hook.
  • Public type change: ThemeColors leaves were previously literal unions inferred from the as const palette (for example "#171717" | "#FFFFFF"). They are now string. This is a widening, so assignments into sx, color, or any string slot are unaffected. Workspace type-check (root, ui, storybook) passes.
  • isDark is always false on first render (the mounted guard), so the memo keys on the resolved mode rather than the raw inputs. The tests assert identity only within a mode.
  • The test file's mocked MUI theme is hoisted to a single constant via vi.hoisted so the top-level memo is observable. Production MUI already memoizes the theme object.

Verification:

pnpm vitest run packages/ui/src/hooks packages/ui/src/components/lineage/__tests__/LineageView.test.tsx packages/ui/src/components/errorboundary
  Test Files  11 passed (11)
       Tests  127 passed (127)
pnpm type:check   # root + @datarecce/ui + @datarecce/storybook: clean
pnpm biome check  # both files: clean

Does this PR introduce a user-facing change?:

NONE

🤖 Generated with Claude Code

gcko and others added 2 commits September 11, 2026 13:12
useThemeColors rebuilt its return object and all five nested sections on
every call, so any consumer placing a section in a useMemo/useCallback
dependency array was invalidated every render.

Hoist the light and dark values into two module-level tables and return
a single useMemo keyed on the resolved mode and the MUI theme. Section
references are now stable while the mode is unchanged, and a
light -> dark -> light round trip restores the original references.

Color values are unchanged; all 20 leaves per mode were mechanically
compared against the previous ternaries. ThemeColors leaf types widen
from palette literal unions to string.

Tests: four identity-stability cases added (same-mode rerender for
light and dark, top-level object identity, and the mode-flip round
trip). Each was watched failing on Object.is before the change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Jared Scott <jared.scott@datarecce.io>
@gcko gcko self-assigned this Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant