poc: CSS Modules using shadowrootadoptedstylesheets for the Lit SSR pipeline#3012
Open
zeroedin wants to merge 23 commits into
Open
poc: CSS Modules using shadowrootadoptedstylesheets for the Lit SSR pipeline#3012zeroedin wants to merge 23 commits into
shadowrootadoptedstylesheets for the Lit SSR pipeline#3012zeroedin wants to merge 23 commits into
Conversation
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one. |
|
✅ Deploy Preview for red-hat-design-system ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
|
Size Change: -21.9 kB (-6.96%) ✅ Total Size: 292 kB 📦 View Changed
ℹ️ View Unchanged
|
Member
|
@zeroedin really cool! please link to the proposal explainer and to browser standards positions in the description |
Contributor
Documentation Health
|
| Category | Score | |
|---|---|---|
| Element description | 8/25 | ❌ |
| Attribute documentation | 20/20 | ✅ |
| Slot documentation | 15/15 | ✅ |
| CSS documentation | 15/15 | ✅ |
| Event documentation | 15/15 | ✅ |
| Demos | 0/0 | ❌ |
colorPalettes — 65/90 ⚠️
| Category | Score | |
|---|---|---|
| Element description | 0/25 | ❌ |
| Attribute documentation | 20/20 | ✅ |
| Slot documentation | 15/15 | ✅ |
| CSS documentation | 15/15 | ✅ |
| Event documentation | 15/15 | ✅ |
| Demos | 0/0 | ❌ |
colorPalettes — 65/90 ⚠️
| Category | Score | |
|---|---|---|
| Element description | 0/25 | ❌ |
| Attribute documentation | 20/20 | ✅ |
| Slot documentation | 15/15 | ✅ |
| CSS documentation | 15/15 | ✅ |
| Event documentation | 15/15 | ✅ |
| Demos | 0/0 | ❌ |
PaletteController — 65/90 ⚠️
| Category | Score | |
|---|---|---|
| Element description | 0/25 | ❌ |
| Attribute documentation | 20/20 | ✅ |
| Slot documentation | 15/15 | ✅ |
| CSS documentation | 15/15 | ✅ |
| Event documentation | 15/15 | ✅ |
| Demos | 0/0 | ❌ |
Recommendations:
- PaletteController: use RFC 2119 keywords (MUST, SHOULD, AVOID) to clarify requirements (Element description, +5 pts)
- colorPalettes: add a description to explain what this declaration does (Element description, +5 pts)
- colorPalettes: use RFC 2119 keywords (MUST, SHOULD, AVOID) to clarify requirements (Element description, +5 pts)
- colorPalettes: describe purpose and context using words like 'for', 'when', 'provides', 'allows' (Element description, +5 pts)
- colorPalettes: add a description to explain what this declaration does (Element description, +5 pts)
Member
|
wdyt about outputting each style module in body, just prior to its first use? that might improve streaming performance |
Collaborator
Author
that's an interesting thought. |
zeroedin
changed the title
shadowrootadoptedstylesheets for the Lit SSR pipelineshadowrootadoptedstylesheets for the Lit SSR pipeline
|
Just a heads up in case you don't know this. I noticed that on the home page (both production and preview) that the dsd templates include a significant amount of custom comments: |
Member
|
/agentic_review |
bennypowers
reviewed
May 11, 2026
Collaborator
Author
|
What changed:
|
…compiled js for SSR
Collaborator
Author
|
Update on latest commit: When the SSR pipeline switched from importing But the TypeScript compiler's The fix replaces marker-based identification with object identity: a |
Member
|
/agentic_review |
Code Review by Qodo
1. Polyfill throws on unsupported APIs
|
![qodo-for-redhat-ux[bot]](https://avatars.githubusercontent.com/in/3663350?s=60&v=4){kind=small}
Comment on lines
+1
to
+33
| (function() { | ||
| if ('shadowRootAdoptedStyleSheets' in HTMLTemplateElement.prototype) { | ||
| return; | ||
| } | ||
| const SHEET_ATTR = 'shadowRootAdoptedStyleSheets'; | ||
| const HOST_SEL = `[${SHEET_ATTR}]:not([${SHEET_ATTR}=""])`; | ||
| const sheets = new Map(); | ||
| const applied = new WeakSet(); | ||
| function collectStyles() { | ||
| document.querySelectorAll( | ||
| 'style[type="module"][specifier]:not([specifier=""])' | ||
| ).forEach(function(el) { | ||
| const spec = el.getAttribute('specifier').trim(); | ||
| if (!sheets.has(spec)) { | ||
| const s = new CSSStyleSheet(); | ||
| s.replaceSync(el.textContent); | ||
| sheets.set(spec, s); | ||
| } | ||
| }); | ||
| } | ||
| function applyToHost(el) { | ||
| if (applied.has(el) || !el.shadowRoot) { | ||
| return; | ||
| } | ||
| applied.add(el); | ||
| el.shadowRoot.adoptedStyleSheets.push( | ||
| ...el.getAttribute(SHEET_ATTR).trim().split(/\s+/) | ||
| .flatMap(function(n) { | ||
| return sheets.has(n) ? [sheets.get(n)] : []; | ||
| }) | ||
| ); | ||
| el.shadowRoot.querySelectorAll(HOST_SEL).forEach(applyToHost); | ||
| } |
There was a problem hiding this comment.
1. Polyfill throws on unsupported apis 🐞 Bug ☼ Reliability
shadowroot-adopted-stylesheets.js unconditionally uses new CSSStyleSheet(), replaceSync(), and shadowRoot.adoptedStyleSheets.push(...) without feature-detecting availability, which can throw and break page rendering on browsers without constructable/adopted stylesheet support.
Agent Prompt
## Issue description
`docs/assets/javascript/shadowroot-adopted-stylesheets.js` assumes constructable stylesheets and `adoptedStyleSheets` exist, and will throw when they do not. Because this script is loaded on all pages, a single runtime exception can break SSR/hydration and page scripts.
## Issue Context
- The script constructs `CSSStyleSheet` and calls `replaceSync`, then mutates `shadowRoot.adoptedStyleSheets`.
- It’s included globally in the base layout.
## Fix Focus Areas
- Add explicit feature detection for **both** constructable stylesheet creation (`CSSStyleSheet` + `replaceSync`) and adoption (`ShadowRoot.prototype.adoptedStyleSheets`).
- Provide a safe fallback path when unsupported (e.g., clone `<style type="module" specifier>` contents into each shadowRoot as `<style data-specifier="...">...</style>`), or early-return without throwing.
- Wrap sheet creation/adoption in try/catch to avoid taking down the page.
### Fix Focus Areas (code pointers)
- docs/assets/javascript/shadowroot-adopted-stylesheets.js[1-33]
- docs/_includes/layouts/base.njk[11-28]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+20
to
+23
| const source = typeof result.source === 'string' ? | ||
| result.source | ||
| : result.source?.toString(); | ||
| if (!source || !source.includes('html`')) { |
There was a problem hiding this comment.
2. Arraybuffer source corrupted 🐞 Bug ≡ Correctness
The lit-html-minifier-node loader converts ArrayBuffer module sources via .toString(), which does not decode the buffer contents; if nextLoad() returns an ArrayBuffer, minification will operate on garbage text and can break SSR module loading.
Agent Prompt
## Issue description
The ESM loader hook in `docs/_plugins/lit-ssr/lit-html-minifier-node.ts` calls `result.source?.toString()` when `source` is an `ArrayBuffer`. This does not decode bytes into the JavaScript source text and can corrupt the module source passed into `minifyHTMLLiterals()`.
## Issue Context
`HookContext` explicitly allows `source: string | ArrayBuffer`, so this branch is a supported code path.
## Fix Focus Areas
- Convert `ArrayBuffer` to a string with `Buffer.from(arrayBuffer).toString('utf8')` (or `new TextDecoder('utf-8').decode(arrayBuffer)`), not `.toString()`.
- Preserve behavior for string sources.
### Fix Focus Areas (code pointers)
- docs/_plugins/lit-ssr/lit-html-minifier-node.ts[4-34]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+86
to
+94
| case 'elements': { | ||
| const jsPath = join(cwd, `${path}.js`); | ||
| const body = await readFile(jsPath, 'utf8'); | ||
| return { | ||
| body, | ||
| status: 200, | ||
| headers: { 'Content-Type': 'text/javascript' }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
3. Dev js handler can throw 🐞 Bug ☼ Reliability
The Eleventy dev server handler for @rhds/elements now reads ${path}.js directly without error
handling; if the compiled JS is missing or stale, readFile() will reject and can crash the request
handler.
Agent Prompt
## Issue description
In `docs/_plugins/typescript-assets.ts`, the `@rhds/elements` JS route reads a compiled `.js` file with no try/catch. Any ENOENT/permission/read error will propagate and can break the dev server response path.
## Issue Context
The file already contains `transformTypescriptSource()` with explicit error handling and a structured 500 response.
## Fix Focus Areas
- Wrap the `readFile(jsPath, 'utf8')` call in try/catch.
- Return a 404 when the file does not exist (ENOENT), and/or fall back to `transformTypescriptSource(join(cwd, `${path}.ts`))` when the `.js` is missing.
- Keep response headers consistent.
### Fix Focus Areas (code pointers)
- docs/_plugins/typescript-assets.ts[16-48]
- docs/_plugins/typescript-assets.ts[80-102]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
3 participants
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.
PROOF OF CONCEPT
Summary
Implements
shadowrootadoptedstylesheetsfor the Lit SSR pipeline. Instead of inlining duplicate<style>blocks inside every component's declarative shadow DOM<template>, shared styles are defined once per page as<style type="module" specifier="name">blocks and referenced viashadowrootadoptedstylesheetsattributes on host elements and templates. Template literal whitespace and comments are also stripped at build time via@literals/html-css-minifier.How it works
Marker injection (
lit-css-node.ts) — The CSS loader prepends/* @sheet:specifier */markers to CSS content during SSR, derived from the CSS filename (e.g.,rh-button.css→@sheet:rh-button). The marker survives the@pwrs/lit-csstransform and becomes part ofCSSResult.cssText.Renderer extraction (
worker.ts) — A customLitElementRendereroverride extracts@sheet:markers fromelementStylesinconnectedCallback(), emitsshadowrootadoptedstylesheetsattributes on host elements and template tags, and processes CSS through lightningcss. After rendering, a post-processing step collects all unique stylesheets into shared<style type="module">blocks inserted before</head>.Template minification (
lit-html-minifier-node.ts) — An ESM loader hook intercepts element module imports and runs@literals/html-css-minifieron the compiled source, stripping whitespace and comments fromhtmlandcsstagged template literals before SSR renders them.Polyfill (
shadowroot-adopted-stylesheets.js) — Since no browser natively supportsshadowrootadoptedstylesheetsyet, a MutationObserver-based polyfill in<head>applies styles as elements appear during HTML parsing. It collects<style type="module" specifier="...">blocks intoCSSStyleSheetobjects and applies them to shadow roots viaadoptedStyleSheets. Because MutationObserver callbacks run as microtasks before the browser paints, styles are applied before each frame — preventing FOUC on Safari and Firefox.Color palette marker (
lib/color-palettes.ts) — ThecolorPalettesdecorator injects a@sheet:rhds-color-palettemarker into its CSS so the SSR renderer can extract and deduplicate it alongside component stylesheets. This is inert when SSR is not available.Based on work by @sorvell https://lit.dev/playground/#gist=0a095178701ec2c5426eddb1a37545ea
Related links
adoptedstylesheetsattribute mozilla/standards-positions#1081adoptedstylesheetsattribute WebKit/standards-positions#407shadowrootadoptedstylesheetsPerformance ReportComparison of production (ux.redhat.com) vs deploy preview (deploy-preview-3012) with
shadowrootadoptedstylesheetsCSS deduplication and template literal minification.HTML Payload Size
Lighthouse Performance (Mobile Emulation)
Single-run scores. Results vary between runs.
Key Findings
HTML Payload
<style>tags dropped from hundreds (or thousands) to 1 per pageLighthouse
Caveats
Test plan
npm run docsbuilds successfully<style type="module" specifier="...">blocks appear once before</head><style>inside<template>tags for components with specifiersshadowrootadoptedstylesheetsattributes present on host elements and<template>tagsadoptedStyleSheetsbase.njkanddemo.html)Testing Instructions
Notes to Reviewers