Skip to content

Conversation

HexaField
Copy link
Contributor

@HexaField HexaField commented Sep 3, 2025

Summary by CodeRabbit

  • New Features
    • Wrap React components as native Web Components (Custom Elements).
    • Supports Shadow DOM, slot-based children, and optional automatic registration by tag name.
    • Maps attributes to props with type parsing, observed properties, and initial props.
    • Allows styling via adoptedStyleSheets or inline styles.
    • Batches updates for efficient rendering and handles connect/disconnect lifecycles.
    • Exposed as a new public API for embedding React in non-React environments.

Copy link
Contributor

coderabbitai bot commented Sep 3, 2025

Walkthrough

Adds a new utility to turn React components into Custom Elements and exposes it via the package’s public API by exporting reactToWebComponent from the React entry point.

Changes

Cohort / File(s) Summary
Public API exposure
ad4m-hooks/react/src/index.ts
Imports and re-exports reactToWebComponent, extending the module’s public API surface.
React-to-WebComponent utility
ad4m-hooks/react/src/reactToWebComponent.ts
Introduces reactToWebComponent that creates a Custom Element class wrapping a React component. Handles Shadow DOM, styles, props/attrs mapping with parsing, observed props, mutation observing, microtask-batched rendering, slot children, lifecycle mount/unmount, and optional auto-registration by tagName.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Dev as Developer Code
  participant WC as reactToWebComponent()
  participant CE as Custom Element Class
  participant DOM as Browser Custom Elements
  participant RD as ReactDOM Root
  participant RC as React Component

  Dev->>WC: reactToWebComponent(ReactComponent, React, ReactDOM, options)
  WC-->>Dev: returns HTMLElement subclass (CE)
  opt tagName provided and not defined
    WC->>DOM: customElements.define(tagName, CE)
  end

  DOM->>CE: new CE() + connectedCallback
  CE->>CE: init host (shadow or light), container, styles
  CE->>CE: read attributes → props (with prefix, parsing)
  CE->>RD: createRoot(container)
  CE->>RD: render(<ReactComponent {...props} host children/>)
  RD->>RC: mount

  note over CE,RC: Attribute/property changes
  DOM-->>CE: MutationObserver/prop set
  CE->>CE: update internal props (microtask-batched)
  CE->>RD: re-render with updated props

  DOM->>CE: disconnectedCallback
  CE->>RD: unmount
  CE->>CE: disconnect observers, cleanup
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

I twitch my ears at custom tags so neat,
A Reacty burrow where DOM and JSX meet.
With shadowed nests and props that softly flow,
I hop through attrs parsed from high to low.
One render per microtask—swift as my feet— 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/add-react-to-webcomponent

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (3)
ad4m-hooks/react/src/reactToWebComponent.ts (3)

59-61: Consider handling consecutive hyphens in kebab-case conversion.

The toCamel function doesn't handle edge cases like consecutive hyphens (--) or leading/trailing hyphens, which could lead to unexpected camelCase conversions.

Consider a more robust implementation:

-  const toCamel = (s: string) =>
-    s.replace(/-([a-z])/g, (_m, c) => c.toUpperCase());
+  const toCamel = (s: string) =>
+    s.replace(/-+([a-z])/g, (_m, c) => c.toUpperCase());

142-150: Props setter should perform deep comparison to avoid unnecessary renders.

The props setter triggers a render even when the same object reference is assigned or when props haven't actually changed, which could lead to performance issues.

Consider implementing a shallow comparison:

        set: (next: Record<string, any>) => {
          if (next && typeof next === "object") {
+            // Check if props actually changed
+            const hasChanged = Object.keys(next).some(key => this._props[key] !== next[key]) ||
+                              Object.keys(this._props).some(key => !(key in next));
+            if (!hasChanged) return;
            Object.assign(this._props, next);
            enqueueRender(this);
          }
        },

231-235: Silently ignoring already-defined custom elements could hide issues.

When a tag name is already defined, the code silently skips registration. This could mask configuration errors or naming conflicts.

Consider warning about the conflict:

   if (tagName) {
     if (!customElements.get(tagName)) {
       customElements.define(tagName, ReactCustomElement);
+    } else {
+      console.warn(`Custom element "${tagName}" is already defined. Skipping registration.`);
     }
   }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b8407e4 and 207a4e4.

📒 Files selected for processing (2)
  • ad4m-hooks/react/src/index.ts (1 hunks)
  • ad4m-hooks/react/src/reactToWebComponent.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ad4m-hooks/react/src/reactToWebComponent.ts (1)
ad4m-hooks/react/src/index.ts (1)
  • reactToWebComponent (11-11)
🔇 Additional comments (2)
ad4m-hooks/react/src/reactToWebComponent.ts (1)

179-196: Race condition in MutationObserver callback.

The MutationObserver processes multiple attribute changes and queues renders, but doesn't batch prop updates. This could cause intermediate invalid states if multiple attributes are changed simultaneously.

Consider batching all prop updates before queuing the render:

      this._observer = new MutationObserver((records) => {
+        let propsChanged = false;
         for (const r of records) {
           if (r.type !== "attributes" || !r.attributeName) continue;
           const attr = r.attributeName;
           if (attrPrefix && !attr.startsWith(attrPrefix)) continue;

           const logical = toCamel(
             attrPrefix ? attr.slice(attrPrefix.length) : attr
           );
           const raw = this.getAttribute(attr);
           const val = raw === null ? undefined : parseAttr(raw);
           // undefined deletes the prop (useful when attribute removed)
           if (val === undefined) delete this._props[logical];
           else this._props[logical] = val;
+          propsChanged = true;
         }
-        enqueueRender(this);
+        if (propsChanged) enqueueRender(this);
       });

Likely an incorrect or invalid review comment.

ad4m-hooks/react/src/index.ts (1)

8-11: LGTM! Clean API addition.

The export of reactToWebComponent is properly integrated into the module's public API.

Comment on lines +64 to +85
const parseAttr = (raw: string) => {
const v = raw.trim();
if (v === "") return ""; // empty string stays empty
if (v === "true") return true;
if (v === "false") return false;
if (v === "null") return null;
if (v === "undefined") return undefined;
// number?
if (/^[+-]?\d+(\.\d+)?$/.test(v)) return Number(v);
// try JSON (objects/arrays)
if (
(v.startsWith("{") && v.endsWith("}")) ||
(v.startsWith("[") && v.endsWith("]"))
) {
try {
return JSON.parse(v);
} catch {
/* fallthrough */
}
}
return raw; // as-is string
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add protection against prototype pollution in JSON parsing.

The parseAttr function parses JSON without validation, which could potentially be exploited if malicious attributes contain keys like __proto__, constructor, or prototype.

Consider validating parsed JSON objects:

      try {
-        return JSON.parse(v);
+        const parsed = JSON.parse(v);
+        // Basic prototype pollution protection
+        if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
+          const hasUnsafeKeys = Object.keys(parsed).some(key => 
+            key === '__proto__' || key === 'constructor' || key === 'prototype'
+          );
+          if (hasUnsafeKeys) {
+            console.warn(`Potentially unsafe JSON attribute ignored: ${v}`);
+            return raw;
+          }
+        }
+        return parsed;
      } catch {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const parseAttr = (raw: string) => {
const v = raw.trim();
if (v === "") return ""; // empty string stays empty
if (v === "true") return true;
if (v === "false") return false;
if (v === "null") return null;
if (v === "undefined") return undefined;
// number?
if (/^[+-]?\d+(\.\d+)?$/.test(v)) return Number(v);
// try JSON (objects/arrays)
if (
(v.startsWith("{") && v.endsWith("}")) ||
(v.startsWith("[") && v.endsWith("]"))
) {
try {
return JSON.parse(v);
} catch {
/* fallthrough */
}
}
return raw; // as-is string
};
const parseAttr = (raw: string) => {
const v = raw.trim();
if (v === "") return ""; // empty string stays empty
if (v === "true") return true;
if (v === "false") return false;
if (v === "null") return null;
if (v === "undefined") return undefined;
// number?
if (/^[+-]?\d+(\.\d+)?$/.test(v)) return Number(v);
// try JSON (objects/arrays)
if (
(v.startsWith("{") && v.endsWith("}")) ||
(v.startsWith("[") && v.endsWith("]"))
) {
try {
const parsed = JSON.parse(v);
// Basic prototype pollution protection
if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
const hasUnsafeKeys = Object.keys(parsed).some(key =>
key === '__proto__' || key === 'constructor' || key === 'prototype'
);
if (hasUnsafeKeys) {
console.warn(`Potentially unsafe JSON attribute ignored: ${v}`);
return raw;
}
}
return parsed;
} catch {
/* fallthrough */
}
}
return raw; // as-is string
};
🤖 Prompt for AI Agents
In ad4m-hooks/react/src/reactToWebComponent.ts around lines 64 to 85, the
parseAttr JSON branch does an unvalidated JSON.parse which can allow
prototype-pollution keys; after parsing, validate the result recursively and
reject any object that contains keys "__proto__", "constructor" or "prototype"
(including nested objects and arrays), or use JSON.parse with a reviver that
throws when those keys are encountered; if validation fails, fall back to
returning the original raw string (or null/undefined as appropriate) instead of
the parsed object, ensuring only safe plain objects/arrays are returned.

Comment on lines 108 to 133
// Where React renders
let host: ShadowRoot | HTMLElement;
if (shadow) {
const init: ShadowRootInit =
typeof shadow === "object" ? shadow : { mode: "open" };
host = this.attachShadow(init);
// Inject styles
if (styles) {
if (
Array.isArray(styles) &&
(host as any).adoptedStyleSheets !== undefined
) {
(host as any).adoptedStyleSheets = [
...(host as any).adoptedStyleSheets,
...styles,
];
} else {
const styleEl = document.createElement("style");
styleEl.textContent = Array.isArray(styles) ? "" : String(styles);
host.appendChild(styleEl);
}
}
} else {
host = this;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Potential memory leak with styles in non-Shadow DOM mode.

When shadow is false and styles are provided, the styles are not applied but the configuration suggests they should be. This could confuse users who expect styles to work in light DOM mode.

Either apply styles in light DOM mode or document that styles only work with Shadow DOM:

      } else {
        host = this;
+        // Apply styles to light DOM if provided
+        if (styles && !Array.isArray(styles)) {
+          const styleEl = document.createElement("style");
+          styleEl.textContent = String(styles);
+          this.appendChild(styleEl);
+        }
      }

Alternatively, consider throwing an error or warning when styles are provided without Shadow DOM enabled.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Where React renders
let host: ShadowRoot | HTMLElement;
if (shadow) {
const init: ShadowRootInit =
typeof shadow === "object" ? shadow : { mode: "open" };
host = this.attachShadow(init);
// Inject styles
if (styles) {
if (
Array.isArray(styles) &&
(host as any).adoptedStyleSheets !== undefined
) {
(host as any).adoptedStyleSheets = [
...(host as any).adoptedStyleSheets,
...styles,
];
} else {
const styleEl = document.createElement("style");
styleEl.textContent = Array.isArray(styles) ? "" : String(styles);
host.appendChild(styleEl);
}
}
} else {
host = this;
}
// Where React renders
let host: ShadowRoot | HTMLElement;
if (shadow) {
const init: ShadowRootInit =
typeof shadow === "object" ? shadow : { mode: "open" };
host = this.attachShadow(init);
// Inject styles
if (styles) {
if (
Array.isArray(styles) &&
(host as any).adoptedStyleSheets !== undefined
) {
(host as any).adoptedStyleSheets = [
...(host as any).adoptedStyleSheets,
...styles,
];
} else {
const styleEl = document.createElement("style");
styleEl.textContent = Array.isArray(styles) ? "" : String(styles);
host.appendChild(styleEl);
}
}
} else {
host = this;
// Apply styles to light DOM if provided
if (styles && !Array.isArray(styles)) {
const styleEl = document.createElement("style");
styleEl.textContent = String(styles);
this.appendChild(styleEl);
}
}
🤖 Prompt for AI Agents
In ad4m-hooks/react/src/reactToWebComponent.ts around lines 108 to 133, styles
are only injected when a ShadowRoot is created, so when shadow is false and
styles are provided they are ignored; update the branch that sets host = this to
handle styles as well: if styles is provided and shadow is false, either create
and append a <style> element to the host (converting array/other types to string
like the shadow branch does) or adopt the provided styleSheets into
document.styleSheets (or document.head) as appropriate, and add a console.warn
fallback if you prefer to notify consumers that styles are being applied
globally rather than in a shadow root; ensure you mirror the same array/string
handling logic used for the Shadow DOM case and avoid duplicating style elements
on repeated renders.

Comment on lines 214 to 217
disconnectedCallback() {
this._observer?.disconnect();
this._root.unmount();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing cleanup for styles and mount element.

The disconnectedCallback doesn't clean up the mount element or injected styles, which could cause memory leaks if elements are frequently attached/detached.

Add proper cleanup:

     disconnectedCallback() {
       this._observer?.disconnect();
       this._root.unmount();
+      // Clean up mount element
+      this._mount?.remove();
+      // Clean up styles if in light DOM
+      if (!shadow && styles) {
+        const styleEl = this.querySelector('style');
+        styleEl?.remove();
+      }
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
disconnectedCallback() {
this._observer?.disconnect();
this._root.unmount();
}
disconnectedCallback() {
this._observer?.disconnect();
this._root.unmount();
// Clean up mount element
this._mount?.remove();
// Clean up styles if in light DOM
if (!shadow && styles) {
const styleEl = this.querySelector('style');
styleEl?.remove();
}
}

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (4)
ad4m-hooks/react/src/reactToWebComponent.ts (4)

63-85: Harden JSON parsing to prevent prototype pollution.

Parsing attribute JSON without a reviver allows dangerous keys to slip in. Reject proto, prototype, and constructor at any depth.

Apply this diff:

       try {
-        return JSON.parse(v);
+        const unsafe = new Set(["__proto__", "prototype", "constructor"]);
+        const parsed = JSON.parse(v, (k, val) => {
+          if (unsafe.has(k)) throw new Error("unsafe key");
+          return val;
+        });
+        return parsed;
       } catch {
         /* fallthrough */
       }

131-134: Apply styles in light DOM mode too (and keep a handle for cleanup).

When shadow is false, provided styles are ignored. Inject a <style> for string styles and keep a reference for cleanup.

Apply this diff:

       } else {
         // For light DOM, set host now but defer creating children until connectedCallback
         this._host = this;
+        // Apply styles in light DOM if provided (string only)
+        if (styles) {
+          if (Array.isArray(styles)) {
+            console.warn(
+              "reactToWebComponent: CSSStyleSheet[] requires Shadow DOM; in light DOM use string CSS."
+            );
+          } else {
+            this._styleEl = document.createElement("style");
+            this._styleEl.textContent = String(styles);
+            this.appendChild(this._styleEl);
+          }
+        }
       }

And add a field to the class:

   class ReactCustomElement extends HTMLElement {
     private _root!: ReturnType<ReactDOMLike["createRoot"]>;
     private _mount!: HTMLElement;
     private _host!: ShadowRoot | HTMLElement;
+    private _styleEl?: HTMLStyleElement;

226-229: Fix re-attach behavior and clean up DOM nodes to prevent leaks.

After unmount, the root shouldn’t be reused; also remove the mount node and any injected style. Without resetting _isMounted, reconnection won’t remount.

Apply this diff:

     disconnectedCallback() {
       this._observer?.disconnect();
-      if (this._root) this._root.unmount();
+      if (this._root) {
+        this._root.unmount();
+        // @ts-expect-error: allow reset for reconnection
+        this._root = undefined;
+      }
+      this._mount?.remove();
+      this._isMounted = false;
+      // Clean up light DOM style tag
+      if (this._styleEl) {
+        this._styleEl.remove();
+        this._styleEl = undefined;
+      }
     }

231-240: Don’t overwrite user-provided children; only add in Shadow DOM.

Always passing a slot clobbers a children prop and, in light DOM, won’t project content.

Apply this diff:

-      const element = React.createElement(ReactComponent as any, {
-        ...this._props,
-        host: this,
-        children: React.createElement("slot"),
-      });
-      this._root.render(element);
+      const hasChildren = Object.prototype.hasOwnProperty.call(
+        this._props,
+        "children"
+      );
+      const props: any = { ...this._props, host: this };
+      if (!hasChildren && this._host instanceof ShadowRoot) {
+        props.children = React.createElement("slot");
+      }
+      this._root.render(React.createElement(ReactComponent as any, props));
🧹 Nitpick comments (4)
ad4m-hooks/react/src/reactToWebComponent.ts (4)

35-35: Type nit: redundant any | any.

-  ReactComponent: (props: P) => any | any,
+  ReactComponent: (props: P) => any,

87-95: Add a microtask fallback for older environments.

queueMicrotask isn’t universal; fallback to a resolved Promise.

-    queueMicrotask(() => {
+    const enqueue =
+      typeof queueMicrotask === "function"
+        ? queueMicrotask
+        : (cb: () => void) => Promise.resolve().then(cb);
+    enqueue(() => {
       el._renderQueued = false;
       el._render();
     });

41-42: attrPrefix default of "" may cause noisy re-renders.

With no prefix, mutations to class, style, etc., become props and trigger renders. Consider recommending a prefix like "data-" in docs, or defaulting to it.


243-248: Validate tagName shape before define.

Custom element names must contain a hyphen and be lowercase; warn if invalid to prevent runtime errors.

Example:

if (tagName) {
  const valid = /^[a-z0-9]+(-[a-z0-9-]+)+$/.test(tagName);
  if (!valid) {
    console.warn(`reactToWebComponent: invalid tagName "${tagName}"`);
  } else if (!customElements.get(tagName)) {
    customElements.define(tagName, ReactCustomElement);
  }
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 207a4e4 and b389aa1.

📒 Files selected for processing (1)
  • ad4m-hooks/react/src/reactToWebComponent.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
ad4m-hooks/react/src/reactToWebComponent.ts (1)
ad4m-hooks/react/src/index.ts (1)
  • reactToWebComponent (11-11)
🔇 Additional comments (1)
ad4m-hooks/react/src/reactToWebComponent.ts (1)

34-46: Nice API surface and minimal React contract.

Clean separation of concerns; defaults are sensible and the factory return type is clear.

Comment on lines +31 to +33
* - Attribute values are parsed into primitives/JSON when possible.
* - Light-DOM children are supported via <slot/> passed as `children`.
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Clarify docs: slot-based children work only with Shadow DOM.

In light DOM there’s no slotting; advise passing children via props or enable shadow.

- * - Light-DOM children are supported via <slot/> passed as `children`.
+ * - Children projection via <slot/> works with Shadow DOM only.
+ *   In light DOM, pass children via props (or enable `shadow`).
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
* - Attribute values are parsed into primitives/JSON when possible.
* - Light-DOM children are supported via <slot/> passed as `children`.
*/
* - Attribute values are parsed into primitives/JSON when possible.
* - Children projection via <slot/> works with Shadow DOM only.
* In light DOM, pass children via props (or enable `shadow`).
*/
🤖 Prompt for AI Agents
In ad4m-hooks/react/src/reactToWebComponent.ts around lines 31 to 33, update the
documentation to clarify that slot-based children are only supported when the
component uses Shadow DOM; in Light DOM there is no slotting so consumers should
either pass children via props or enable Shadow DOM on the web component. Adjust
the comment text to explicitly state this limitation and provide a brief example
or note recommending props for Light DOM usage or how to enable shadow when slot
behavior is required.

Comment on lines +116 to +130
if (styles) {
if (
Array.isArray(styles) &&
(this._host as any).adoptedStyleSheets !== undefined
) {
(this._host as any).adoptedStyleSheets = [
...(this._host as any).adoptedStyleSheets,
...styles,
];
} else {
const styleEl = document.createElement("style");
styleEl.textContent = Array.isArray(styles) ? "" : String(styles);
this._host.appendChild(styleEl);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid silently dropping styles when CSSStyleSheet[] is provided but adoptedStyleSheets is unsupported.

Current fallback injects an empty <style>, losing all CSS. Warn instead, and only inject text when a string is supplied.

Apply this diff:

-        if (styles) {
-          if (
-            Array.isArray(styles) &&
-            (this._host as any).adoptedStyleSheets !== undefined
-          ) {
-            (this._host as any).adoptedStyleSheets = [
-              ...(this._host as any).adoptedStyleSheets,
-              ...styles,
-            ];
-          } else {
-            const styleEl = document.createElement("style");
-            styleEl.textContent = Array.isArray(styles) ? "" : String(styles);
-            this._host.appendChild(styleEl);
-          }
-        }
+        if (styles) {
+          const supportsAdoption =
+            (this._host as any).adoptedStyleSheets !== undefined;
+          if (Array.isArray(styles)) {
+            if (supportsAdoption) {
+              (this._host as any).adoptedStyleSheets = [
+                ...(this._host as any).adoptedStyleSheets,
+                ...styles,
+              ];
+            } else {
+              console.warn(
+                "reactToWebComponent: adoptedStyleSheets unsupported; provide `styles` as a string."
+              );
+            }
+          } else {
+            const styleEl = document.createElement("style");
+            styleEl.textContent = String(styles);
+            this._host.appendChild(styleEl);
+          }
+        }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (styles) {
if (
Array.isArray(styles) &&
(this._host as any).adoptedStyleSheets !== undefined
) {
(this._host as any).adoptedStyleSheets = [
...(this._host as any).adoptedStyleSheets,
...styles,
];
} else {
const styleEl = document.createElement("style");
styleEl.textContent = Array.isArray(styles) ? "" : String(styles);
this._host.appendChild(styleEl);
}
}
if (styles) {
const supportsAdoption =
(this._host as any).adoptedStyleSheets !== undefined;
if (Array.isArray(styles)) {
if (supportsAdoption) {
(this._host as any).adoptedStyleSheets = [
...(this._host as any).adoptedStyleSheets,
...styles,
];
} else {
console.warn(
"reactToWebComponent: adoptedStyleSheets unsupported; provide `styles` as a string."
);
}
} else {
const styleEl = document.createElement("style");
styleEl.textContent = String(styles);
this._host.appendChild(styleEl);
}
}
🤖 Prompt for AI Agents
In ad4m-hooks/react/src/reactToWebComponent.ts around lines 116-130, the current
fallback for when styles is a CSSStyleSheet[] but the host does not support
adoptedStyleSheets creates an empty <style> element (losing all CSS); change the
logic so that if styles is an array and adoptedStyleSheets is undefined you do
NOT inject an empty style but instead emit a console.warn (or use the existing
logger) explaining that CSSStyleSheet[] cannot be applied in this environment,
and only create and append a <style> element when styles is a string (or
non-array) by setting its textContent to String(styles).

@lucksus
Copy link
Member

lucksus commented Sep 24, 2025

@HexaField, I think coderabbit might have a point. Can you take a look at what it's saying and either fix or resolve the threads?

@HexaField HexaField marked this pull request as draft September 24, 2025 08:52
@HexaField
Copy link
Contributor Author

HexaField commented Sep 24, 2025

@HexaField, I think coderabbit might have a point. Can you take a look at what it's saying and either fix or resolve the threads?

Yes sorry, this was meant to be a draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants