chore(client): scrub style={[...]} array patterns on DOM-leaf primitives + ESLint guardrail #239

New issue

Closed

opened 2026-06-23 12:16:19 +00:00 by james · 0 comments

james commented

2026-06-23 12:16:19 +00:00

Owner

Context

PR #209 surfaced a runtime crash on the universal client: clicking certain elements threw CSSStyleProperties doesn't have an indexed property setter for '0' (Firefox wording; same bug on Chromium). Root cause: React DOM's setValueForStyles iterates the style prop with for-in and writes node.style[key] = value. When the input is a style array (or RNW passes through a nested array via <Link asChild> / similar merging), for-in iterates numeric indices and the DOM rejects them.

PR #232 and #211 adopted the safe pattern in the nav shell, login server row, and a few other places:

// Avoid:
<View style={[styles.a, cond && styles.b, { color: theme.tokens.text }]} />

// Prefer:
const viewStyle = cond
  ? { ...styles.a, ...styles.b, color: theme.tokens.text }
  : { ...styles.a, color: theme.tokens.text };
<View style={viewStyle} />

The bug doesn't reproduce in every style={[...]} site — most paths through RNW flatten correctly. But the failure modes are subtle (specific Link/Pressable merging contexts; certain prop-spread chains), so leaving array sites in place is leaving land mines. The agent for #235 noted that login.tsx's pre-existing form elements still use style={[...]} and only the new LoginServerRow adopted the safe shape — that's a representative snapshot of the codebase today.

Source

Follow-up flagged in PR #238 and traced back to the runtime bug fixed in PR #232.

Scope

Audit pass. rg 'style={\[' apps/client/ to enumerate every style={[...]} site. There are dozens; the migration is mechanical.
Migration pass. For each site:
- If the array is [styles.a, styles.b] with no conditionals → { ...styles.a, ...styles.b }.
- If the array has cond && { ... } entries → pre-compute the merged object outside the JSX, or use a ternary.
- Multi-line arrays with theme overrides → resolve the merged style into a named local (const containerStyle = ...) before handoff.
- Don't touch style={[...]} props on react-native-internal non-leaf components where the array reliably flattens (e.g. <FlatList contentContainerStyle={[...]}>); the audit's focus is DOM-leaf primitives (View, Text, Pressable, TextInput, Image, ScrollView).
Guardrail. Add an ESLint rule (custom, or a no-restricted-syntax-style configuration) that flags style={[...]} on the DOM-leaf primitives. Allow an opt-out comment for cases where an array is the right call (e.g. animated styles via Reanimated). The rule's error message should link to this ticket so future contributors learn the pattern.

The scope is limited to apps/client/. The deleted Next.js UI's leftover styles in apps/api/app/themes/preferences.ts are pure data, not affected.

Files most likely impacted

(Grep results from a recent audit — confirm during implementation.)

apps/client/app/login.tsx
apps/client/app/register.tsx
apps/client/app/server-setup.tsx
apps/client/app/(app)/notes.tsx
apps/client/app/(app)/profile.tsx
apps/client/app/(app)/skills.tsx
apps/client/app/(app)/experience.tsx
apps/client/app/(app)/account.tsx
apps/client/lib/Placeholder.tsx
A handful of subcomponents in apps/client/lib/nav/ already migrated; double-check completeness.

Acceptance criteria

rg 'style=\{\[' apps/client/ --type tsx returns nothing on DOM-leaf primitives, or returns only flagged exemptions (Reanimated-style array contexts).
ESLint rule rejects new style={[...]} on DOM-leaf primitives with a clear error message pointing here.
No visual regression on web or native — the migration is style-shape only.
pnpm -F @carol/client test + export:web stay green.
Brief note added to CONTRIBUTING.md "Conventions" pointing at the rule.

Out of scope

Refactoring deeper structure (extracting subcomponents, renaming files) — this is a style-shape migration, not a redesign.
Animated styles via react-native-reanimated's useAnimatedStyle — those legitimately return objects already; if any site uses an array of an animated style + a static style, leave it but verify it doesn't trip the bug.
The (deleted) Next.js UI — gone with #185.

Composes with

PR #232 — original fix + the defensive pattern documentation.
#233 — unit tests for the URL rewriter (the other "PR #232 didn't get test coverage" follow-up).
#234 — native E2E smoke (would catch any visual regression introduced by this migration).

Part of

Standalone — no parent epic.

## Context PR [#209](https://forge.wynning.tech/james/carol/pulls/209) surfaced a runtime crash on the universal client: clicking certain elements threw `CSSStyleProperties doesn't have an indexed property setter for '0'` (Firefox wording; same bug on Chromium). Root cause: React DOM's `setValueForStyles` iterates the style prop with `for-in` and writes `node.style[key] = value`. When the input is a **style array** (or RNW passes through a nested array via `<Link asChild>` / similar merging), `for-in` iterates numeric indices and the DOM rejects them. PR [#232](https://forge.wynning.tech/james/carol/pulls/232) and [#211](https://forge.wynning.tech/james/carol/pulls/211) adopted the safe pattern in the nav shell, login server row, and a few other places: ```tsx // Avoid: <View style={[styles.a, cond && styles.b, { color: theme.tokens.text }]} /> // Prefer: const viewStyle = cond ? { ...styles.a, ...styles.b, color: theme.tokens.text } : { ...styles.a, color: theme.tokens.text }; <View style={viewStyle} /> ``` The bug doesn't reproduce in every `style={[...]}` site — most paths through RNW flatten correctly. But the failure modes are subtle (specific Link/Pressable merging contexts; certain prop-spread chains), so leaving array sites in place is leaving land mines. The agent for [#235](https://forge.wynning.tech/james/carol/issues/235) noted that `login.tsx`'s pre-existing form elements still use `style={[...]}` and only the new `LoginServerRow` adopted the safe shape — that's a representative snapshot of the codebase today. ## Source Follow-up flagged in [PR #238](https://forge.wynning.tech/james/carol/pulls/238) and traced back to the runtime bug fixed in [PR #232](https://forge.wynning.tech/james/carol/pulls/232). ## Scope 1. **Audit pass.** `rg 'style={\[' apps/client/` to enumerate every `style={[...]}` site. There are dozens; the migration is mechanical. 2. **Migration pass.** For each site: - If the array is `[styles.a, styles.b]` with no conditionals → `{ ...styles.a, ...styles.b }`. - If the array has `cond && { ... }` entries → pre-compute the merged object outside the JSX, or use a ternary. - Multi-line arrays with theme overrides → resolve the merged style into a named local (`const containerStyle = ...`) before handoff. - Don't touch `style={[...]}` props on **react-native-internal** non-leaf components where the array reliably flattens (e.g. `<FlatList contentContainerStyle={[...]}>`); the audit's focus is DOM-leaf primitives (View, Text, Pressable, TextInput, Image, ScrollView). 3. **Guardrail.** Add an ESLint rule (custom, or a `no-restricted-syntax`-style configuration) that flags `style={[...]}` on the DOM-leaf primitives. Allow an opt-out comment for cases where an array is the right call (e.g. animated styles via Reanimated). The rule's error message should link to this ticket so future contributors learn the pattern. The scope is limited to `apps/client/`. The deleted Next.js UI's leftover styles in `apps/api/app/themes/preferences.ts` are pure data, not affected. ## Files most likely impacted (Grep results from a recent audit — confirm during implementation.) - `apps/client/app/login.tsx` - `apps/client/app/register.tsx` - `apps/client/app/server-setup.tsx` - `apps/client/app/(app)/notes.tsx` - `apps/client/app/(app)/profile.tsx` - `apps/client/app/(app)/skills.tsx` - `apps/client/app/(app)/experience.tsx` - `apps/client/app/(app)/account.tsx` - `apps/client/lib/Placeholder.tsx` - A handful of subcomponents in `apps/client/lib/nav/` already migrated; double-check completeness. ## Acceptance criteria - [ ] `rg 'style=\{\[' apps/client/ --type tsx` returns nothing on DOM-leaf primitives, or returns only flagged exemptions (Reanimated-style array contexts). - [ ] ESLint rule rejects new `style={[...]}` on DOM-leaf primitives with a clear error message pointing here. - [ ] No visual regression on web or native — the migration is style-shape only. - [ ] `pnpm -F @carol/client test` + `export:web` stay green. - [ ] Brief note added to `CONTRIBUTING.md` "Conventions" pointing at the rule. ## Out of scope - Refactoring deeper structure (extracting subcomponents, renaming files) — this is a style-shape migration, not a redesign. - Animated styles via `react-native-reanimated`'s `useAnimatedStyle` — those legitimately return objects already; if any site uses an array of an animated style + a static style, leave it but verify it doesn't trip the bug. - The (deleted) Next.js UI — gone with #185. ## Composes with - [PR #232](https://forge.wynning.tech/james/carol/pulls/232) — original fix + the defensive pattern documentation. - [#233](https://forge.wynning.tech/james/carol/issues/233) — unit tests for the URL rewriter (the other "PR #232 didn't get test coverage" follow-up). - [#234](https://forge.wynning.tech/james/carol/issues/234) — native E2E smoke (would catch any visual regression introduced by this migration). ## Part of Standalone — no parent epic.

james referenced this issue from a commit

2026-06-23 21:00:12 +00:00

feat(client): projects screen — list, create, edit, delete (#26)

james referenced this issue

2026-06-23 21:00:40 +00:00

feat(api+client): projects feature (#26) #265

james referenced this issue from a commit

2026-06-23 21:17:28 +00:00

feat(client): page-level edit toggle on Profile

james referenced this issue

2026-06-23 21:18:02 +00:00

feat(client): page-level edit toggle on Profile (per design package) #266