Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .changeset/lint-console-runner-react-correctness.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
---
"@object-ui/console": patch
"@object-ui/runner": patch
---

fix(console,runner): the approvals inbox renders against one ticking clock, and both packages now run ESLint

`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
(#2923 declared them as DEBT; this closes the gap). Both now carry
`"lint": "eslint ."` and the `DEBT` list in `scripts/check-lint-coverage.mjs`
is empty — every workspace package is linted.

What the errors actually were, once read one by one:

- **8x `react-hooks/purity` — real, and user-visible.** 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 anything here displays), so render is a
pure function of props+state *and* the figures actually tick.
- Alongside that, `sla_due_at` is now parsed through a guard. A due date the
backend sends in a shape `Date.parse` can't read used to render as
"SLA NaNh left"; it now renders nothing.
- **1x `react-hooks/static-components` — real.** `StatusBadge` was declared
inside `ApprovalsInboxPage`, making it a brand-new component type on every
render, so React unmounted and remounted every status chip in the table each
time the page re-rendered. Hoisted to module scope, with the translated label
passed as a prop.
- **6x `react-hooks/static-components` — false positives** (3 in the console's
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 component reference is stable across renders and nothing is created during
render. The rule cannot see through the call, so these carry the same targeted
`eslint-disable-next-line` + justification the repo already uses at a dozen
icon-registry sites, and the resolvers themselves now say so in a comment.
(Verified rather than assumed: typing into a settings field keeps focus and
every character, so no state was ever being reset there.)
- **2 minor.** A dead `token` initializer on the console's auth preflight path
(`no-useless-assignment` — read, not blind-deleted: no intended write was
missing, every path out of the try/catch either assigns or returns), and a
`prefer-const` in the SDUI workbench preview.
1 change: 1 addition & 0 deletions apps/console/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
"build:plugin": "tsc -p tsconfig.plugin.json",
"build": "tsc && vite build && pnpm build:plugin",
"type-check": "tsc --noEmit",
"lint": "eslint .",
"build:analyze": "pnpm build && echo 'Bundle analysis available at dist/stats.html'",
"preview": "vite preview",
"test": "vitest run",
Expand Down
6 changes: 5 additions & 1 deletion apps/console/src/lib/auth-preflight.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,14 @@ const ACTIVE_ORG_KEY = 'auth-active-organization-id';

export async function preflightAuth(authBaseUrl: string): Promise<void> {
if (typeof window === 'undefined') return;
let token: string | null = null;
// No initializer: every path out of the try/catch below either assigns
// `token` or returns, so a `= null` seed would be dead (no-useless-assignment).
let token: string | null;
try {
token = localStorage.getItem(TOKEN_KEY);
} catch {
// Storage blocked (Safari private mode / partitioned iframe) — nothing to
// validate, and nothing we could purge either.
return;
}
if (!token) return;
Expand Down
5 changes: 4 additions & 1 deletion apps/console/src/pages/settings/SettingsField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,10 @@ export function SettingsField(props: SettingsFieldProps) {
) : null}
</div>
<Button size="sm" variant="secondary" onClick={onAction} disabled={saving}>
{Icon ? <Icon className="h-4 w-4 mr-1.5" /> : null}
{Icon ? (
// eslint-disable-next-line react-hooks/static-components -- getIcon returns a module-cached stable component per name, not one created during render
<Icon className="h-4 w-4 mr-1.5" />
) : null}
{actionLabel}
</Button>
</div>
Expand Down
1 change: 1 addition & 0 deletions apps/console/src/pages/settings/SettingsHub.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ function SettingCard({ m, onOpen }: { m: SettingsManifest; onOpen: () => void })
>
<CardHeader className="pb-3">
<div className="flex items-start justify-between">
{/* eslint-disable-next-line react-hooks/static-components -- getIcon returns a module-cached stable component per name, not one created during render */}
<Icon className="h-6 w-6 text-muted-foreground" />
{m.beta ? (
<Badge variant="secondary" className="text-[10px]">
Expand Down
5 changes: 4 additions & 1 deletion apps/console/src/pages/settings/SettingsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,10 @@ export function SettingsView() {
</Button>

<div className="mt-3 flex items-start gap-3">
{Icon ? <Icon className="h-7 w-7 mt-0.5 text-muted-foreground" /> : null}
{Icon ? (
// eslint-disable-next-line react-hooks/static-components -- getIcon returns a module-cached stable component per name, not one created during render
<Icon className="h-7 w-7 mt-0.5 text-muted-foreground" />
) : null}
<div className="flex-1">
<div className="flex items-center gap-2">
<h1 className="text-2xl font-semibold tracking-tight">{title}</h1>
Expand Down
137 changes: 96 additions & 41 deletions apps/console/src/pages/system/ApprovalsInboxPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,21 @@ const STATUS_CLASSES: Record<string, string> = {
returned: 'border-violet-200 bg-violet-50 text-violet-700 dark:border-violet-500/30 dark:bg-violet-500/10 dark:text-violet-400',
};

/**
* Status chip. Module scope on purpose: declared inside `ApprovalsInboxPage`
* it was a brand-new component type on every render, so React unmounted and
* remounted every badge in the table each time the page re-rendered
* (`react-hooks/static-components`). The translated label is passed in
* because `statusLabel` closes over the page's `t`.
*/
function StatusBadge({ status, label }: { status: string; label: string }) {
return (
<Badge variant="outline" className={cn('font-medium', STATUS_CLASSES[status] ?? '')}>
{label}
</Badge>
);
}

function formatDate(s: string | null | undefined): string {
if (!s) return '—';
try { return new Date(s).toLocaleString(); } catch { return s; }
Expand Down Expand Up @@ -214,19 +229,24 @@ function submittedAt(r: ApprovalRequestRow): string | undefined {
return r.submitted_at || r.created_at || undefined;
}

/** Hours a pending request has been waiting; null when no timestamp. */
function waitingHours(r: ApprovalRequestRow): number | null {
/**
* Hours a pending request has been waiting at instant `now`; null when no
* timestamp. `now` is a parameter rather than a `Date.now()` read so callers
* on the render path stay pure (`react-hooks/purity`) — see the `now` state in
* {@link ApprovalsInboxPage}.
*/
function waitingHours(r: ApprovalRequestRow, now: number): number | null {
const s = submittedAt(r);
if (!s) return null;
const t = Date.parse(s);
if (Number.isNaN(t)) return null;
return (Date.now() - t) / 36e5;
return (now - t) / 36e5;
}

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

/**
* SLA position at instant `now`, or null when there is nothing to show — no
* due date, or one the backend sent in a shape `Date.parse` can't read (which
* previously rendered as "SLA NaNh left"). Takes `now` for the same purity
* reason as {@link waitingHours}.
*/
function slaState(dueAt: string | null | undefined, now: number): { overdue: boolean; ms: number } | null {
if (!dueAt) return null;
const due = Date.parse(dueAt);
if (Number.isNaN(due)) return null;
return { overdue: due < now, ms: Math.abs(now - due) };
}

const PAYLOAD_SYSTEM_KEYS = new Set([
'id', 'created_at', 'updated_at', 'created_by', 'updated_by', 'organization_id',
]);
Expand Down Expand Up @@ -354,12 +387,31 @@ export function ApprovalsInboxPage() {
[t],
);

/**
* The one clock this page renders against.
*
* Every age/SLA figure below used to call `Date.now()` mid-render, which is
* impure (`react-hooks/purity`): the output depended on when React happened
* to render, so it disagreed with itself under StrictMode's double render and
* froze between renders — an inbox left open showed "just now" indefinitely.
* Holding the instant in state and advancing it on a timer makes render a
* pure function of props+state *and* makes the countdowns actually tick.
*
* A minute is the finest granularity anything here displays ("5m ago",
* "36h", "3d"), so that is the tick rate.
*/
const [now, setNow] = useState(() => Date.now());
useEffect(() => {
const id = setInterval(() => setNow(Date.now()), 60_000);
return () => clearInterval(id);
}, []);

/** Localized relative time, e.g. "5m ago" / "5 分钟前". */
const formatRelative = useCallback((s: string | null | undefined): string => {
if (!s) return '—';
const ts = Date.parse(s);
if (Number.isNaN(ts)) return s;
const sec = Math.round((Date.now() - ts) / 1000);
const sec = Math.round((now - ts) / 1000);
if (sec < 45) return tr('justNow', 'just now');
const min = Math.round(sec / 60);
if (min < 60) return tr('minutesAgo', '{{count}}m ago', { count: min });
Expand All @@ -368,7 +420,7 @@ export function ApprovalsInboxPage() {
const day = Math.round(hr / 24);
if (day < 30) return tr('daysAgo', '{{count}}d ago', { count: day });
try { return new Date(s).toLocaleDateString(language); } catch { return s; }
}, [tr, language]);
}, [tr, language, now]);

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

function StatusBadge({ status }: { status: string }) {
return (
<Badge variant="outline" className={cn('font-medium', STATUS_CLASSES[status] ?? '')}>
{statusLabel(status)}
</Badge>
);
}
/** SLA chip copy for a {@link slaState} result. */
const slaLabel = useCallback(
(sla: { overdue: boolean; ms: number }): string =>
sla.overdue
? tr('slaOverdue', 'SLA overdue {{dur}}', { dur: compactDuration(sla.ms) })
: tr('slaRemaining', 'SLA {{dur}} left', { dur: compactDuration(sla.ms) }),
[tr],
);

const recordHref = useCallback((r: ApprovalRequestRow): string => {
const app = appName || 'setup';
Expand Down Expand Up @@ -1387,23 +1440,24 @@ export function ApprovalsInboxPage() {
</div>
)}
</TableCell>
<TableCell><StatusBadge status={r.status} /></TableCell>
<TableCell><StatusBadge status={r.status} label={statusLabel(r.status)} /></TableCell>
<TableCell
className={cn('text-xs whitespace-nowrap', agingClass(r))}
className={cn('text-xs whitespace-nowrap', agingClass(r, now))}
title={formatDate(submittedAt(r))}
>
<Clock className="h-3 w-3 inline mr-1" />
{formatRelative(submittedAt(r))}
{r.status === 'pending' && r.sla_due_at && (
<div className={cn(
'mt-0.5 text-[10px] font-medium',
Date.parse(r.sla_due_at) < Date.now() ? 'text-red-600 dark:text-red-400' : 'text-muted-foreground',
)}>
{Date.parse(r.sla_due_at) < Date.now()
? tr('slaOverdue', 'SLA overdue {{dur}}', { dur: compactDuration(Date.now() - Date.parse(r.sla_due_at)) })
: tr('slaRemaining', 'SLA {{dur}} left', { dur: compactDuration(Date.parse(r.sla_due_at) - Date.now()) })}
</div>
)}
{r.status === 'pending' && (() => {
const sla = slaState(r.sla_due_at, now);
return sla ? (
<div className={cn(
'mt-0.5 text-[10px] font-medium',
sla.overdue ? 'text-red-600 dark:text-red-400' : 'text-muted-foreground',
)}>
{slaLabel(sla)}
</div>
) : null;
})()}
</TableCell>
<TableCell className="w-20"><InlineActions r={r} /></TableCell>
</TableRow>
Expand All @@ -1420,7 +1474,7 @@ export function ApprovalsInboxPage() {
<CardContent className="p-3 space-y-1.5">
<div className="flex items-start justify-between gap-2">
<div className="font-medium text-sm truncate">{processLabel(r)}</div>
<StatusBadge status={r.status} />
<StatusBadge status={r.status} label={statusLabel(r.status)} />
</div>
<div className="text-sm truncate">
{r.record_title || formatIdentity(r.record_id)}
Expand All @@ -1442,7 +1496,7 @@ export function ApprovalsInboxPage() {
<UserIcon className="h-3 w-3" />{submitterDisplay(r)}
</span>
)}
<span className={cn('inline-flex items-center gap-1 whitespace-nowrap', agingClass(r))}>
<span className={cn('inline-flex items-center gap-1 whitespace-nowrap', agingClass(r, now))}>
<Clock className="h-3 w-3" />{formatRelative(submittedAt(r))}
</span>
</div>
Expand Down Expand Up @@ -1585,7 +1639,7 @@ export function ApprovalsInboxPage() {
<div className="space-y-4 mt-6">
{/* Status strip */}
<div className="flex flex-wrap items-center gap-2 text-xs text-muted-foreground">
<StatusBadge status={selected.status} />
<StatusBadge status={selected.status} label={statusLabel(selected.status)} />
{(selected.round ?? 1) > 1 && (
<Badge variant="outline" className="text-[10px] border-violet-200 text-violet-700 dark:border-violet-500/30 dark:text-violet-400">
{tr('roundChip', 'Round {{n}}', { n: selected.round })}
Expand All @@ -1598,18 +1652,19 @@ export function ApprovalsInboxPage() {
{selected.completed_at && (
<span>· {tr('completedAt', 'Completed {{when}}', { when: formatRelative(selected.completed_at) })}</span>
)}
{selected.status === 'pending' && selected.sla_due_at && (
<Badge variant="outline" className={cn(
'text-[10px]',
Date.parse(selected.sla_due_at) < Date.now()
? 'border-red-200 bg-red-50 text-red-700 dark:border-red-500/30 dark:bg-red-500/10 dark:text-red-400'
: 'border-border text-muted-foreground',
)}>
{Date.parse(selected.sla_due_at) < Date.now()
? tr('slaOverdue', 'SLA overdue {{dur}}', { dur: compactDuration(Date.now() - Date.parse(selected.sla_due_at)) })
: tr('slaRemaining', 'SLA {{dur}} left', { dur: compactDuration(Date.parse(selected.sla_due_at) - Date.now()) })}
</Badge>
)}
{selected.status === 'pending' && (() => {
const sla = slaState(selected.sla_due_at, now);
return sla ? (
<Badge variant="outline" className={cn(
'text-[10px]',
sla.overdue
? 'border-red-200 bg-red-50 text-red-700 dark:border-red-500/30 dark:bg-red-500/10 dark:text-red-400'
: 'border-border text-muted-foreground',
)}>
{slaLabel(sla)}
</Badge>
) : null;
})()}
</div>

{/* Business summary card */}
Expand Down
2 changes: 1 addition & 1 deletion apps/console/src/sdui-workbench-preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ if (new URLSearchParams(location.search).has('disable')) {
}

// ---- in-memory adapter (just enough for ListView + ObjectForm) ----
let store: any[] = [
const store: any[] = [
{ id: '1', name: 'Apollo Migration', status: 'active', health: 'green', budget: 120000, owner: 'Dana Lee' },
{ id: '2', name: 'Billing Revamp', status: 'planned', health: 'yellow', budget: 80000, owner: 'Sam Ortiz' },
{ id: '3', name: 'Mobile App v2', status: 'active', health: 'red', budget: 210000, owner: 'Priya N.' },
Expand Down
8 changes: 8 additions & 0 deletions apps/console/src/utils/getIcon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,14 @@ function toKebab(name: string): string {

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

/**
* Resolve a Lucide icon component by name.
*
* The result is memoised per name in the module-level `cache`, so call sites
* get a *stable* component reference across renders — nothing is created during
* render. `react-hooks/static-components` cannot see through the call, so the
* JSX sites that render the result carry a targeted disable pointing back here.
*/
export function getIcon(name?: string): React.ElementType {
if (!name) return Database;
const cached = cache.get(name);
Expand Down
1 change: 1 addition & 0 deletions packages/runner/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"dev": "vite",
"build": "vite build",
"type-check": "tsc --noEmit",
"lint": "eslint .",
"preview": "vite preview",
"test": "vitest run"
},
Expand Down
Loading
Loading