From 36e4759b055f0639f85b0971600e8e2cfd79d26e Mon Sep 17 00:00:00 2001 From: Joe Elstner Date: Sat, 19 Sep 2026 22:12:53 -0500 Subject: [PATCH] fix(concierge): prove the array indexing the types only asserted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gridiron is the first consumer whose tsconfig sets noUncheckedIndexedAccess, and it typechecks this package's source because we ship raw src/. Eight errors surfaced in concierge.tsx that no other site could see — none of them new, all of them the same shape: an index read whose presence was asserted by a return type rather than proven. - readCookie destructured match[1] behind `if (!match)`, which narrows the match but not the group. The catch branch then reached for match[1] again, where `value` was already the same string. - parseRgba declared a 4-tuple return and built it from m[1..4] unchecked. Now the groups are destructured and checked, and a null match falls through the same branch — identical behaviour, provable arity. - luminance destructured a .map() result as a 3-tuple. A mapped array is number[], so each channel was number | undefined. Mapping each channel through one srgb() keeps the arity in the types. Behaviour is unchanged, and "unchanged" is tested rather than asserted: auto-theme now has a test that a dark page and a light page resolve to DIFFERENT bar backgrounds. Forcing parseRgba to null fails it; forcing luminance to 1 fails it. Without that pair the test would pass vacuously on a shared fallback. (jsdom implements no hit-testing, so elementFromPoint is stubbed.) Scope is concierge.tsx only — the entry gridiron imports. article-toc.tsx and bento-grid.tsx carry the same class of error and are left for whoever first consumes them strictly. Suite 42/42 · typecheck clean · concierge.tsx clean under --strict --noUncheckedIndexedAccess. Co-Authored-By: Claude Opus 5 (1M context) --- package.json | 2 +- src/concierge.test.tsx | 37 +++++++++++++++++++++++++++++++++++++ src/concierge.tsx | 22 ++++++++++++++-------- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index ef15ff5..0f2e2cf 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@isimplifyme/ui", - "version": "1.4.0", + "version": "1.4.1", "description": "React/Next.js UI library for iSimplifyMe properties \u2014 design system, article layouts, SEO helpers, bot middleware, and the iSM Concierge widget.", "homepage": "https://isimplifyme.com", "type": "module", diff --git a/src/concierge.test.tsx b/src/concierge.test.tsx index bf70594..bd7cf0c 100644 --- a/src/concierge.test.tsx +++ b/src/concierge.test.tsx @@ -114,3 +114,40 @@ describe('with a leading node', () => { expect(slot.style.pointerEvents).toBe('none'); }); }); + +describe('auto theme — the colour parsing these refactors touched', () => { + // parseRgba and luminance were rewritten to satisfy + // noUncheckedIndexedAccess (gridiron is the first consumer strict enough + // to typecheck this package's source). Neither is exported, so the honest + // test is the observable one: does auto-theme still TELL THE TWO APART? + function barBackgroundOn(bodyColor: string) { + // jsdom implements no hit-testing; the auto path samples whatever sits + // under the bar, which on a real page is the section behind it. + (document as unknown as { elementFromPoint: () => Element }) + .elementFromPoint = () => document.body; + document.body.style.backgroundColor = bodyColor; + const { bar } = render({ theme: 'auto' }); + return bar.style.background; + } + + it('resolves a dark page differently from a light one', () => { + const onDark = barBackgroundOn('rgb(14, 15, 19)'); + act(() => root.unmount()); + container.remove(); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + const onLight = barBackgroundOn('rgb(247, 247, 245)'); + + expect(onDark).not.toBe(''); + expect(onLight).not.toBe(''); + // If parseRgba returned null for either, both would land on the same + // fallback and this would pass vacuously — so assert they DIFFER. + expect(onDark).not.toBe(onLight); + }); + + it('survives a background colour it cannot parse', () => { + const bg = barBackgroundOn('color(display-p3 0.1 0.1 0.1)'); + expect(bg).not.toBe(''); + }); +}); diff --git a/src/concierge.tsx b/src/concierge.tsx index f200b12..0521e05 100644 --- a/src/concierge.tsx +++ b/src/concierge.tsx @@ -168,11 +168,13 @@ function readCookie(name: string): string { const match = document.cookie.match( new RegExp(`(?:^|;\\s*)${name}=([^;]*)`) ); - if (!match) return ''; + const value = match?.[1]; + if (value === undefined) return ''; try { - return decodeURIComponent(match[1]); + return decodeURIComponent(value); } catch { - return match[1]; + // Malformed percent-encoding — hand back the raw value rather than ''. + return value; } } @@ -278,16 +280,20 @@ export default function ConciergeWidget({ function parseRgba(color: string): [number, number, number, number] | null { const m = color.match(/rgba?\((\d+),\s*(\d+),\s*(\d+)(?:,\s*([\d.]+))?\)/); - if (!m) return null; - return [+m[1], +m[2], +m[3], m[4] !== undefined ? +m[4] : 1]; + const [, r, g, b, a] = m ?? []; + if (r === undefined || g === undefined || b === undefined) return null; + return [+r, +g, +b, a !== undefined ? +a : 1]; } function luminance(r: number, g: number, b: number): number { - const [rs, gs, bs] = [r, g, b].map((c) => { + // A mapped array is `number[]`, not a 3-tuple, so destructuring it + // yields `number | undefined` under noUncheckedIndexedAccess. Mapping + // each channel keeps the arity in the types. + const srgb = (c: number) => { const s = c / 255; return s <= 0.03928 ? s / 12.92 : Math.pow((s + 0.055) / 1.055, 2.4); - }); - return 0.2126 * rs + 0.7152 * gs + 0.0722 * bs; + }; + return 0.2126 * srgb(r) + 0.7152 * srgb(g) + 0.0722 * srgb(b); } function sampleBackground(el: Element): 'dark' | 'light' {