Skip to content

Commit b076050

Browse files
os-zhuangclaude
andauthored
fix(console,runner): render the approvals inbox against one ticking clock, and lint both packages (#2927) (#2930)
`apps/console` and `packages/runner` had no `lint` script, so `turbo run lint` skipped them silently and their 17 ESLint errors had never been seen. Both now carry `"lint": "eslint ."`, and the `DEBT` list in check-lint-coverage.mjs is empty — 45/45 workspace packages are linted. Read one by one, the 17 were not one class of problem: 8x react-hooks/purity — real. The approvals inbox read `Date.now()` mid-render for every age tint, "5m ago" label and SLA chip. Render must be pure: the output depended on when React happened to render, so it disagreed with itself under StrictMode's double render and then froze — an inbox left open kept saying "just now" and an SLA countdown never counted down. The page now renders against a single `now` held in state and advanced once a minute (the finest granularity it displays), so render is a pure function of props+state and the figures tick. `sla_due_at` also goes through a parse guard now: a due date the backend sends in a shape `Date.parse` can't read rendered as "SLA NaNh left"; it renders nothing. 1x react-hooks/static-components — real. `StatusBadge` was declared inside `ApprovalsInboxPage`, so it was a brand-new component type every render and React remounted every status chip in the table on each re-render. Hoisted to module scope with the translated label passed as a prop. 6x react-hooks/static-components — false positives (3 settings pages, 3 in the runner's LayoutRenderer). All six render the result of `getIcon`/`getLazyIcon`, which memoises per name in a module-level cache: the reference is stable and nothing is created during render. They carry the same targeted disable + justification the repo already uses at a dozen icon-registry sites, and both resolvers now document the cache. Verified rather than assumed — typing into a settings field keeps focus and every character, so nothing was being reset. 2 minor — a dead `token` initializer on the auth-preflight path (read, not blind -deleted: no intended write was missing) and a `prefer-const`. Closes #2927. Refs #2911, #2923. Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent cf20681 commit b076050

12 files changed

Lines changed: 185 additions & 59 deletions

File tree

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
---
2+
"@object-ui/console": patch
3+
"@object-ui/runner": patch
4+
---
5+
6+
fix(console,runner): the approvals inbox renders against one ticking clock, and both packages now run ESLint
7+
8+
`apps/console` and `packages/runner` had no `lint` script, so `turbo run lint`
9+
skipped them silently and their 17 ESLint **errors** had never been seen
10+
(#2923 declared them as DEBT; this closes the gap). Both now carry
11+
`"lint": "eslint ."` and the `DEBT` list in `scripts/check-lint-coverage.mjs`
12+
is empty — every workspace package is linted.
13+
14+
What the errors actually were, once read one by one:
15+
16+
- **8x `react-hooks/purity` — real, and user-visible.** The approvals inbox
17+
read `Date.now()` mid-render for every age tint, "5m ago" label and SLA chip.
18+
Render must be pure: the output depended on when React happened to render, so
19+
it disagreed with itself under StrictMode's double render and then **froze**
20+
an inbox left open kept saying "just now" and an SLA countdown never counted
21+
down. The page now renders against a single `now` held in state and advanced
22+
once a minute (the finest granularity anything here displays), so render is a
23+
pure function of props+state *and* the figures actually tick.
24+
- Alongside that, `sla_due_at` is now parsed through a guard. A due date the
25+
backend sends in a shape `Date.parse` can't read used to render as
26+
"SLA NaNh left"; it now renders nothing.
27+
- **1x `react-hooks/static-components` — real.** `StatusBadge` was declared
28+
inside `ApprovalsInboxPage`, making it a brand-new component type on every
29+
render, so React unmounted and remounted every status chip in the table each
30+
time the page re-rendered. Hoisted to module scope, with the translated label
31+
passed as a prop.
32+
- **6x `react-hooks/static-components` — false positives** (3 in the console's
33+
settings pages, 3 in the runner's `LayoutRenderer`). All six render the result
34+
of `getIcon`/`getLazyIcon`, which memoises per name in a module-level cache —
35+
the component reference is stable across renders and nothing is created during
36+
render. The rule cannot see through the call, so these carry the same targeted
37+
`eslint-disable-next-line` + justification the repo already uses at a dozen
38+
icon-registry sites, and the resolvers themselves now say so in a comment.
39+
(Verified rather than assumed: typing into a settings field keeps focus and
40+
every character, so no state was ever being reset there.)
41+
- **2 minor.** A dead `token` initializer on the console's auth preflight path
42+
(`no-useless-assignment` — read, not blind-deleted: no intended write was
43+
missing, every path out of the try/catch either assigns or returns), and a
44+
`prefer-const` in the SDUI workbench preview.

apps/console/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
"build:plugin": "tsc -p tsconfig.plugin.json",
4747
"build": "tsc && vite build && pnpm build:plugin",
4848
"type-check": "tsc --noEmit",
49+
"lint": "eslint .",
4950
"build:analyze": "pnpm build && echo 'Bundle analysis available at dist/stats.html'",
5051
"preview": "vite preview",
5152
"test": "vitest run",

apps/console/src/lib/auth-preflight.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,14 @@ const ACTIVE_ORG_KEY = 'auth-active-organization-id';
3636

3737
export async function preflightAuth(authBaseUrl: string): Promise<void> {
3838
if (typeof window === 'undefined') return;
39-
let token: string | null = null;
39+
// No initializer: every path out of the try/catch below either assigns
40+
// `token` or returns, so a `= null` seed would be dead (no-useless-assignment).
41+
let token: string | null;
4042
try {
4143
token = localStorage.getItem(TOKEN_KEY);
4244
} catch {
45+
// Storage blocked (Safari private mode / partitioned iframe) — nothing to
46+
// validate, and nothing we could purge either.
4347
return;
4448
}
4549
if (!token) return;

apps/console/src/pages/settings/SettingsField.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,10 @@ export function SettingsField(props: SettingsFieldProps) {
219219
) : null}
220220
</div>
221221
<Button size="sm" variant="secondary" onClick={onAction} disabled={saving}>
222-
{Icon ? <Icon className="h-4 w-4 mr-1.5" /> : null}
222+
{Icon ? (
223+
// eslint-disable-next-line react-hooks/static-components -- getIcon returns a module-cached stable component per name, not one created during render
224+
<Icon className="h-4 w-4 mr-1.5" />
225+
) : null}
223226
{actionLabel}
224227
</Button>
225228
</div>

apps/console/src/pages/settings/SettingsHub.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ function SettingCard({ m, onOpen }: { m: SettingsManifest; onOpen: () => void })
4343
>
4444
<CardHeader className="pb-3">
4545
<div className="flex items-start justify-between">
46+
{/* eslint-disable-next-line react-hooks/static-components -- getIcon returns a module-cached stable component per name, not one created during render */}
4647
<Icon className="h-6 w-6 text-muted-foreground" />
4748
{m.beta ? (
4849
<Badge variant="secondary" className="text-[10px]">

apps/console/src/pages/settings/SettingsView.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,10 @@ export function SettingsView() {
158158
</Button>
159159

160160
<div className="mt-3 flex items-start gap-3">
161-
{Icon ? <Icon className="h-7 w-7 mt-0.5 text-muted-foreground" /> : null}
161+
{Icon ? (
162+
// eslint-disable-next-line react-hooks/static-components -- getIcon returns a module-cached stable component per name, not one created during render
163+
<Icon className="h-7 w-7 mt-0.5 text-muted-foreground" />
164+
) : null}
162165
<div className="flex-1">
163166
<div className="flex items-center gap-2">
164167
<h1 className="text-2xl font-semibold tracking-tight">{title}</h1>

apps/console/src/pages/system/ApprovalsInboxPage.tsx

Lines changed: 96 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,21 @@ const STATUS_CLASSES: Record<string, string> = {
121121
returned: 'border-violet-200 bg-violet-50 text-violet-700 dark:border-violet-500/30 dark:bg-violet-500/10 dark:text-violet-400',
122122
};
123123

124+
/**
125+
* Status chip. Module scope on purpose: declared inside `ApprovalsInboxPage`
126+
* it was a brand-new component type on every render, so React unmounted and
127+
* remounted every badge in the table each time the page re-rendered
128+
* (`react-hooks/static-components`). The translated label is passed in
129+
* because `statusLabel` closes over the page's `t`.
130+
*/
131+
function StatusBadge({ status, label }: { status: string; label: string }) {
132+
return (
133+
<Badge variant="outline" className={cn('font-medium', STATUS_CLASSES[status] ?? '')}>
134+
{label}
135+
</Badge>
136+
);
137+
}
138+
124139
function formatDate(s: string | null | undefined): string {
125140
if (!s) return '—';
126141
try { return new Date(s).toLocaleString(); } catch { return s; }
@@ -214,19 +229,24 @@ function submittedAt(r: ApprovalRequestRow): string | undefined {
214229
return r.submitted_at || r.created_at || undefined;
215230
}
216231

217-
/** Hours a pending request has been waiting; null when no timestamp. */
218-
function waitingHours(r: ApprovalRequestRow): number | null {
232+
/**
233+
* Hours a pending request has been waiting at instant `now`; null when no
234+
* timestamp. `now` is a parameter rather than a `Date.now()` read so callers
235+
* on the render path stay pure (`react-hooks/purity`) — see the `now` state in
236+
* {@link ApprovalsInboxPage}.
237+
*/
238+
function waitingHours(r: ApprovalRequestRow, now: number): number | null {
219239
const s = submittedAt(r);
220240
if (!s) return null;
221241
const t = Date.parse(s);
222242
if (Number.isNaN(t)) return null;
223-
return (Date.now() - t) / 36e5;
243+
return (now - t) / 36e5;
224244
}
225245

226246
/** Aging tint: quiet under a day, amber 1–3 days, red beyond 3 days. */
227-
function agingClass(r: ApprovalRequestRow): string {
247+
function agingClass(r: ApprovalRequestRow, now: number): string {
228248
if (r.status !== 'pending') return 'text-muted-foreground';
229-
const h = waitingHours(r);
249+
const h = waitingHours(r, now);
230250
if (h == null) return 'text-muted-foreground';
231251
if (h > 72) return 'text-red-600 dark:text-red-400 font-medium';
232252
if (h > 24) return 'text-amber-600 dark:text-amber-400';
@@ -239,6 +259,19 @@ function compactDuration(ms: number): string {
239259
return h < 48 ? `${h}h` : `${Math.round(h / 24)}d`;
240260
}
241261

262+
/**
263+
* SLA position at instant `now`, or null when there is nothing to show — no
264+
* due date, or one the backend sent in a shape `Date.parse` can't read (which
265+
* previously rendered as "SLA NaNh left"). Takes `now` for the same purity
266+
* reason as {@link waitingHours}.
267+
*/
268+
function slaState(dueAt: string | null | undefined, now: number): { overdue: boolean; ms: number } | null {
269+
if (!dueAt) return null;
270+
const due = Date.parse(dueAt);
271+
if (Number.isNaN(due)) return null;
272+
return { overdue: due < now, ms: Math.abs(now - due) };
273+
}
274+
242275
const PAYLOAD_SYSTEM_KEYS = new Set([
243276
'id', 'created_at', 'updated_at', 'created_by', 'updated_by', 'organization_id',
244277
]);
@@ -354,12 +387,31 @@ export function ApprovalsInboxPage() {
354387
[t],
355388
);
356389

390+
/**
391+
* The one clock this page renders against.
392+
*
393+
* Every age/SLA figure below used to call `Date.now()` mid-render, which is
394+
* impure (`react-hooks/purity`): the output depended on when React happened
395+
* to render, so it disagreed with itself under StrictMode's double render and
396+
* froze between renders — an inbox left open showed "just now" indefinitely.
397+
* Holding the instant in state and advancing it on a timer makes render a
398+
* pure function of props+state *and* makes the countdowns actually tick.
399+
*
400+
* A minute is the finest granularity anything here displays ("5m ago",
401+
* "36h", "3d"), so that is the tick rate.
402+
*/
403+
const [now, setNow] = useState(() => Date.now());
404+
useEffect(() => {
405+
const id = setInterval(() => setNow(Date.now()), 60_000);
406+
return () => clearInterval(id);
407+
}, []);
408+
357409
/** Localized relative time, e.g. "5m ago" / "5 分钟前". */
358410
const formatRelative = useCallback((s: string | null | undefined): string => {
359411
if (!s) return '—';
360412
const ts = Date.parse(s);
361413
if (Number.isNaN(ts)) return s;
362-
const sec = Math.round((Date.now() - ts) / 1000);
414+
const sec = Math.round((now - ts) / 1000);
363415
if (sec < 45) return tr('justNow', 'just now');
364416
const min = Math.round(sec / 60);
365417
if (min < 60) return tr('minutesAgo', '{{count}}m ago', { count: min });
@@ -368,7 +420,7 @@ export function ApprovalsInboxPage() {
368420
const day = Math.round(hr / 24);
369421
if (day < 30) return tr('daysAgo', '{{count}}d ago', { count: day });
370422
try { return new Date(s).toLocaleDateString(language); } catch { return s; }
371-
}, [tr, language]);
423+
}, [tr, language, now]);
372424

373425
const statusLabel = useCallback((status: string): string => {
374426
switch (status) {
@@ -397,13 +449,14 @@ export function ApprovalsInboxPage() {
397449
return err?.message || fallback;
398450
}, [tr]);
399451

400-
function StatusBadge({ status }: { status: string }) {
401-
return (
402-
<Badge variant="outline" className={cn('font-medium', STATUS_CLASSES[status] ?? '')}>
403-
{statusLabel(status)}
404-
</Badge>
405-
);
406-
}
452+
/** SLA chip copy for a {@link slaState} result. */
453+
const slaLabel = useCallback(
454+
(sla: { overdue: boolean; ms: number }): string =>
455+
sla.overdue
456+
? tr('slaOverdue', 'SLA overdue {{dur}}', { dur: compactDuration(sla.ms) })
457+
: tr('slaRemaining', 'SLA {{dur}} left', { dur: compactDuration(sla.ms) }),
458+
[tr],
459+
);
407460

408461
const recordHref = useCallback((r: ApprovalRequestRow): string => {
409462
const app = appName || 'setup';
@@ -1387,23 +1440,24 @@ export function ApprovalsInboxPage() {
13871440
</div>
13881441
)}
13891442
</TableCell>
1390-
<TableCell><StatusBadge status={r.status} /></TableCell>
1443+
<TableCell><StatusBadge status={r.status} label={statusLabel(r.status)} /></TableCell>
13911444
<TableCell
1392-
className={cn('text-xs whitespace-nowrap', agingClass(r))}
1445+
className={cn('text-xs whitespace-nowrap', agingClass(r, now))}
13931446
title={formatDate(submittedAt(r))}
13941447
>
13951448
<Clock className="h-3 w-3 inline mr-1" />
13961449
{formatRelative(submittedAt(r))}
1397-
{r.status === 'pending' && r.sla_due_at && (
1398-
<div className={cn(
1399-
'mt-0.5 text-[10px] font-medium',
1400-
Date.parse(r.sla_due_at) < Date.now() ? 'text-red-600 dark:text-red-400' : 'text-muted-foreground',
1401-
)}>
1402-
{Date.parse(r.sla_due_at) < Date.now()
1403-
? tr('slaOverdue', 'SLA overdue {{dur}}', { dur: compactDuration(Date.now() - Date.parse(r.sla_due_at)) })
1404-
: tr('slaRemaining', 'SLA {{dur}} left', { dur: compactDuration(Date.parse(r.sla_due_at) - Date.now()) })}
1405-
</div>
1406-
)}
1450+
{r.status === 'pending' && (() => {
1451+
const sla = slaState(r.sla_due_at, now);
1452+
return sla ? (
1453+
<div className={cn(
1454+
'mt-0.5 text-[10px] font-medium',
1455+
sla.overdue ? 'text-red-600 dark:text-red-400' : 'text-muted-foreground',
1456+
)}>
1457+
{slaLabel(sla)}
1458+
</div>
1459+
) : null;
1460+
})()}
14071461
</TableCell>
14081462
<TableCell className="w-20"><InlineActions r={r} /></TableCell>
14091463
</TableRow>
@@ -1420,7 +1474,7 @@ export function ApprovalsInboxPage() {
14201474
<CardContent className="p-3 space-y-1.5">
14211475
<div className="flex items-start justify-between gap-2">
14221476
<div className="font-medium text-sm truncate">{processLabel(r)}</div>
1423-
<StatusBadge status={r.status} />
1477+
<StatusBadge status={r.status} label={statusLabel(r.status)} />
14241478
</div>
14251479
<div className="text-sm truncate">
14261480
{r.record_title || formatIdentity(r.record_id)}
@@ -1442,7 +1496,7 @@ export function ApprovalsInboxPage() {
14421496
<UserIcon className="h-3 w-3" />{submitterDisplay(r)}
14431497
</span>
14441498
)}
1445-
<span className={cn('inline-flex items-center gap-1 whitespace-nowrap', agingClass(r))}>
1499+
<span className={cn('inline-flex items-center gap-1 whitespace-nowrap', agingClass(r, now))}>
14461500
<Clock className="h-3 w-3" />{formatRelative(submittedAt(r))}
14471501
</span>
14481502
</div>
@@ -1585,7 +1639,7 @@ export function ApprovalsInboxPage() {
15851639
<div className="space-y-4 mt-6">
15861640
{/* Status strip */}
15871641
<div className="flex flex-wrap items-center gap-2 text-xs text-muted-foreground">
1588-
<StatusBadge status={selected.status} />
1642+
<StatusBadge status={selected.status} label={statusLabel(selected.status)} />
15891643
{(selected.round ?? 1) > 1 && (
15901644
<Badge variant="outline" className="text-[10px] border-violet-200 text-violet-700 dark:border-violet-500/30 dark:text-violet-400">
15911645
{tr('roundChip', 'Round {{n}}', { n: selected.round })}
@@ -1598,18 +1652,19 @@ export function ApprovalsInboxPage() {
15981652
{selected.completed_at && (
15991653
<span>· {tr('completedAt', 'Completed {{when}}', { when: formatRelative(selected.completed_at) })}</span>
16001654
)}
1601-
{selected.status === 'pending' && selected.sla_due_at && (
1602-
<Badge variant="outline" className={cn(
1603-
'text-[10px]',
1604-
Date.parse(selected.sla_due_at) < Date.now()
1605-
? 'border-red-200 bg-red-50 text-red-700 dark:border-red-500/30 dark:bg-red-500/10 dark:text-red-400'
1606-
: 'border-border text-muted-foreground',
1607-
)}>
1608-
{Date.parse(selected.sla_due_at) < Date.now()
1609-
? tr('slaOverdue', 'SLA overdue {{dur}}', { dur: compactDuration(Date.now() - Date.parse(selected.sla_due_at)) })
1610-
: tr('slaRemaining', 'SLA {{dur}} left', { dur: compactDuration(Date.parse(selected.sla_due_at) - Date.now()) })}
1611-
</Badge>
1612-
)}
1655+
{selected.status === 'pending' && (() => {
1656+
const sla = slaState(selected.sla_due_at, now);
1657+
return sla ? (
1658+
<Badge variant="outline" className={cn(
1659+
'text-[10px]',
1660+
sla.overdue
1661+
? 'border-red-200 bg-red-50 text-red-700 dark:border-red-500/30 dark:bg-red-500/10 dark:text-red-400'
1662+
: 'border-border text-muted-foreground',
1663+
)}>
1664+
{slaLabel(sla)}
1665+
</Badge>
1666+
) : null;
1667+
})()}
16131668
</div>
16141669

16151670
{/* Business summary card */}

apps/console/src/sdui-workbench-preview.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ if (new URLSearchParams(location.search).has('disable')) {
2222
}
2323

2424
// ---- in-memory adapter (just enough for ListView + ObjectForm) ----
25-
let store: any[] = [
25+
const store: any[] = [
2626
{ id: '1', name: 'Apollo Migration', status: 'active', health: 'green', budget: 120000, owner: 'Dana Lee' },
2727
{ id: '2', name: 'Billing Revamp', status: 'planned', health: 'yellow', budget: 80000, owner: 'Sam Ortiz' },
2828
{ id: '3', name: 'Mobile App v2', status: 'active', health: 'red', budget: 210000, owner: 'Priya N.' },

apps/console/src/utils/getIcon.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,14 @@ function toKebab(name: string): string {
2020

2121
const cache = new Map<string, React.ElementType>();
2222

23+
/**
24+
* Resolve a Lucide icon component by name.
25+
*
26+
* The result is memoised per name in the module-level `cache`, so call sites
27+
* get a *stable* component reference across renders — nothing is created during
28+
* render. `react-hooks/static-components` cannot see through the call, so the
29+
* JSX sites that render the result carry a targeted disable pointing back here.
30+
*/
2331
export function getIcon(name?: string): React.ElementType {
2432
if (!name) return Database;
2533
const cached = cache.get(name);

packages/runner/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"dev": "vite",
1818
"build": "vite build",
1919
"type-check": "tsc --noEmit",
20+
"lint": "eslint .",
2021
"preview": "vite preview",
2122
"test": "vitest run"
2223
},

0 commit comments

Comments
 (0)