From 07835b7b567ef4c3af0a285e5b2d0acf327a2538 Mon Sep 17 00:00:00 2001 From: ekklesia Date: Thu, 27 Aug 2026 07:21:14 -0500 Subject: [PATCH] feat: give module UI real stylesheets, and every artifact its own root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0023 refused stylesheet imports for a good reason: esbuild would emit a `.css` sidecar beside the artifact, nothing would declare it, no host would fetch it, and it would work in a local test and silently do nothing in a real host. Inline style objects were the whole story. They cannot express `:hover`, `:focus-visible`, `:disabled`, `@media`, or `@keyframes` — so an accessible focus treatment was not expressible, and the shared elements added in the previous commit are built from real CSS for exactly that reason. Compile the stylesheet into the artifact instead of beside it. A module imports plain `.css`; the canonical builder resolves local `@import`s through a nested bundle and turns each file into its compiled text, which the author declares as `defineModuleUi({ styles })`. One declared file still contains everything: nothing extra to publish, nothing to fetch, and no way for CSS to become an asset nobody serves. The build additionally fails if a CSS output ever appears beside an artifact, so a future loader change cannot bring the sidecar back. Two rules are enforced with their own codes: a module stylesheet may reference `--ekklesia-*` and never declare one, and every `@import` must resolve locally or the artifact would fetch styles at run time. Plain `.css` only; other dialects are a toolchain decision needing their own ADR. Then make the isolation real. The generic loader mounts every browser artifact — primary, display, and later a contributed renderer — inside an open Shadow DOM root it creates, installs the artifact's stylesheet there, and hands the artifact an element inside it. Shadow DOM is the one mechanism a browser actually enforces; a class prefix or a "scope your selectors" instruction is a convention one mistake defeats, and the failure mode is a module silently restyling the Dashboard. It applies unconditionally, including to artifacts with no stylesheet, because a conditional boundary is one a module can fall out of by editing a source file. Both wrappers are `display: contents` through base rules installed before the artifact's own, so layout is unchanged and a full-viewport display can still override `:host`. Unmount removes the whole root in a `finally`, so a cleanup that throws cannot leave styles behind, and remounting cannot accumulate a second copy. This is a correctness boundary for styles and is not described as a sandbox: the root is `open` precisely because module code is trusted, same-realm code that could reach `document` anyway. An artifact's DOM is no longer a direct child of the host's element, so the mount ABI is versioned explicitly. `defineModuleUi` stamps `MODULE_UI_ABI_VERSION` so it can never drift from the SDK the artifact compiled against, and a version-1 artifact fails with its own code — "rebuild your module", not a typo hunt. No dual acceptance, matching every manifest cutover here. Migration is one `pnpm build`. `pnpm boundaries` allows `.css` only under `src/ui/` and `src/display/`: a stylesheet anywhere else in a module is the undeclared sidecar again, just relocated. Co-Authored-By: Claude Opus 5 --- CONTRIBUTING.md | 12 +- apps/api/package.json | 2 + apps/api/tests/module-ui-host-e2e.test.ts | 57 ++++- apps/api/tests/module-ui-routes.test.ts | 40 +++- .../0023-module-ui-build-pipeline.md | 9 + ...dule-stylesheets-and-artifact-isolation.md | 166 +++++++++++++ docs/architecture/decisions/README.md | 1 + docs/architecture/repository-boundaries.md | 14 +- docs/module-development/README.md | 112 +++++++-- .../module-ui-build/src/build-module-ui.ts | 125 ++++++++-- .../tests/build-module-ui.test.ts | 120 ++++++++-- packages/module-ui-host/src/index.ts | 1 + packages/module-ui-host/src/isolated-root.ts | 108 +++++++++ packages/module-ui-host/src/load-module-ui.ts | 55 ++++- .../artifact-module/module-styles.d.ts | 4 + .../fixtures/artifact-module/src/ui/index.tsx | 6 +- .../artifact-module/src/ui/styles.css | 5 + .../tests/load-module-ui.test.ts | 221 ++++++++++++++++-- .../tests/real-artifact.test.ts | 43 +++- .../module-ui-sdk/src/define-module-ui.ts | 20 +- packages/module-ui-sdk/src/module-ui.ts | 75 +++++- .../tests/module-ui-contract.test.ts | 76 +++++- pnpm-lock.yaml | 12 + scripts/check-boundaries.ts | 47 +++- surfaces/shell/tests/display-app.test.tsx | 39 ++-- surfaces/shell/tests/host-app.test.tsx | 47 ++-- surfaces/shell/tests/support/isolated-dom.ts | 55 +++++ templates/module/package.json | 2 + templates/module/src/display/index.tsx | 25 +- templates/module/src/display/styles.css | 36 +++ templates/module/src/module-styles.d.ts | 17 ++ templates/module/src/ui/ExampleApp.tsx | 29 ++- templates/module/src/ui/index.tsx | 9 + templates/module/src/ui/styles.css | 31 +++ templates/module/tests/module-ui.test.ts | 22 +- templates/module/tsconfig.ui.json | 8 +- tests/boundaries.test.ts | 9 +- .../modules/calendar/src/ui/styles.css | 4 + 38 files changed, 1460 insertions(+), 204 deletions(-) create mode 100644 docs/architecture/decisions/0040-module-stylesheets-and-artifact-isolation.md create mode 100644 packages/module-ui-host/src/isolated-root.ts create mode 100644 packages/module-ui-host/tests/fixtures/artifact-module/module-styles.d.ts create mode 100644 packages/module-ui-host/tests/fixtures/artifact-module/src/ui/styles.css create mode 100644 surfaces/shell/tests/support/isolated-dom.ts create mode 100644 templates/module/src/display/styles.css create mode 100644 templates/module/src/module-styles.d.ts create mode 100644 templates/module/src/ui/styles.css create mode 100644 tests/fixtures/boundaries/module-has-ui/modules/calendar/src/ui/styles.css diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8aa8986..12c939e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -35,13 +35,15 @@ pnpm infrastructure:config private source paths. - Use `workspace:` ranges for internal package dependencies. - Modules own their own UI and author it in React and TypeScript (ADR 0022). A module package may - contain `.tsx` and depend on `react`, `react-dom`, and `@ekklesia/module-ui-sdk`. Every other UI - framework, `.jsx`, stylesheets, and other presentation formats remain rejected by - `pnpm boundaries`. Primary and display sources live at the exact `src/ui/index.tsx` and + contain `.tsx` and depend on `react`, `react-dom`, and `@ekklesia/module-ui-sdk`, and may author + plain `.css` inside a browser-artifact source root (ADR 0040). Every other UI framework, `.jsx`, + other stylesheet dialects, and other presentation formats remain rejected by `pnpm boundaries`. Primary and display sources live at the exact `src/ui/index.tsx` and `src/display/index.tsx` conventions; one `pnpm build` invocation atomically compiles every role the v6 manifest declares via `@ekklesia/module-ui-build` (ADR 0023/0033). Module authors write no - bundler configuration, and styling is React inline style objects only for now. Both roles use the - same UI SDK context with no presentation-mode flag. + bundler configuration. A module's stylesheets are compiled into its declared artifact as text and + installed by the host into that artifact's own Shadow DOM root, so module styles cannot leak into + the Dashboard or another module (ADR 0040). Both roles use the same UI SDK context with no + presentation-mode flag, and the same versioned mount ABI. - A module may also consume exactly two shared design packages — `@ekklesia/design-tokens` and `@ekklesia/ui` (ADR 0039) — so two modules' screens look like one product. That is a named allowlist: every other shared role (`api-client`, `auth-client`, `surface-sdk`, `module-ui-host`) diff --git a/apps/api/package.json b/apps/api/package.json index 2932788..5b50008 100644 --- a/apps/api/package.json +++ b/apps/api/package.json @@ -28,11 +28,13 @@ "devDependencies": { "@biomejs/biome": "2.5.3", "@ekklesia/module-ui-host": "workspace:*", + "@ekklesia/module-ui-sdk": "workspace:*", "@ekklesia/typescript-config": "workspace:*", "@types/node": "24.13.3", "@types/pg": "8.20.0", "@types/smtp-server": "3.5.13", "better-auth": "1.6.23", + "happy-dom": "20.0.11", "pg": "8.22.0", "smtp-server": "3.19.3", "typescript": "7.0.2", diff --git a/apps/api/tests/module-ui-host-e2e.test.ts b/apps/api/tests/module-ui-host-e2e.test.ts index cc857b4..2fdb391 100644 --- a/apps/api/tests/module-ui-host-e2e.test.ts +++ b/apps/api/tests/module-ui-host-e2e.test.ts @@ -20,7 +20,13 @@ import { discoverModules, startDiscoveredModuleRuntime, } from "@ekklesia/core/modules"; -import { loadModuleUi } from "@ekklesia/module-ui-host"; +import { + loadModuleUi, + MODULE_UI_ISOLATION_ATTRIBUTE, + MODULE_UI_ISOLATION_ROOT_ATTRIBUTE, +} from "@ekklesia/module-ui-host"; +import { MODULE_UI_ABI_VERSION } from "@ekklesia/module-ui-sdk"; +import { Window } from "happy-dom"; import { bootstrapDisplayHost, bootstrapHost, @@ -165,6 +171,7 @@ async function createFixtureModule( await writeFile( path.join(moduleDirectory, "dist/ui/index.js"), `export default { + abi: ${MODULE_UI_ABI_VERSION}, id: ${JSON.stringify(id)}, version: "0.1.0", async mount(container, context) { @@ -184,6 +191,7 @@ async function createFixtureModule( await writeFile( path.join(moduleDirectory, "dist/display/index.js"), `export default { + abi: ${MODULE_UI_ABI_VERSION}, id: ${JSON.stringify(id)}, version: "0.1.0", async mount(container, context) { @@ -310,10 +318,35 @@ async function publishedArtifactUrl(harness: Harness, sourceUrl: string): Promis return `data:text/javascript;base64,${bytes.toString("base64")}`; } -/** The loader hands the container straight to the module and never inspects it, so a plain sink is - * enough and no DOM implementation is needed in this Node-environment suite. */ -function containerSink(): { textContent: string } { - return { textContent: "" }; +/** + * A real host-owned container, built from `happy-dom` **as a library** rather than as this suite's + * test environment. + * + * Since ADR 0040 the generic loader builds an isolated Shadow DOM root inside whatever container it + * is given, so a plain object sink is no longer a faithful stand-in — and the isolation is exactly + * what an end-to-end proof should exercise rather than stub out. Installing happy-dom as the + * *environment* is not an option here: it replaces the module runner's resolution, and this suite + * dynamically imports real module server entrypoints from temporary directories, which then fail to + * resolve. Constructing one `Window` gives the DOM these few assertions need and leaves every global + * this server-side suite depends on exactly as Node provides it. + */ +function containerSink(): HostContainer { + return new Window().document.createElement("div") as unknown as HostContainer; +} + +interface HostContainer { + readonly children: { readonly length: number }; + querySelector(selectors: string): HostContainer | null; + readonly shadowRoot: { querySelector(selectors: string): { textContent: string } | null } | null; +} + +/** What the mounted artifact rendered, read through its open isolated root. */ +function mountedText(container: HostContainer): string { + const shadowHost = container.querySelector(`[${MODULE_UI_ISOLATION_ATTRIBUTE}]`); + return ( + shadowHost?.shadowRoot?.querySelector(`[${MODULE_UI_ISOLATION_ROOT_ATTRIBUTE}]`)?.textContent ?? + "" + ); } type LoaderContainer = Parameters[0]["container"]; @@ -365,13 +398,13 @@ describe("end to end: a discovered module's UI mounts in the Ekklesia host", () }); // 5. The module rendered, having reached its own backend through the host. - expect(container.textContent).toBe("example:200:2"); + expect(mountedText(container)).toBe("example:200:2"); // 6. It asked for "records"; the host resolved that onto Ekklesia's module mount prefix. expect(harness.moduleApiCalls).toEqual(["GET /api/v1/modules/example/records"]); // 7. And it tears down cleanly. await mounted.unmount(); - expect(container.textContent).toBe(""); + expect(mountedText(container)).toBe(""); expect(mounted.mounted).toBe(false); } finally { await harness.stop(); @@ -443,10 +476,10 @@ describe("end to end: a discovered module's UI mounts in the Ekklesia host", () }, }); - expect(container.textContent).toBe("example:display:200:2"); + expect(mountedText(container)).toBe("example:display:200:2"); expect(harness.moduleApiCalls).toEqual(["GET /api/v1/modules/example/records"]); await mounted.unmount(); - expect(container.textContent).toBe(""); + expect(mountedText(container)).toBe(""); expect(mounted.mounted).toBe(false); } finally { await harness.stop(); @@ -514,7 +547,7 @@ describe("end to end: a discovered module's UI mounts in the Ekklesia host", () }); // Hiding a control is a usability improvement: the module simply did not ask. - expect(container.textContent).toBe("example:denied"); + expect(mountedText(container)).toBe("example:denied"); expect(harness.moduleApiCalls).toEqual([]); } finally { await harness.stop(); @@ -593,7 +626,7 @@ describe("end to end: a discovered module's UI mounts in the Ekklesia host", () version: groupedEntry.descriptor.version, }, }); - expect(container.textContent).toBe("example:200:2"); + expect(mountedText(container)).toBe("example:200:2"); await mounted.unmount(); // Now move it somewhere else entirely. @@ -645,7 +678,7 @@ describe("end to end: a discovered module's UI mounts in the Ekklesia host", () version: movedEntry.descriptor.version, }, }); - expect(movedContainer.textContent).toBe("example:200:2"); + expect(mountedText(movedContainer)).toBe("example:200:2"); expect(harness.moduleApiCalls.slice(callsBeforeRemount)).toEqual([ "GET /api/v1/modules/example/records", ]); diff --git a/apps/api/tests/module-ui-routes.test.ts b/apps/api/tests/module-ui-routes.test.ts index a59dd9a..7b521dc 100644 --- a/apps/api/tests/module-ui-routes.test.ts +++ b/apps/api/tests/module-ui-routes.test.ts @@ -16,6 +16,8 @@ import { EMPTY_MODULE_REALTIME_REGISTRY, } from "@ekklesia/core/modules"; import { loadModuleUi } from "@ekklesia/module-ui-host"; +import { MODULE_UI_ABI_VERSION } from "@ekklesia/module-ui-sdk"; +import { Window } from "happy-dom"; import type { Hono } from "hono"; import { afterEach, describe, expect, it, vi } from "vitest"; import { createInMemoryDashboardStore } from "./support/in-memory-dashboard-store.js"; @@ -59,6 +61,7 @@ async function createModulesDirectory(): Promise { * sequence whose exact publication and loader-compatibility can be asserted. */ function artifactSource(id: string, version: string): string { return `export default { + abi: ${MODULE_UI_ABI_VERSION}, id: ${JSON.stringify(id)}, version: ${JSON.stringify(version)}, mount(container, context) { @@ -862,6 +865,12 @@ describe("module UI artifact URL generation", () => { }); }); +interface HostContainer { + readonly children: { readonly length: number }; + querySelector(selectors: string): HostContainer | null; + readonly shadowRoot: { querySelector(selectors: string): { textContent: string } | null } | null; +} + describe("module UI artifact publication — the generic loader consumes what is published", () => { /** * Node cannot `import()` an `http:` URL, so the two halves of the chain are proven separately and @@ -880,16 +889,29 @@ describe("module UI artifact publication — the generic loader consumes what is } /** - * The loader forwards the container untouched to the module's own `mount` and never inspects it, - * so a plain sink is enough and no DOM implementation is required. The container's type is read - * back off `loadModuleUi` itself rather than written as `Element`, because `apps/api` compiles - * against Node libs only — naming a DOM type here would be the same browser/server confusion this - * route exists to keep apart. + * A real host-owned container, because since ADR 0040 the loader builds an isolated Shadow DOM + * root inside whatever it is given. The container's type is read back off `loadModuleUi` itself + * rather than written as `Element`, because `apps/api` compiles against Node libs only — naming a + * DOM type here would be the same browser/server confusion this route exists to keep apart. */ type LoaderContainer = Parameters[0]["container"]; - function containerSink(): { textContent: string } { - return { textContent: "" }; + /** + * `happy-dom` used as a library rather than as this suite's environment: the loader now builds a + * real isolated root inside the container, and installing a DOM environment here would replace + * the module runner's resolution for a server-side suite that dynamically imports real module + * entrypoints. + */ + function containerSink(): HostContainer { + return new Window().document.createElement("div") as unknown as HostContainer; + } + + /** What the mounted artifact rendered, read through its open isolated root. */ + function mountedText(container: HostContainer): string { + const shadowHost = container.querySelector("[data-ekklesia-module-ui]"); + return ( + shadowHost?.shadowRoot?.querySelector("[data-ekklesia-module-ui-root]")?.textContent ?? "" + ); } function hostContext() { @@ -915,9 +937,9 @@ describe("module UI artifact publication — the generic loader consumes what is source: { moduleId: "announcements", url, version: "1.4.2" }, }); - expect(container.textContent).toBe("announcements:mounted"); + expect(mountedText(container)).toBe("announcements:mounted"); await mounted.unmount(); - expect(container.textContent).toBe(""); + expect(container.children).toHaveLength(0); }); it("still rejects the published artifact when the host expects a different identity", async () => { diff --git a/docs/architecture/decisions/0023-module-ui-build-pipeline.md b/docs/architecture/decisions/0023-module-ui-build-pipeline.md index 1cff059..7bc3939 100644 --- a/docs/architecture/decisions/0023-module-ui-build-pipeline.md +++ b/docs/architecture/decisions/0023-module-ui-build-pipeline.md @@ -100,6 +100,15 @@ This is deliberately the smallest viable styling story. A real stylesheet contra field, a host loading mechanism, and a style-isolation decision, none of which exist — and inventing them here would expand the manifest and runtime contract before a host can validate them. +> **Correction — this rule is superseded.** +> [ADR 0040](0040-module-stylesheets-and-artifact-isolation.md) supplies the three missing pieces and +> reverses this one decision: a module may import plain `.css` from a browser-artifact source root, +> the canonical builder compiles it **into** the declared artifact as text (so no sidecar is ever +> emitted, which is what this section was protecting), and the host installs it into a Shadow DOM +> root it creates for every artifact. No manifest field was needed after all. The rest of this ADR — +> esbuild, bundled React, fixed source paths, no author bundler configuration, unminified output — is +> unchanged. + ### Browser TypeScript stays scoped to UI source `@ekklesia/typescript-config/browser-react.json` extends the existing `browser.json` and sets diff --git a/docs/architecture/decisions/0040-module-stylesheets-and-artifact-isolation.md b/docs/architecture/decisions/0040-module-stylesheets-and-artifact-isolation.md new file mode 100644 index 0000000..3722420 --- /dev/null +++ b/docs/architecture/decisions/0040-module-stylesheets-and-artifact-isolation.md @@ -0,0 +1,166 @@ +# 0040: Module stylesheets, and every browser artifact gets an isolated root + +- Status: Accepted +- Date: 2026-08-27 +- Supersedes: none +- Related: [ADR 0021](0021-modules-as-hostable-products.md), + [ADR 0022](0022-module-ui-authoring-and-host-contract.md), + [ADR 0023](0023-module-ui-build-pipeline.md), + [ADR 0024](0024-browser-safe-ui-sdk-and-host-loader.md), + [ADR 0033](0033-display-kiosk-host-contract.md), + [ADR 0039](0039-shared-design-tokens-and-elements.md) + +## Context + +ADR 0023 rejected stylesheet imports in module UI, and gave an honest reason: esbuild would emit a +`.css` sidecar beside the artifact, nothing would declare it, no host would fetch it, and it would +*work in a local test and silently do nothing in a real host*. Inline style objects were the whole +supported story, and a stylesheet contract was deferred to "a manifest field, a host loading +mechanism, and a style-isolation decision, none of which exist yet." + +Two of those three now exist or are unnecessary, and the remaining gap is blocking real work: + +- Inline style objects cannot express `:hover`, `:focus-visible`, `:disabled`, `@media`, or + `@keyframes`. A production management screen needs all of them, and an accessible focus treatment + needs `:focus-visible` specifically. This is not a preference; it is a capability inline styles do + not have. +- ADR 0039's shared elements are built from real CSS for exactly that reason, so a module using them + already needs somewhere for a stylesheet to live. +- A contributed renderer (a later ADR) makes the isolation question urgent rather than theoretical: + one module's renderer mounted inside another module's screen is precisely where a leaked selector + does damage nobody can trace. + +The manifest field turns out not to be needed, and the loading mechanism and the isolation decision +turn out to be the same decision. + +## Decision + +### A stylesheet is compiled **into** the declared artifact, as text + +A module imports its local `.css` files. `@ekklesia/module-ui-build` compiles each one — resolving +local `@import`s through a nested bundle — into a JavaScript module whose default export is that +stylesheet's text. The author passes the text to `defineModuleUi({ styles })`. + +That is why no manifest field is required. There is still exactly **one** declared file per browser +role, containing everything: nothing new to publish, nothing for a host to fetch, no second URL, and +no way for a stylesheet to become an undeclared sidecar — the failure ADR 0023 named. The builder +additionally fails if any CSS output file is ever produced beside an artifact, so a future option or +loader change cannot quietly reintroduce one. + +Two rules are enforced on the compiled text, each with its own sanitized error code: + +- **A module stylesheet may reference Ekklesia's global theme variables and must never declare one.** + Declaring `--ekklesia-*` would invent a global outside the registry a future theme editor + enumerates (ADR 0039). A private alias under the module's own prefix is the supported way to name + a local value, and is explicitly allowed. +- **An `@import` that survives compilation is rejected.** A local import is inlined and leaves + nothing behind, so anything remaining is a remote or unresolvable reference, and an artifact that + fetches a stylesheet at run time is not self-contained. + +Plain `.css` only. `.scss`, `.less`, `.styl`, and friends stay rejected: adding a preprocessor is a +language and toolchain decision needing its own ADR, not a detail of a build pass. + +`pnpm boundaries` allows `.css` inside a module **only** under the browser-artifact source roots the +canonical builder actually compiles (`src/ui/`, `src/display/`, and the contribution sources a later +ADR adds). Anywhere else in a module a stylesheet is a file no build reads and no host loads — the +undeclared sidecar again, just relocated. Every other stylesheet dialect stays rejected everywhere. + +### Every browser artifact mounts inside an isolated root + +The generic loader creates a Shadow DOM root inside the host's container, installs the artifact's +stylesheet there, and hands the artifact an element inside that root: + +```text +host container ← the host's element; the loader never mutates it beyond this one child +└─
← shadow host, created and removed by the loader + #shadow-root (open) + ├─