Make the default theme a default: ship it in a cascade layer - #50
Merged
Conversation
`applyThemeVariables` wrote DEFAULT_THEME_VARS to `documentElement.style`. An inline style outranks every author stylesheet, so the neutral map beat the very declarations it exists to back up — a default that wins the cascade is not a default. This is not a corner case. `hostContext.styles.variables` is a closed enum, so a host whose design system is larger than that enum has to deliver the remainder as a stylesheet injected into the app document; that is the only channel it has. Every such var the SDK also defaulted was replaced by the SDK's value permanently, with no self-heal path — a host sending its brand accent that way rendered in our neutral blue. The defaults now install as `@layer synapse-defaults`. Unlayered beats layered, so a host's or an app's `:root` rule wins, while a var nobody declares still resolves to a theme-correct neutral default. Host variables that do arrive over the protocol still go inline and still win over both. An app overriding a token now needs a plain `:root` rule rather than `!important`. One managed `<style>` element, replaced in place, so a mode flip swaps the map atomically and repeat calls are cheap. Falls back to `documentElement` where `head` is absent, and still no-ops without a `document`. Verified in Chrome 148 against a real host: the four tokens a larger-than-enum host loses (`--color-text-accent`, `--nb-color-danger/success/warning`) now compute to the host's values, protocol-delivered tokens are unchanged, an undeclared var still gets the neutral default in both modes, and an app's own `:root` rule overrides it. Four of the six new tests fail if the defaults go back inline. Also restores the CHANGELOG's missing `## [0.9.0]` and `## [0.8.0]` headings (#48), whose absence filed their entries — including a `### Breaking` — under 0.10.0, misdating the `FileResult` break by two minors. Two lines in a file this change already edits; separable on request.
The `@layer` change alters the built IIFE, and `python/nimblebrain_synapse/ _assets/synapse-ui.iife.js` is a checked-in copy of it, so CI's freshness diff failed — `npm run ci` doesn't cover that step. Rebuilt and re-copied, and bumped `__client_version__` to 0.13.1 to match `package.json`, which is the other gate a version bump trips. Worth noting what this proves: the vendored IIFE now carries the layer, so the fix reaches the cross-host `connectUI` consumers too, not only the ext-apps paths. Both CI gates replicated locally before pushing this time.
…e guard Writing the defaults inline had a load-bearing side effect I removed without replacing: it rewrote every defaulted key on every call, which healed any stale host value pinned inline. A layer cannot do that — it can't outrank an inline property. So an ext-apps `host-context-changed` that flips mode while omitting `styles` (the spec's normal partial shape, which `extractTheme` renders as an empty var set) left the host's light values pinned inline while the layer flipped to dark. Reproduced against origin/main as control: `--color-background-primary` stayed `#ffffff` here where main showed `#18181b`, and `--color-text-secondary` on that retained white computes to 2.56:1, under AA. A defaulted key the incoming set doesn't carry is now removed rather than overwritten, which is strictly less destructive than the old behaviour and keeps this change's whole point: the cascade resolves it against the host's stylesheet, the app's rule, then the layer. Verified in Chrome both ways — with a host stylesheet the host's own declaration wins (it is the host's to update), and with no stylesheet the layer takes over to a coherent dark. A non-defaulted host var is left alone, since the SDK has no opinion on it and absent means unchanged. `document.head ?? document.documentElement` is gone. `ensureStyle` in this same package calls `document.head.appendChild` unguarded, browsers always have a head, and so does happy-dom — the only thing that reached the fallback was a test manufacturing the condition with `Object.defineProperty`. Both removed. Also: the module docblock described the mechanism this branch deletes; `prepend` is load-bearing for layer order and now says so; the template literal is safe only because its input is module-local, which matters because `applyThemeFontFaces` 90 lines below cites non-concatenation as deliberate, so a later change folding host vars in would make it an injection sink; and the idempotence test counted every `<style>` in the document rather than ours. CHANGELOG now warns that an injected `<style>` is subject to `style-src` where a CSSOM write was not, so a strict-CSP host loses the map — with constructable stylesheets named as the escape hatch if that becomes real.
The clear loop tested `k in incoming`; the write loop tested that plus `typeof v === "string"`. A defaulted key arriving with a non-string value fell in the gap — not removed, not written — so the previous theme's value stayed pinned inline, where the layer, the host's stylesheet and the app's `:root` all cannot reach it. That is the un-self-healing pin this change exists to remove, re-entered through the malformed-value door instead of the omitted-key one, and it is a regression against main rather than a pre-existing gap: same input, `--color-text-primary` after a dark flip read `#111827` here where main gave `#fafafa`. Wire data is untyped, which is why `normalizeFontFaces` exists too. Normalized once into a filtered map that both loops read, which also deletes the per-entry re-checks. I had written in the downstream PR that two spellings of one rule are what drift, then shipped two here. The clear loop's docblock justified itself with #46's symptom. Its durable reason is a host legitimately narrowing its key set; #46 (core.ts replacing the context wholesale, which connect.ts already defends against) is the accidental source of an empty var set today, and fixing it is the better outcome — it keeps the host's brand across a mode flip rather than collapsing to our neutral — without making the loop unnecessary. Both now stated, with the issue cited so the rationale can't outlive the bug. The suite header claimed to pin the property; happy-dom computes no cascade layers, so it pins the mechanism and the cascade is verified by hand in Chrome. Says so now, and says why the mechanism is still the right thing to assert. The `--font-sans` test reads as a known limit rather than as intent. `@layer synapse-defaults` is a new public contract, so theming.mdx now documents the resolution order and that overriding a default needs no `!important`. That page also still claimed every token falls back to its `var()` second argument standalone, which stopped being true for colors in 0.10.2. The CSP note gains the other way the block gets dropped wholesale — a browser without `@layer`.
The theming page ranked the host's injected stylesheet above the app's own `:root` as though the cascade decided between them. It does not: both are unlayered `:root` rules of identical specificity with no `!important`, so document order is the only thing separating them. And the order is fixed against the host — the runtime injects its theme block by replacing the `<head>` opening tag, putting it ahead of the app's bundled styles — so an app that declares `--color-text-accent` overrides the host's brand accent. That is the mirror image of the bug this branch fixes, documented as impossible, on the one page an app author or third-party host integrator reads to reason about exactly this question. Reproduced in a browser against the real injection order before rewriting it. The tier collapses into one and the page states the rule rather than an order, then says plainly that a host token delivered by stylesheet is the app's to override, since that is both true and a reasonable policy.
The clear loop iterated `DEFAULT_THEME_VARS[mode]`, but that is not the set this module writes inline — the host writes it. `styles.variables` carries any spec-enum key, and roughly 25 theme-sensitive ones (`--color-text-danger`, `--color-background-inverse`, `--color-ring-warning`, …) have no neutral default. A host that sends one and then hits `core.ts`'s wholesale-replace path kept it pinned at the previous mode's value while the layer flipped: the half-light/half-dark state the clear loop exists to prevent, still reachable by a different door. Keying removal off a module-level set of the keys last written closes it, and costs three lines rather than the key history the old test comment imagined it would need. It also narrows the blast radius in the other direction — the SDK now never removes an inline property it did not set, so an app driving `documentElement.style` itself is left alone, which the neutral-map loop did not guarantee. The docblock also drops the claim that a host wins through either channel. It wins over this module's default through either, but only the protocol channel outranks the app; between a host stylesheet and the app's own rules document order decides, and not by anything this module does.
A patch understates this. The change adds a capability the docs now advertise — an app overrides an SDK default with a plain `:root` rule, no `!important` and no layer of its own — and it moves cascade precedence for every existing consumer that declares one of these vars. `0.14.0` is the honest signal for a consumer deciding whether a bump can move pixels. Re-vendors the Python IIFE from this tree's build and moves `__client_version__` with it, which the CI freshness diff and pin gate both require.
The standalone-fallback claim was rewritten on `concepts/theming.mdx` when the defaults moved into a layer, and the identical sentence survived on its sibling `components/tokens.mdx` — same heading, same list, still promising "a generic blue" for colors. It is wrong in the direction that matters. Colors now resolve through the `synapse-defaults` layer, and a `var()` second argument is a static literal, so the sentence describes light-mode literals rendering in dark. The two pages are one page's worth of contract split across two files; a reader who lands on the components page gets the pre-0.10.2 answer. Points at the concepts page for the full order rather than restating it, so the next change to the resolution rules has one place to land.
`applyDefaultThemeLayer` reused whatever `getElementById` returned for its own id. An app that happens to own `synapse-theme-defaults` had that element's text content overwritten, and since a non-style element carries no rules, the whole default map silently stopped applying — a failure with no error and no visible cause short of reading the DOM. An `instanceof` check makes the mismatch fall through to creating the element this module owns, which is the behaviour the id was standing in for. Also syncs `package-lock.json`, which `cb8c5da` left at `0.13.1` while `package.json` and `__client_version__` moved to `0.14.0`. `npm ci` does not fail on a root-version mismatch and the CI pin gate only covers the Python side, so a clean checkout rewrote it on first install — a third source of truth for the version, in the commit that sets it.
Three things this round, all narrowing the gap between what the change does and what a consumer reading it is told. **The prerequisite ships.** The analysis of what a host must do to adopt this lived only in the PR description, which no consumer sees. The CHANGELOG is in the npm tarball and read two `### Fixed` bullets as an unqualified improvement. It now states the rule — anything varying with light/dark has to reach the app on a channel the host can update — names the NimbleBrain release that satisfies it, and says not to bump ahead of it. `theming.mdx` carries the same caution for anyone integrating a new host. **The `instanceof HTMLStyleElement` guard is removed.** It was added on a suggestion and it was the wrong call: no test covers it (the suite is 496/496 with it gone), the threat it names is an app squatting a namespaced id, and its fall-through creates a *second* element with the same id — breaking the "one element, replaced in place" invariant the docblock above it asserts. A real fix here would hold a module reference rather than round-tripping through `getElementById`, and would come with a test; neither belongs in a PR about cascade precedence. **Three call sites lose a description of the old mechanism.** `core.ts`, `connect.ts` and `provider.tsx` each said the defaults "back any var the host omits, then host values win," which describes the inline write this change replaced. The module docblock was corrected in this same branch and these were missed. Also retitles theming.mdx's standalone section, whose heading promised the `var()` fallbacks win directly above a sentence explaining that colors resolve through the layer instead.
Both token pages told an app author that with no host, color tokens resolve through `synapse-defaults` and stay theme-correct in dark — naming "a static render, a Storybook, a unit test" as the cases. Only one nearby case is true. `applyTheme` has five call sites and every one is inside a connection path (`connect`, `createSynapse`, `connectUI`, `SynapseProvider`). Nothing under `ui/` installs the layer, so rendering a component directly installs none: verified by rendering `<Button>` with no host and no provider and finding no `#synapse-theme-defaults` element. Those renders fall back to the static `var()` literals, light-flavoured in both modes — the exact failure the default map exists to prevent, promised as solved on the page an author reads to reason about it. The split is now stated as one: a connection with no host behind it (the `inline` adapter, or a host that ships no tokens) gets the layer; a render that never connects does not. The dark-mode note is deliberate rather than prescriptive — `extractTheme` resolves a missing context to `light`, so wrapping a story in `SynapseProvider` installs the layer but installs the light one, and saying "wrap it" would trade one wrong promise for another.
mgoldsborough
added a commit
that referenced
this pull request
Jul 28, 2026
Three releases landed on main after this branch's last push: the cascade layer (#50) and its framing (#52), cut as npm v0.14.0, and the Python package's catch-up release (#53), cut as nimblebrain-synapse-v0.4.0. npm 0.14.0 is published, so the warm removal stays under `[Unreleased]` and releases as 0.15.0. The migration note's unaffected range widens to `^0.11.0`-`^0.14.0` to match what consumers can now be pinned to. The vendored Python IIFE is rebuilt from the merged source rather than resolved to either side, since #50 changed the build output. Both nimblebrain-synapse-v0.4.0 and v0.14.0 are tagged at commits that predate this merge, so both shipped artifacts still carry the warm channel their changelogs describe. theme-defaults.ts auto-merged: #50's cascade-layer docblock and this branch's correction to the neutrality claim are different hunks.
This was referenced Jul 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #49
Closes #48
The bug
applyThemeVariableswroteDEFAULT_THEME_VARStodocument.documentElement.style— the inline style attribute — before layering the host's variables on top. An inline style outranks every author stylesheet, so the neutral map beat the declarations it exists to back up.Its own docblock described the intended behaviour correctly: "the host's values win for the keys it provides, and any var it omits still resolves to a theme-correct neutral default." That only held for vars the host delivered over the protocol.
And a host cannot always use the protocol.
hostContext.styles.variablesis a closed enum, so a host whose design system is larger than the enum has to deliver the remainder as a stylesheet injected into the app document — that is the only channel available to it. Those are ordinary unlayered author rules, and inline beat all of them. Every such var the SDK also defaulted was silently replaced by the SDK's value, permanently, with no self-heal path.A host sending its brand accent that way rendered in our neutral blue.
The fix
The defaults install as
@layer synapse-defaultsinstead — one managed<style>element, replaced in place, so a mode flip swaps the whole map atomically and repeat calls are cheap. Unlayered rules outrank layered ones, so::root:rootat equal specificity — see belowThe last row is the point of the map and is unchanged. The middle rows are new: overriding a token from an app previously needed
!important, or was impossible.The host-stylesheet and app-
:rootcases are deliberately one row rather than a ranked pair. Nothing in the cascade separates them, so document order decides — and against the NimbleBrain runtime the order is fixed, becauseinjectThemeStyles(NimbleBrainInc/nimblebrain,web/src/bridge/iframe.ts:170) injects the host theme block by replacing the<head>opening tag, putting it ahead of the app's bundled styles. So the app wins. A host token delivered by stylesheet is the app's to override; one delivered over the protocol is not.docs/concepts/theming.mdxis rewritten to say exactly that. It previously listed the two as a ranked ladder in the host's favour — the mirror image of the bug this PR fixes, documented as impossible, on the one page an app author reads to reason about this. Its siblingdocs/components/tokens.mdxcarried the same pre-0.10.2 fallback claim and is corrected alongside it.Note that the injection order is a fact about one host, not a guarantee this repo can make, and nothing currently pins it in either repo — see Downstream.
Removal is keyed off what the SDK wrote
A layer cannot mask a stale inline pin the way inline rewrites did, so a key the host stops sending has to be actively removed or it stays pinned forever, where the layer, the host's stylesheet and the app's rules all cannot reach it. That removal iterates a module-level set of the keys last written inline, not
DEFAULT_THEME_VARS.They are different sets. A host may send any spec-enum var, and roughly 25 theme-sensitive ones (
--color-text-danger,--color-background-inverse,--color-ring-warning, …) have no neutral default — keying off the default map left exactly those pinned at the previous mode's value while the layer flipped, which is the half-light/half-dark state the removal exists to prevent, reachable by a different door. Tracking what was written also means the SDK never removes an inline property it did not set, so an app drivingdocumentElement.styleitself is left alone.Verification
npm run cigreen — 36 files, 496 tests. The docs site builds with all internal links valid.Ten tests in
src/__tests__/theme-default-layer.test.tsassert the property rather than the mechanism, so they hold as the map grows. Mutation-checked, both ways:DEFAULT_THEME_VARSagain → 2 of 10 fail (the two that cover the case above)The file says out loud (lines 19–26) that happy-dom does not compute cascade layers, so a green suite is not evidence of the cascade outcome — the tests pin the mechanism a future edit could silently reverse, and the browser settles the outcome.
Confirmed in Chrome 148 against a real larger-than-enum host — its full token set as an injected
:rootrule, plus the spec-filtered subset over the protocol:--color-text-accent#0055FF#2563eb❌#0055FF✅--nb-color-danger#c02a24#dc2626❌#c02a24✅--nb-color-success#0f7a4f#059669❌#0f7a4f✅--nb-color-warning#8a5f0a#f59e0b❌#8a5f0a✅--color-text-primary#09090b#09090b✅#09090b✅--font-sansAnd the controls, so the fix isn't just "defaults stopped applying":
--color-text-accentis unset before, then resolves to#2563ebin light and#818cf8in dark. The fallback still works, in both modes.:root { --color-text-accent: #ff00ff }now wins.<head>(asinjectThemeStylesplaces it) and the app's<style>after, the app's value is what computes — the finding the doc rewrite records.@layer appbefore the SDK inserts its sheet still wins (#ff6600). The SDK prepends at runtime, after the app's layer names are already registered, and the browser re-sorts layer order on insertion — sosynapse-defaultssorts first and the app's layer after it. This is the one claim the docs make that the cascade does not make for us, so it is now measured rather than reasoned.Downstream — this needs a host-side counterpart before it reaches users
NimbleBrainInc/nimblebrain#804 holds a red guard (
web/src/__tests__/host-token-contract.test.ts) pinning exactly this contract, and this is the fix it is waiting for. But shipping the SDK half alone would regress that host.The host delivers its non-spec tokens —
--color-text-accentand the--nb-color-*set — as a:rootblock baked into the iframe'ssrcdocat mount (buildThemeStyleBlock(mode),injectThemeStylesatiframe.ts:259). That block is mode-specific and frozen for the frame's life: the sandbox deliberately withholdsallow-same-origin, so the host cannot reach back in, andSlotRendererkeeps iframes mounted across a theme change on purpose (placementKeyisresourceUrionly). Theme changes arrive overhost-context-changed, spec-filtered — so those keys never travel that path.Eight keys are in this SDK's default map, emitted by the host's block, and off the protocol:
--color-text-accent,--nb-color-{accent-foreground,danger,success,warning,processing,processing-light,info-light}. Today the SDK's inline defaults mask the staleness by overriding the host's block entirely — which is bug #49. Move them into a layer and the frozen block wins, so after a light→dark toggle they hold the light palette on a dark page. Reproduced on Chrome 148: background flips to#000000and text to#fafafa, while--color-text-accentstays pinned at#0055FF— 3.74:1, under AA, where main gives#818cf8at 7.04:1.#804's guard cannot catch it: it reads the inline style attribute, which is precisely what this PR stops writing.Two bugs, one masking the other. This PR fixes one and exposes the other; the exposed one is host-side.
The root fix belongs in the host, and the shape is already established here. A token that varies with mode has to travel on a channel that can be updated when the mode varies.
srcdocis write-once. Thesynapse/*host-context extension is not —synapse/fontFaces(0.13.0) already carries non-spec data over it,filterHostContextForSpecalready exists to keep such keys off the spec field, and they would land inline like every other host token and track the mode for free. That retires the frozen block as a token channel rather than working around it.Sequencing. The exact pin in
#804is0.13.0, so merging and publishing0.14.0reaches no one — but it also means#804's guard stays red until it moves to0.14.0. That bump is the gate, and it should not land before the host carries these tokens on an updatable channel. Tracked as NimbleBrainInc/nimblebrain#817, which carries the reproduction and the proposed channel fix.Also in this diff
Restores the CHANGELOG's missing
## [0.9.0]and## [0.8.0]headings (#48). Their absence filed both releases' entries — including a### Breaking— under## [0.10.0], which misdates theFileResult.base64Data→FileResult.idbreak by two minor versions and cost a downstream consumer a round of tag-diffing to disprove. Two lines, in a file this change already edits, and separable if you'd rather it went alone.Version
0.14.0, not a patch: the change adds a capability the docs now advertise (an app overrides an SDK default with a plain:rootrule) and moves cascade precedence for every existing consumer that declares one of these vars. The Python IIFE is re-vendored from this tree's build with__client_version__moved to match, which CI's freshness diff and pin gate both require.