fix(dialtone-tokens): NO-JIRA fix shadow DOM setAttribute crash in theme setters

[Nina Repetto (ninarepetto)]

Summary

Fix a TypeError crash in all theme setter functions (setMode, setBrand, setContrast, setMaterial, initDialtoneTheme) when called with a host element that has a shadow root.

Problem

Every setter was reassigning rootNode = rootNode.shadowRoot, then calling setAttribute on the resulting ShadowRoot. ShadowRoot extends DocumentFragment → Node, not Element, so it has no setAttribute — causing a TypeError.

Additionally, even if it hadn't crashed, style tags injected into a ShadowRoot with [data-dt-mode] attributes set on the ShadowRoot itself would not match descendant selectors since ShadowRoot is not an element.

Fix

Separate style injection from attribute-setting:

// Before
if (rootNode?.shadowRoot) {
  rootNode = rootNode.shadowRoot;  // ShadowRoot — no setAttribute
}
rootNode?.setAttribute('data-dt-mode', mode);  // TypeError

// After
const styleRoot = rootNode?.shadowRoot ?? rootNode;
_setStyleTag('dialtone-css-core', ..., styleRoot);  // ShadowRoot: querySelector/appendChild ✓
rootNode?.setAttribute('data-dt-mode', mode);        // host Element: setAttribute ✓

• Style tags → injected into ShadowRoot (querySelector/appendChild work on it)
• Attributes → set on the host element (an actual Element)
• ShadowRoot-direct-pass warnings now also return early instead of crashing

[coderabbitai]

Fixes a TypeError crash in theme setter functions (setMode, setBrand, setContrast, setMaterial, initDialtoneTheme) when called with shadow-host elements. The fix separates style injection (targeting ShadowRoot) from attribute setting (targeting the host element), and returns early with warnings when ShadowRoot is passed directly instead of crashing.

Overall Judgement: ✅ Ready to merge — The fix properly addresses the core crash by separating style and attribute concerns, matches the intended behavior shown in tests, and contains no public API changes.

Walkthrough

The PR refactors theme-setting functions to cleanly separate host element attribute management from shadow-root CSS injection. A new styleRoot pattern (resolving to rootNode?.shadowRoot ?? rootNode) directs CSS injection, while attributes always target the original rootNode. Public setters and init/reset functions now warn and return early if given a ShadowRoot instance.

Changes

Theme root handling refactor

Layer / File(s)	Summary
Core theme functions packages/dialtone-tokens/themes/config.js	_setThemeLegacy and _setThemeLayered compute styleRoot and inject CSS into it while updating attributes (data-dt-contrast, data-dt-brand, data-dt-mode) on the original rootNode.
Public setter methods packages/dialtone-tokens/themes/config.js	setMode, setBrand, setContrast, and setMaterial add early-return warnings for ShadowRoot inputs and apply styleRoot for CSS injection while setting attributes on rootNode.
Initialization and reset packages/dialtone-tokens/themes/config.js	initDialtoneTheme and resetTheme implement ShadowRoot input warnings and apply styleRoot for core token/color injection and cleanup operations.
Test verification packages/dialtone-tokens/tests/config.test.js	Active test confirms initDialtoneTheme sets host attributes and injects style elements into the provided shadowRoot.

Possibly related PRs

• dialpad/dialtone#1262: Theming-system Vitest coverage that the test changes in this PR extend.

Suggested reviewers

• francisrupert
• braddialpad

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/shadow-dom-setattribute-crash

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Please add either the visual-test-ready or no-visual-test label to this PR depending on whether you want to run visual tests or not.
It is recommended to run visual tests if your PR changes any UI. ‼️

[github-actions]

✔️ Deploy previews ready!
😎 Dialtone documentation preview: https://dialtone.dialpad.com/deploy-previews/pr-1271/
😎 Dialtone-vue preview: https://dialtone.dialpad.com/vue/deploy-previews/pr-1271/

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

packages/dialtone-tokens/themes/config.js (2)

636-656: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

resetTheme() is missing the ShadowRoot guard present in all peer APIs and will throw when given a ShadowRoot.

The type ThemeRootNode includes ShadowRoot, but resetTheme() lacks the instanceof ShadowRoot check that exists in setTheme(), setMode(), setBrand(), setContrast(), setMaterial(), and initDialtoneTheme(). When passed a ShadowRoot, the call to rootNode?.removeAttribute() will fail since ShadowRoot has no removeAttribute method. The code comment even acknowledges this: "Remove theme attributes from the host element (not ShadowRoot — it has no removeAttribute)" — but the guard was not implemented.

Add the guard to match the pattern in peer functions:

Suggested fix

 export function resetTheme(rootNode = document.documentElement) {
+  if (rootNode instanceof ShadowRoot) {
+    console.warn(
+      '[Dialtone] You passed a ShadowRoot directly to resetTheme(). ' +
+      'Please pass the host element instead. The function will access shadowRoot automatically.',
+    );
+    return;
+  }
   const styleRoot = rootNode?.shadowRoot ?? rootNode;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/dialtone-tokens/themes/config.js` around lines 636 - 656, The
resetTheme function should skip calling removeAttribute when passed a ShadowRoot
just like the other APIs; update resetTheme (which uses rootNode and styleRoot)
to check "if (!(rootNode instanceof ShadowRoot))" (or equivalent) before
invoking rootNode.removeAttribute('data-dt-mode'), 'data-dt-brand',
'data-dt-contrast', and 'data-dt-material' so the method mirrors
setTheme/setMode/setBrand/setContrast/setMaterial/initDialtoneTheme and avoids
calling removeAttribute on a ShadowRoot.

---

97-113: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Add early return to setTheme() and resetTheme() when ShadowRoot is passed directly.

setTheme() warns on direct ShadowRoot input but does not return, allowing execution to reach _setThemeLegacy() or _setThemeLayered(). Both functions call setAttribute() on the passed node. Since ShadowRoot lacks setAttribute() and removeAttribute() methods (only Elements have these), the function throws TypeError at runtime.

resetTheme() has the same issue: it calls removeAttribute() directly on a ShadowRoot without guarding.

This is a public npm library API; a crashing function shipped without a BREAKING CHANGE footer violates compatibility guidelines. Use the pattern already present in setMode() which correctly returns early.

Suggested fix

 export function setTheme (theme, rootNode = document.documentElement, contrastTheme = null) {
   // Warn if someone passed shadowRoot directly instead of the host
   if (rootNode instanceof ShadowRoot) {
     console.warn(
       '[Dialtone] You passed a ShadowRoot directly to setTheme(). ' +
       'Please pass the host element instead. The function will access shadowRoot automatically.',
     );
+    return;
   }

Apply the same to resetTheme().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/dialtone-tokens/themes/config.js` around lines 97 - 113, setTheme
currently warns when passed a ShadowRoot but continues, causing
_setThemeLegacy/_setThemeLayered to call setAttribute on a ShadowRoot and throw;
add the same early-return pattern used in setMode: after detecting rootNode
instanceof ShadowRoot log the console.warn and then return immediately to avoid
further execution. Do the same fix in resetTheme so it returns early when
rootNode is a ShadowRoot (preventing removeAttribute from being called on a
ShadowRoot). Ensure you reference the existing warning message behavior and keep
calls to _setThemeLegacy and _setThemeLayered unchanged when an Element host is
provided.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@packages/dialtone-tokens/themes/config.js`:
- Around line 636-656: The resetTheme function should skip calling
removeAttribute when passed a ShadowRoot just like the other APIs; update
resetTheme (which uses rootNode and styleRoot) to check "if (!(rootNode
instanceof ShadowRoot))" (or equivalent) before invoking
rootNode.removeAttribute('data-dt-mode'), 'data-dt-brand', 'data-dt-contrast',
and 'data-dt-material' so the method mirrors
setTheme/setMode/setBrand/setContrast/setMaterial/initDialtoneTheme and avoids
calling removeAttribute on a ShadowRoot.
- Around line 97-113: setTheme currently warns when passed a ShadowRoot but
continues, causing _setThemeLegacy/_setThemeLayered to call setAttribute on a
ShadowRoot and throw; add the same early-return pattern used in setMode: after
detecting rootNode instanceof ShadowRoot log the console.warn and then return
immediately to avoid further execution. Do the same fix in resetTheme so it
returns early when rootNode is a ShadowRoot (preventing removeAttribute from
being called on a ShadowRoot). Ensure you reference the existing warning message
behavior and keep calls to _setThemeLegacy and _setThemeLayered unchanged when
an Element host is provided.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 89377d2b-459a-406e-bc0b-8c6d413d97cd

📥 Commits

Reviewing files that changed from the base of the PR and between 6087db9 and 1cad05f.

📒 Files selected for processing (2)

• packages/dialtone-tokens/tests/config.test.js
• packages/dialtone-tokens/themes/config.js