diff --git a/.changeset/plenty-donkeys-shake.md b/.changeset/plenty-donkeys-shake.md new file mode 100644 index 00000000..17a137de --- /dev/null +++ b/.changeset/plenty-donkeys-shake.md @@ -0,0 +1,75 @@ +--- +'@youversion/platform-core': major +'@youversion/platform-react-hooks': major +'@youversion/platform-react-ui': major +--- + +Scope all SDK CSS to `data-yv-sdk` subtrees, so the global CSS of a host app +cannot change SDK components. + +Every SDK selector now carries the gate +`:is([data-yv-sdk], [data-yv-sdk] *:where(:not([data-yv-slot], [data-yv-slot] *)))`. +The stylesheet then ships in two halves: everything on a property exemption list +stays unlayered and normal, and every other declaration goes into a cascade layer +named `yv` and carries `!important`. `theme.css` also declares the inherited +properties (`font`, `color`, `box-sizing`, `margin`, `padding`, `border`) on SDK +roots and on their descendants. A gate on a selector does not stop inheritance +from consumer DOM. + +**What this corrects.** A host reset such as `* { box-sizing: content-box }`, a +bare `button { padding: 1rem }`, Tailwind Preflight, a `body { font-family; +color; line-height }` block, `button { padding: 2rem !important }` and +`#app button { padding: 1rem }` all changed the appearance of SDK components +before. None of them change it now. A regression harness in +`packages/ui/src/components/style-isolation.stories.tsx` measures every component +against seven host-CSS fixtures and asserts zero leaks on all of them. + +**Your own content inside an SDK component keeps your styling.** Content you +pass as `children` or as a render prop is wrapped in an element marked +`data-yv-slot`, and no SDK rule matches inside it. Inheritance still applies: +your content inherits `color`, `font-size` and the rest from where it sits. +`BibleReader.Root` children and a picker `Trigger` rendered with `asChild` do +not get the wrapper — add `data-yv-slot` to your own element there. + +**Your root font size no longer scales SDK components.** A host page with +`html { font-size: 62.5% }` used to shrink every SDK size by 37.5 percent, +because a `rem` resolves against the document root and no selector or layer can +stop that. The build now converts every `rem` in the sheet to `px` at +1rem = 16px, and the SDK root declares `font-size: 16px`. The trade-off is that +SDK text no longer grows with a reader's raised browser default font size. +Browser zoom is unaffected. To scale the SDK on purpose, override +`[data-yv-sdk] { font-size }` — it stays outside the layer for that reason — or +pass a component's `fontSize` prop. + +**What breaks.** Any consumer CSS that overrides an SDK declaration. + +- Rules that target SDK internals (our class names, `data-slot` values or DOM + structure) now lose the cascade. SDK rules gained specificity: the gate adds + `0,1,0` on its own, on top of whatever the rule already had. +- Rules that depend on inheritance into SDK components now lose. The SDK declares + those properties itself. +- **Rules that use `!important` now lose too.** For important declarations the + cascade reverses layer order and ranks unlayered CSS last, so a declaration in + `@layer yv` beats an unlayered important one at any specificity. This was the + documented escape hatch in earlier previews of this release. It is gone. + +**What you can still change.** A short property list stays outside the layer, +because our popovers and animations set it at runtime: `position`, +`top`/`right`/`bottom`/`left`, `inset-*`, `z-index`, `min-width`, `visibility`, +`pointer-events`, `transform` and the rest of the transform pipeline, `opacity`, +`height`, `filter`, every `animation-*` and `transition-*`, `font-size`, +`background-color`, `border-*` and their shorthands, and every `--*` custom +property. + +**How to migrate.** Replace each consumer CSS override with a supported path: + +1. Set `--yv-*` design tokens on `[data-yv-sdk]`. +2. Use the `theme` prop on `YouVersionProvider`, or the `background` prop on a + component. +3. Open an issue if neither path covers your case. + +For the rationale and the rejected alternatives (`@scope` and shadow DOM), read +`docs/adr/0005-scope-sdk-css-to-data-yv-sdk-subtrees.md`, +`docs/adr/0006-layer-and-importantize-the-sdk-sheet.md`, +`docs/adr/0007-convert-rem-to-px-in-the-sdk-sheet.md` and +`docs/adr/0008-stop-sdk-css-at-consumer-slots.md`. diff --git a/docs/adr/0005-scope-sdk-css-to-data-yv-sdk-subtrees.md b/docs/adr/0005-scope-sdk-css-to-data-yv-sdk-subtrees.md new file mode 100644 index 00000000..f6288d08 --- /dev/null +++ b/docs/adr/0005-scope-sdk-css-to-data-yv-sdk-subtrees.md @@ -0,0 +1,299 @@ +# 5. Scope SDK CSS to `[data-yv-sdk]` subtrees instead of cascade layers + +Date: 2026-08-06 + +## Status + +Accepted. This decision replaces the cascade-layer strategy from `bcfb868` +(v1.20.0). + +Amended 2026-08-07 by +[ADR-0006](0006-layer-and-importantize-the-sdk-sheet.md). The +`:is([data-yv-sdk], [data-yv-sdk] *)` gate below is unchanged and still carries +the outbound guarantee. Two statements below are superseded: the sheet is no +longer in no layer, and a consumer `!important` rule no longer reaches our +components. ADR-0006 has the cascade math and the property exemption list. The +sections marked **Superseded** name what changed. + +Amended again 2026-08-07 by +[ADR-0008](0008-stop-sdk-css-at-consumer-slots.md). The gate's descendant arm is +now `[data-yv-sdk] *:where(:not([data-yv-slot], [data-yv-slot] *))`, so SDK CSS +stops at consumer content the SDK only passed through. The gate still adds +0,1,0: the exclusion sits inside `:where()`, which contributes nothing, so every +specificity claim below still holds. Every literal gate string below is the +pre-0008 form. Read it as an outline of the mechanism, not as the shipped +selector. + +## Context + +A partner puts `BibleTextView` into their app. Their `button { padding: 1rem }` +changes the shape of our version picker trigger. Their +`body { font-family: Comic Sans }` changes our verse text. The partner sees a +broken Bible experience and has no supported way to correct it. Ticket YPE-4113 +covers this problem. + +### This decision changed three times before + +| Date | Commit | Version | Position | +| --- | --- | --- | --- | +| 2026-01-09 | `8e3a672` | 0.10.1 | Added `scripts/strip-layers.js`. SDK CSS in no layer. | +| 2026-01-13 | `694325f` | 1.6.2 | "Opt out of CSS layers." Deleted `strip-layers.js`. | +| 2026-03-09 | `bcfb868` | 1.20.0 | Used layers again, written into `global.css` source. | + +Only the changelog records the last change. This ADR exists to prevent a fourth +change by accident. + +### Why cascade layers cannot work + +`global.css:33` declared nine layers and put every SDK rule into a `yv-sdk-*` +layer. CSS Cascade 5 [§6.1](https://www.w3.org/TR/css-cascade-5/#cascade-sort) +states two facts. A declaration that is in no layer joins an implicit final +layer. The browser compares layer order **before** specificity. +[MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/@layer) says it plainly: +"Styles that are not defined in a layer always override styles declared in named +and anonymous layers." + +A consumer `button {}` rule in no layer, at 0,0,1, thus overrides every `yv:` +utility at any specificity. This was not a fault in the layer setup. The layer +setup worked as designed. `bcfb868` chose layers on purpose, to stop SDK styles +from overriding consumer CSS. The same choice guaranteed the leak in the other +direction. + +### Two channels for a leak, not one + +| Channel | Example | Why it overrides the SDK rule | +| --- | --- | --- | +| Direct match | `button { padding: 1rem }` matches our button | CSS in no layer overrides layered CSS at any specificity | +| Inheritance | `body { color: green }` flows into our text | The SDK declares no `color`, so the value is inherited | + +The second channel is independent of the cascade. An inherited value applies +where the element declares nothing. Specificity and layer order do not change +this. Only a declaration changes it. + +## Decision + +### 1. SDK CSS in no layer, with a `[data-yv-sdk]` gate on every selector + +> **Superseded in part by ADR-0006.** The gate stands. "In no layer" now holds +> only for the exempt half of the sheet. Everything else ships in `@layer yv` +> and is `!important`. The reasoning below — that a layered *normal* declaration +> loses to unlayered consumer CSS — is correct, and it is why ADR-0006 splits +> the sheet by property instead of layering all of it. + +Do not protect the consumer with cascade order. Protect the consumer with the +DOM subtree. + +```css +/* before */ +.yv\:mt-4 { margin-top: 1rem } + +/* after */ +:is([data-yv-sdk], [data-yv-sdk] *).yv\:mt-4 { margin-top: 1rem } +``` + +`:is()` takes the specificity of its most specific argument, so the gate adds +exactly 0,1,0. The compound form matches the marked root and its descendants in +one selector. + +This corrects both directions at the same time: + +- **Outbound.** SDK CSS cannot match DOM that the SDK did not render. This is a + structural guarantee, not a bet on cascade order. It is stronger than the + protection that layers gave the consumer. +- **Inbound.** Inside an SDK subtree, every SDK rule is at 0,1,0 or more. It thus + overrides a consumer element selector at 0,0,1 and a universal selector at + 0,0,0. + +The rewrite runs after the Tailwind build, in +`packages/ui/scripts/scope-selectors.mjs`. Tailwind v4 has no equivalent +function. `prefix(yv)` only renames classes. `@import "tailwindcss" important` +takes no selector argument. The v3 option `important: '#app'` has no v4 +replacement. Adam Wathan says that a selector around the import works "mostly by +coincidence" +([discussion #13779](https://github.com/tailwindlabs/tailwindcss/discussions/13779)). +It also breaks: after the nesting is flattened it emits `[data-yv-sdk] :root`, +which can never match. + +The script uses Lightning CSS, not a regular expression and not +`postcss-prefix-selector`. Its `Selector` visitor is a typed structural API, so +it cannot corrupt an escaped class name such as `.yv\:mt-4`. `@keyframes`, +`@font-face` and `@property` produce no `Selector` nodes, so the script skips +them by construction. The script also flattens CSS nesting. Flattening is +necessary because `bible-reader.css` and `@utility touch-hitbox` emit nested `&` +rules, which a simple prefixer corrupts. We tested `postcss-prefix-selector@2.1.1` +against the real output of this repo and rejected it. It rewrites `:host` into +`[data-yv-sdk] :host`, which can never match, and thus removes every theme +variable without a warning. + +`:root` and `:host` keep no gate. Those rules declare only `--yv-*` custom +properties. They render nothing and cannot collide. + +The script parses its own output again and fails the build on any selector +without a gate. That check is the guarantee, not a comment and not a string +match. `scripts/verify-styles.js` adds two more checks. `dist/index.js` must +contain `:is([data-yv-sdk],[data-yv-sdk] *)`, and it must not contain +`@layer yv-sdk-`. The second check names the `yv-sdk-` prefix on purpose. +Tailwind emits `@layer properties` by itself for the `@property` fallback, +whatever our directives say. + +### 2. Declare the inherited properties, and close the holes that the gate cannot reach + +The gate increases specificity. It does nothing about a property that the SDK +never declares. Three changes to `packages/core/src/styles/theme.css` close that +class of hole: + +1. **Remove the `:where()`.** The reset block was `:where([data-yv-sdk])` at + 0,0,0, which a bare `button {}` overrides. It is now `[data-yv-sdk]`. +2. **Declare the inherited set on the SDK root**: `font-family`, `color`, + `letter-spacing`, `word-spacing`, `text-align`, `text-transform`, + `text-indent`, `white-space`, `text-shadow`, `font-variant`. Every element + with `data-yv-sdk` declares the full set again, so portalled surfaces that + inherit directly from `document.body` are covered too. `direction` is not in + the set: `bible-reader.css` controls RTL, and `text-align: start` follows the + direction. +3. **Add `color: inherit` to the descendant block, and the box model to the + root.** The harness found these two, not the plan. `color` on the root stops + `body { color: … }`, because that value arrives only by inheritance. It does + nothing about `ul { color: #f0f }`, which matches an SDK element directly. The + descendant block is `[data-yv-sdk] *`, which does not match the marked + element. A consumer `* { box-sizing: content-box }` rule does match it. + +We considered `revert-layer` and rejected it. CSS Cascade 5 §7 returns a property +to the layer below, or to the previous origin when no layer is below. For an +inherited property the UA origin declares nothing. The returned value is thus the +inherited value, which is the consumer `body` rule. `revert-layer` removes rules +that target our elements. It does not stop inherited values. + +### 3. `@scope` rejected, without a test + +`@scope` limits the rules written inside it. It does not block inbound styles. +[MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/@scope) is explicit: +"while `@scope` allows you to isolate the application of selectors to specific DOM +subtrees, it does not completely isolate the applied styles to within those +subtrees. This is most noticeable with inheritance. Properties that are inherited +by children (for example `color` or `font-family`) will still be inherited, +beyond any set scope limit." + +[CSS Cascade 6 §2.5.3](https://www.w3.org/TR/css-cascade-6/#scope-nesting) +limits only the selectors in the block. A stylesheet elsewhere in the document +can still match elements inside a scoped subtree. Scope proximity is a cascade +criterion below specificity, so it cannot decide a comparison that the gate does +not decide already. + +`@scope` gives what `:is([data-yv-sdk], [data-yv-sdk] *)` gives already, without +the specificity. It also adds a dependency with Baseline status "newly available" +(December 2025). There is nothing to test. + +### 4. Shadow DOM not adopted. The condition, fixed before the numbers existed + +> **Still not adopted, and the condition is now moot.** ADR-0006 closed the +> `!important` residual as well, so there is no measured residual of either +> kind. The costs table below is unchanged and still applies. + +**If the residual leak includes rules that do not use `!important`, recommend +shadow DOM. If it does not, do not recommend shadow DOM.** + +A consumer who writes `!important` against our elements makes an explicit choice. +We treat that choice as out of contract. The design discussion recorded this +condition before the harness produced one number. The recommendation thus cannot +be adjusted to fit the answer. + +**Measured result: 100 percent of the residual uses `!important`.** +`docs/style-isolation-residual-leak.md` has the numbers. Four of the five consumer CSS +groups report zero leaks on all thirteen consumer-host stories. The remaining +leaks come from two consumer declarations. Both use `!important` and both target +`button`: 880 leaks on 155 buttons in 8 components. No rule without `!important` +gets through. + +The condition is not met. We do not adopt shadow DOM. + +Shadow DOM has these costs, all found during the research: + +| Cost | Evidence | +| --- | --- | +| Radix portals mount to `document.body`, outside the shadow root | `popover.tsx:43`, `dialog.tsx:28`. No `container` prop anywhere in the repo | +| Radix `FocusScope` breaks at the boundary. `document.activeElement` returns the host | [radix-ui/primitives#3353](https://github.com/radix-ui/primitives/issues/3353), open | +| `aria-hidden` hides open dropdown content from the accessibility tree | [radix-ui/primitives#1772](https://github.com/radix-ui/primitives/issues/1772), open | +| Dismissable-layer outside-click reads `event.target` as the host | [radix-ui/primitives#2433](https://github.com/radix-ui/primitives/pull/2433), open, "Needs Investigation" | +| Font and color values still cross the boundary by inheritance | [MDN, "Using shadow DOM"](https://developer.mozilla.org/en-US/docs/Web/API/Web_components/Using_shadow_DOM). Top-level shadow elements inherit from the host | +| React 19 `
'; + 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('consumer-host groups', () => { + it('covers every class of consumer CSS the ticket names', () => { + expect(ALL_CONSUMER_CSS_GROUPS).toEqual([ + 'preflight', + 'bareElements', + 'aggressiveReset', + 'inheritedTypography', + 'important', + 'highSpecificity', + 'remRebase', + ]); + }); + + it('gives every group non-empty CSS', () => { + for (const group of ALL_CONSUMER_CSS_GROUPS) { + expect(CONSUMER_CSS_GROUPS[group].trim().length).toBeGreaterThan(0); + } + }); + + it("widens 'all' into the full group list", () => { + expect(resolveConsumerCssGroups('all')).toEqual(ALL_CONSUMER_CSS_GROUPS); + }); + + it('passes an explicit group list through unchanged', () => { + const groups: ConsumerCssGroup[] = ['aggressiveReset', 'important']; + + expect(resolveConsumerCssGroups(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..d0e875dd --- /dev/null +++ b/packages/ui/src/test/style-diff.ts @@ -0,0 +1,238 @@ +/** + * Computed-style diff harness. + * + * "The component looks wrong" is not a test result. This module turns that + * judgment into a number. It reads every tracked computed property on every + * element of an SDK subtree. It does that once clean, and once under consumer + * host CSS. Then it reports the exact (element, property) pairs that moved. + * + * The output is the residual-leak report that YPE-4113 asks for. The same + * numbers are the pass/fail gate for the later isolation phases. + */ + +/** + * The properties that the report watches. + * + * Longhands only. A shorthand such as `padding` reads back as one string in some + * engines, and as an empty string in others. The phase gates also name + * individual sides (`padding-top`). Each entry here is one of three kinds: + * + * - A box-model property that a consumer reset can move. + * - A typography property that a consumer `body` rule can inherit into us. + * - A color. + * + * Two properties are absent on purpose: `transform` and `opacity`. + * `tw-animate-css` animates both. A sample taken during an animation reports a + * leak that is only a timing artifact. + */ +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', + // Color + '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