From b541ffa6cae06cdc16f92ebf09f6a275cf2e35a4 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Thu, 6 Aug 2026 11:24:39 -0500 Subject: [PATCH 01/20] test(ui): add hostile-host fixture and computed-style diff harness Measures how much consuming-app global CSS reaches SDK components. Five hostile CSS groups (preflight, bareElements, aggressiveReset, inheritedTypography, important) inject after the SDK style tag, matching real consumer source order. Snapshots poll until two consecutive reads agree. A single-frame read measured CSS transitions rather than leaks, reporting ~101 leaks for every group including `important`, which sets only two properties. Preflight embeds the CLI-compiled output, not the source. The source uses `--theme(...)` build-time calls that a browser drops as invalid. Assertions document the current leak. Phases 3 and 4 narrow them. Co-Authored-By: Claude Opus 5 --- packages/ui/.storybook/preview.tsx | 34 +- .../components/style-isolation.stories.tsx | 404 ++++++++++++++++++ packages/ui/src/test/hostile-host.ts | 293 +++++++++++++ packages/ui/src/test/style-diff.test.ts | 213 +++++++++ packages/ui/src/test/style-diff.ts | 196 +++++++++ 5 files changed, 1139 insertions(+), 1 deletion(-) create mode 100644 packages/ui/src/components/style-isolation.stories.tsx create mode 100644 packages/ui/src/test/hostile-host.ts create mode 100644 packages/ui/src/test/style-diff.test.ts create mode 100644 packages/ui/src/test/style-diff.ts diff --git a/packages/ui/.storybook/preview.tsx b/packages/ui/.storybook/preview.tsx index 670e37ce..918681b2 100644 --- a/packages/ui/.storybook/preview.tsx +++ b/packages/ui/.storybook/preview.tsx @@ -1,4 +1,4 @@ -import React, { useEffect } from 'react'; +import React, { useEffect, useLayoutEffect } from 'react'; import type { Preview, ReactRenderer } from '@storybook/react-vite'; import type { PartialStoryFn, StoryContext } from 'storybook/internal/csf'; @@ -12,6 +12,21 @@ import { initialize, mswLoader } from 'msw-storybook-addon'; import { StorybookEnvCheck } from '../src/test/StorybookEnvCheck'; import { YouVersionProvider } from '../src/components/YouVersionProvider'; import { globalHandlers } from '../src/test/mocks/handlers'; +import { injectHostileCss, removeHostileCss, resolveHostileGroups } from '../src/test/hostile-host'; +import type { HostileGroup } from '../src/test/hostile-host'; + +/** + * Opt-in adversarial host CSS, set per story as `parameters.hostileHost`. + * + * `'all'` injects every group; an array injects only the groups named. Leaving + * the parameter off injects nothing, so every pre-existing story is untouched. + */ +type HostileHostParameter = HostileGroup[] | 'all' | undefined; + +function getHostileGroups(parameters: { hostileHost?: HostileHostParameter }): HostileGroup[] { + const value = parameters.hostileHost; + return value === undefined ? [] : resolveHostileGroups(value); +} const THEME_BACKGROUNDS: Record = { light: '#ffffff', @@ -63,6 +78,23 @@ const preview: Preview = { }; }, [theme]); + // A string key, not the array, so the effect does not re-run every render. + const hostileKey = getHostileGroups(context.parameters).join(','); + + // Layout effect, not effect: the SDK's

'; + document.body.appendChild(root); + + expect([...snapshotComputedStyles(root).keys()]).toEqual(['div', 'div > p']); + + root.remove(); + }); + + it('records every tracked property for each element', () => { + const root = document.createElement('div'); + document.body.appendChild(root); + + const values = snapshotComputedStyles(root).get('div'); + + expect(Object.keys(values ?? {})).toEqual([...TRACKED_PROPERTIES]); + + root.remove(); + }); +}); + +describe('hostile-host groups', () => { + it('covers every class of hostile CSS the ticket names', () => { + expect(ALL_HOSTILE_GROUPS).toEqual([ + 'preflight', + 'bareElements', + 'aggressiveReset', + 'inheritedTypography', + 'important', + ]); + }); + + it('gives every group non-empty CSS', () => { + for (const group of ALL_HOSTILE_GROUPS) { + expect(HOSTILE_GROUPS[group].trim().length).toBeGreaterThan(0); + } + }); + + it("widens 'all' into the full group list", () => { + expect(resolveHostileGroups('all')).toEqual(ALL_HOSTILE_GROUPS); + }); + + it('passes an explicit group list through unchanged', () => { + const groups: HostileGroup[] = ['aggressiveReset', 'important']; + + expect(resolveHostileGroups(groups)).toEqual(groups); + }); +}); diff --git a/packages/ui/src/test/style-diff.ts b/packages/ui/src/test/style-diff.ts new file mode 100644 index 00000000..9c0abf4e --- /dev/null +++ b/packages/ui/src/test/style-diff.ts @@ -0,0 +1,196 @@ +/** + * Computed-style diff harness. + * + * "The component looks wrong" is not a test result. This module turns that + * judgement into a number: snapshot every tracked computed property on every + * element of an SDK subtree, do it once clean and once under hostile host CSS, + * and report the exact (element, property) pairs that moved. + * + * The output is the residual-leak report YPE-4113 asks for, and the same numbers + * are the pass/fail gate for the later isolation phases. + */ + +/** + * The properties the report watches. + * + * Longhands only. A shorthand like `padding` reads back as a single string in + * some engines and as an empty string in others, and the phase gates name + * individual sides (`padding-top`). Every entry here is either a box-model + * property a consumer reset can move, a typography property a consumer `body` + * rule can inherit into us, or a colour. + * + * Deliberately excluded: `transform` and `opacity`, which `tw-animate-css` + * animates. Sampling either one mid-animation produces a leak that is really + * just a timing artefact. + */ +export const TRACKED_PROPERTIES = [ + // Box model + 'box-sizing', + 'padding-top', + 'padding-right', + 'padding-bottom', + 'padding-left', + 'margin-top', + 'margin-right', + 'margin-bottom', + 'margin-left', + 'border-top-width', + 'border-right-width', + 'border-bottom-width', + 'border-left-width', + 'border-top-left-radius', + 'border-top-right-radius', + 'border-bottom-right-radius', + 'border-bottom-left-radius', + // Typography + 'font-family', + 'font-size', + 'font-weight', + 'font-style', + 'line-height', + 'letter-spacing', + 'word-spacing', + 'text-align', + 'text-transform', + 'text-indent', + 'white-space', + 'text-decoration-line', + 'list-style-type', + // Colour + 'color', + 'background-color', +] as const; + +export type TrackedProperty = (typeof TRACKED_PROPERTIES)[number]; + +/** Structural path -> computed values for the element at that path. */ +export type StyleSnapshot = Map>; + +/** Elements whose computed style says nothing about how the component looks. */ +const IGNORED_TAGS = new Set(['SCRIPT', 'STYLE', 'LINK', 'META', 'TEMPLATE']); + +/** + * A path that identifies an element by where it sits, not by what it is called. + * + * Class names change between a clean and a hostile render is not the worry; + * the worry is that any identity based on styling would be circular. Tag name + * plus same-tag sibling index is stable as long as the DOM shape is stable, + * which is exactly the condition under which a diff is meaningful. + * + * @example `div > ul > li[2] > button` + */ +function pathOf(element: Element, parentPath: string | null): string { + const tag = element.tagName.toLowerCase(); + + if (parentPath === null) return tag; + + const parent = element.parentElement; + let index = 0; + if (parent) { + for (const sibling of parent.children) { + if (sibling === element) break; + if (sibling.tagName === element.tagName) index += 1; + } + } + + const segment = index === 0 ? tag : `${tag}[${index}]`; + return `${parentPath} > ${segment}`; +} + +/** + * Reads every tracked property on `root` and its descendants. + * + * Call this against the SDK subtree, not against the Storybook canvas. The + * canvas root is consumer DOM, and the SDK makes no promise about it, so + * including it would report a false positive on every run. + */ +export function snapshotComputedStyles(root: Element): StyleSnapshot { + const snapshot: StyleSnapshot = new Map(); + + const visit = (element: Element, parentPath: string | null): void => { + if (IGNORED_TAGS.has(element.tagName)) return; + + const path = pathOf(element, parentPath); + const computed = getComputedStyle(element); + const values: Record = {}; + + for (const property of TRACKED_PROPERTIES) { + values[property] = computed.getPropertyValue(property); + } + + snapshot.set(path, values); + + for (const child of element.children) visit(child, path); + }; + + visit(root, null); + return snapshot; +} + +/** One property on one element that the host CSS moved. */ +export type StyleLeak = { + path: string; + property: string; + clean: string; + hostile: string; +}; + +/** + * Compares two snapshots of the same subtree. + * + * Paths present in only one snapshot are skipped rather than reported. A + * missing path means the DOM shape changed between the two renders, which is a + * harness problem, not a style leak, and reporting it as a leak would bury the + * real findings. + */ +export function diffSnapshots(clean: StyleSnapshot, hostile: StyleSnapshot): StyleLeak[] { + const leaks: StyleLeak[] = []; + + for (const [path, cleanValues] of clean) { + const hostileValues = hostile.get(path); + if (!hostileValues) continue; + + for (const property of Object.keys(cleanValues)) { + const cleanValue = cleanValues[property]; + const hostileValue = hostileValues[property]; + + if (hostileValue === undefined) continue; + if (cleanValue === hostileValue) continue; + + leaks.push({ path, property, clean: cleanValue ?? '', hostile: hostileValue }); + } + } + + return leaks; +} + +/** The distinct property names in a leak set, sorted. Test assertions read this. */ +export function leakedProperties(leaks: StyleLeak[]): string[] { + return [...new Set(leaks.map((leak) => leak.property))].sort(); +} + +/** Leak counts per property, highest first. The residual report reads this. */ +export function summarizeLeaks(leaks: StyleLeak[]): { property: string; count: number }[] { + const counts = new Map(); + + for (const leak of leaks) { + counts.set(leak.property, (counts.get(leak.property) ?? 0) + 1); + } + + return [...counts.entries()] + .map(([property, count]) => ({ property, count })) + .sort((a, b) => b.count - a.count || a.property.localeCompare(b.property)); +} + +/** + * Renders a leak set as a table for the console. + * + * The residual report is read by a human deciding whether to ship Shadow DOM, + * so the harness prints findings rather than only asserting on them. + */ +export function formatLeakReport(label: string, leaks: StyleLeak[]): string { + if (leaks.length === 0) return `${label}: no leaks`; + + const lines = summarizeLeaks(leaks).map(({ property, count }) => ` ${property} x${count}`); + return [`${label}: ${leaks.length} leak(s)`, ...lines].join('\n'); +} From 0c8e55d4843592e96477dcc07118be9e9fa266d9 Mon Sep 17 00:00:00 2001 From: Cameron Pak Date: Thu, 6 Aug 2026 12:21:03 -0500 Subject: [PATCH 02/20] fix(ui): stamp data-yv-sdk and data-yv-theme on every exported root Phase 4 gates all SDK CSS on [data-yv-sdk], so an unstamped root loses its styling. Five roots carried no attribute: ProfileAvatar, Separator, Textarea, the Dialog overlay, and BibleVersionPickerLanguageTrigger. The stamp is not inert. theme.css declares light tokens on the bare [data-yv-sdk] selector with dark as a nested override, so an element that gains the attribute inside a dark scope reverts to light. Every stamp names the enclosing scope's theme, not the provider's. FootnoteContent wrote data-yv-theme from an optional prop with no default, so React dropped the attribute and it rendered light inside a dark reader. It now falls back to useTheme(). ui/button.tsx is deliberately not stamped. It has no theme in scope, it is not a public export, and Phase 4's descendant arm covers it. scope-attribute.test.tsx asserts every component export carries the attribute, so a new component cannot forget. Co-Authored-By: Claude Opus 5 --- packages/ui/src/components/bible-reader.tsx | 7 +- .../src/components/bible-version-picker.tsx | 6 + packages/ui/src/components/profile-avatar.tsx | 8 + .../src/components/scope-attribute.test.tsx | 395 ++++++++++++++++++ .../components/style-isolation.stories.tsx | 47 +++ packages/ui/src/components/ui/dialog.tsx | 5 + packages/ui/src/components/ui/separator.tsx | 9 + packages/ui/src/components/ui/textarea.tsx | 9 + packages/ui/src/components/verse.tsx | 6 +- 9 files changed, 490 insertions(+), 2 deletions(-) create mode 100644 packages/ui/src/components/scope-attribute.test.tsx diff --git a/packages/ui/src/components/bible-reader.tsx b/packages/ui/src/components/bible-reader.tsx index d70fa3ca..1996a6fd 100644 --- a/packages/ui/src/components/bible-reader.tsx +++ b/packages/ui/src/components/bible-reader.tsx @@ -1152,7 +1152,7 @@ function UserMenu() { const { t } = useTranslation(undefined, { i18n }); const { auth, signIn, signOut, userInfo } = useYVAuth(); const yvContext = useContext(YouVersionContext); - const { onSignInPress, onSignOutPress } = useBibleReaderContext(); + const { background, onSignInPress, onSignOutPress } = useBibleReaderContext(); // Prefer host-supplied native actions (e.g. the Expo DOM wrapper bridges these to // native PKCE sign-in). Fall back to the Web SDK's own auth for pure-web usage. @@ -1180,6 +1180,11 @@ function UserMenu() { name={userInfo?.name} src={userInfo?.getAvatarUrl(32, 32)?.toString()} aria-label={userInfo?.name || t('userAvatarAlt')} + // ProfileAvatar stamps `data-yv-sdk`, which re-declares the + // `--yv-*` tokens on itself. The reader's `background` can differ + // from the provider theme, so hand it the reader's theme or the + // avatar would flip back to light tokens inside a dark reader. + data-yv-theme={background} className="yv:size-full" /> diff --git a/packages/ui/src/components/bible-version-picker.tsx b/packages/ui/src/components/bible-version-picker.tsx index d94e047f..93b0aa08 100644 --- a/packages/ui/src/components/bible-version-picker.tsx +++ b/packages/ui/src/components/bible-version-picker.tsx @@ -524,6 +524,7 @@ export function BibleVersionPickerLanguageTrigger({ }: BibleVersionPickerLanguageTriggerProps): React.ReactElement { const { t } = useTranslation(undefined, { i18n }); const { + background, filteredVersions, filteredRecentVersions, setIsLanguagesOpen, @@ -543,6 +544,11 @@ export function BibleVersionPickerLanguageTrigger({ return (