Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Reconciler cannot handle Declarative Shadow DOM (DSD) #26071

Closed
mayerraphael opened this issue Jan 28, 2023 · 17 comments
Closed

Bug: Reconciler cannot handle Declarative Shadow DOM (DSD) #26071

mayerraphael opened this issue Jan 28, 2023 · 17 comments
Labels
Resolution: Stale Automatically closed due to inactivity Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@mayerraphael
Copy link

mayerraphael commented Jan 28, 2023

The reconciler does not ignore <template shadowRoot="open"> but handles them like a normal HostElement.
In reality, as soon as the closing template tag is parsed, the component is replaced in the DOM by #shadow-root (open)

See: https://github.com/mfreed7/declarative-shadow-dom#-behavior

React version: 18.2.0

Steps To Reproduce

I tried this with NextJS 13.1.6, which uses react 18.2.0 and react-dom 18.2.0.

In the end the component is rendered server side and hydrated in the frontend.

  1. Add the following html code to your component
<div>
  <template shadowrootmode="open"> 
    <button type="button">
      <slot></slot>
    </button>
  </template>
  My button
</div>  
  1. Render the component via SSR and hydrate in the frontend.

The current behavior

A hydration warning is thrown:

Expected server HTML to contain a matching <template> in <div>.
    at template
    at div
    at Home 

The expected behavior

In the end i guess a DSD should be handled as an isolation block where on the server the DSD template tag is allowed, but on the client hydration all children of the block are hydrated to the now existing ShadowRoot.

Server -> Render template tag
Client -> Children of template tag are hydrated against the ShadowRoot fragment.

@mayerraphael mayerraphael added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jan 28, 2023
@jacobrask
Copy link

Note that it's now shadowrootmode and not shadowroot.

@jonathandewitt-dev
Copy link

Just removing my previous comments as they weren't as relevant as I believed before I got this far down the rabbit hole. For the time being, I may just steal some of your documentation about the suggestion to patch react-dom, I'm glad you identified this issue and it helped me understand it.

jonathandewitt-dev/react-shadow-scope#2

@o-t-w
Copy link

o-t-w commented Jul 30, 2023

I tried this in the latest version of both Remix and Next.js. It’s definitely an issue.

@jonathandewitt-dev
Copy link

@gaearon Is this on anyone's radar to fix? The issues are very distracting in my current Next project. I'd really rather not abandon React and Next, but unless this issue gets fixed, I might find myself with no other option.

@jonathandewitt-dev
Copy link

jonathandewitt-dev commented Sep 1, 2023

Side note @mayerraphael, at some point Next started precompiling react-dom so now you have to make changes to this thing to see any difference:

node_modules/next/dist/compiled/react-dom/cjs/react-dom.development.js

In case someone lands here from Google, here's what I had to do to make it work: Search for function beginWork$1. On the line BEFORE switch (workInProgress.tag), add the following:

var isDSD = workInProgress.type === 'template' && (
  workInProgress.pendingProps.shadowrootmode === 'open'
  || workInProgress.pendingProps.shadowrootmode === 'closed'
);

if (isDSD) {
  return null;
}

@mayerraphael
Copy link
Author

mayerraphael commented Sep 9, 2023

@jonathandewitt-dev in the end i solved it by only rendering the <template> element on the server. On the client you hydrate the content without the enclosing DSD and hydrate directly into the existing shadowRoot fragment.

I created a POC in Preact/Deno:
See client side hydration here: https://github.com/mayerraphael/webcomponent-streaming-ssr-deno/blob/873c92be4749507d5430b99f388aaa7f819697ee/platform/ui/component.ts#L84C13-L84C13

Se DSD enclosing on server here: https://github.com/mayerraphael/webcomponent-streaming-ssr-deno/blob/873c92be4749507d5430b99f388aaa7f819697ee/platform/ui/runtime.mts#L40

@jonathandewitt-dev
Copy link

Hmm, as far as I can tell, that project is not even depending on react-dom, so I'm not sure that resolves the issue at hand...

@mayerraphael
Copy link
Author

mayerraphael commented Sep 10, 2023

It is about the core concept.

Server -> Render tag, DSD and component nodes
Client -> Hydrate component nodes using existing ShadowRoot.

There is no reason the handle DSD on the client at all. At least not the tag, but React must replace it on client hydration with the ShadowRoot as the hydration root.

It is also not correct as i wrote in my initial post here that React should ignore the <template shadowroot="open"> tag and all its children

Cases like having an onClick here should of course still work:

<div>
  <template shadowrootmode="open">  // If we ignore everything from the DSD onwards, onClick in the next line will not hydrate.
    <button type="button" onClick={handleClick}>
      <slot></slot>
    </button>
  </template>
  My button
</div>  

The isolation of WebComponents and hydrating only specific ShadowRoots helps here. Meaning you can render something for a component on the server (tag, dsd, content) and use a different render/hydration target and just the component content on the client.

The only solution i could think of would be that React handles DSD <template> tags as an isolation block, meaning on hydration it searches for an ShadowRoot fragment in place of the DSD and hydrates all the children to that ShadowRoot.

@mayerraphael mayerraphael changed the title Bug: Reconciler should ignore Declarative Shadow DOM (DSD) Bug: Reconciler should handle Declarative Shadow DOM (DSD) Sep 10, 2023
@mayerraphael mayerraphael changed the title Bug: Reconciler should handle Declarative Shadow DOM (DSD) Bug: Reconciler cannot handle Declarative Shadow DOM (DSD) Sep 10, 2023
@jonathandewitt-dev
Copy link

Cases like having an onClick here should of course still work

I had a similar thought as well. I agree wholeheartedly with the concept. I'm going to see if I can fix this in react-dom. I'll fork it for now, and make a PR.

@jonathandewitt-dev
Copy link

Side note @mayerraphael, at some point Next started precompiling react-dom so now you have to make changes to this thing to see any difference:

node_modules/next/dist/compiled/react-dom/cjs/react-dom.development.js

In case someone lands here from Google, here's what I had to do to make it work: Search for function beginWork$1. On the line BEFORE switch (workInProgress.tag), add the following:

var isDSD = workInProgress.type === 'template' && (
  workInProgress.pendingProps.shadowrootmode === 'open'
  || workInProgress.pendingProps.shadowrootmode === 'closed'
);

if (isDSD) {
  return null;
}

To follow up on this, I've been encountering some strange issues after doing this. HMR breaks and random onClick listeners never get applied properly. This issue just keeps coming back to haunt my nightmares.

@mayerraphael
Copy link
Author

@jonathandewitt-dev

I'm currently working on updating my NextJS/WebComponent Repo by getting a StencilJS component working

Once this is done i will have a look on what needs to be adjusted in react-dom so i do not have to handle server and client differently because of the shadowRoot template tag, as seen here.

Copy link

github-actions bot commented Apr 9, 2024

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 9, 2024
@jonathandewitt-dev
Copy link

This issue is still very relevant, we just can't seem to get eyes on it from the React team... Having this fixed would greatly improve my npm package.

@audunolsen
Copy link

I'm not into the internals of React and do not use SSR myself and I just ran into the issue where just a Vite client based React app can't render the most basic declarative shadow dom example from MDN:

function App() {
  <div id="host">
    <template shadowrootmode="open">
      <span>I'm in the shadow DOM</span>
    </template>
  </div>
}

If I paste said html into my index.html file it does indeed show up, but that is useless if I can't use it within the context of React.

I haven't tried using shadow DOM with the js-API in React, and I would rather not as the DX is atrocious compared to the declarative variant.

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
@MarekZeman91
Copy link

On a current project running on Remix (could be done with Next as well) I had created a web components that support full SSR. I had created 2 versions where I have one with Shadow DOM and one without Shadow DOM. It's not as beautiful as regular JSX/TSX but it works great.

I'd not recommend doing it exactly this way but on the project we had no other option as the native components are provided to us from 3rd party.

Here is a small examples of a button WITH Shadow DOM.

MaButton.native.tsx

import type { JSX as ReactJSX, ReactElement, ReactNode } from "react";
import { renderToString } from "react-dom/server";
import type { MaButtonProps } from "./MaButton";

declare module "react/jsx-runtime" {
  export namespace JSX {
    interface IntrinsicElements {
      "ma-button": IntrinsicElements["div"] & {children?: ReactNode};
      template: ReactJSX.IntrinsicElements["template"] & { shadowrootmode: ShadowRootMode };
    }
  }
}

export const elementName = "ma-button";

// language=CSS
const css = `
  .${ elementName }__heading {
    all: unset;
    display: block;
    font-size: 18px;
    color: red;
    font-family: Arial, Helvetica, sans-serif;
  }
  .${ elementName }__button {
    background-color: silver;
    border: 1px solid silver;
    padding: 0.15em 0.25em 0.125em;
    border-radius: 3px;
    font-family: Arial, Helvetica, sans-serif;
  }
`;

export function template(props: MaButtonProps): ReactElement {
  if (typeof window === "undefined") {
    const html = renderToString(
      <>
        <style>{css}</style>
        <i className={`${elementName}__heading`}>{props.title} by 1</i>
        <button className={`${elementName}__button`} type="button">
          <slot></slot>
        </button>
      </>
    );

    return (
      <>
        <template
          shadowrootmode="open"
          dangerouslySetInnerHTML={{__html: html}}
        />
        {props.children}
      </>
    );
  }
  return <>{props.children}</>;
}

export interface MaButtonNative extends HTMLElement {
  onProps(props: MaButtonProps): void;
}

export function define() {
  if (customElements.get(elementName)) {
    return customElements.get(elementName);
  }

  class MaButtonNative extends HTMLElement {
    onProps(props: MaButtonProps) {
      this.shadowRoot!
        .querySelector<HTMLButtonElement>(`.${elementName}__button`)!
        .onclick = () => props.onClick?.();
    }
  }

  customElements.define(elementName, MaButtonNative);

  return MaButtonNative;
}

MaButton.tsx

import type { ReactNode } from "react";
import { useEffect, useState, useRef } from "react";
import { useOnComponentMount } from "../hooks/useOnComponentMount";
import { define, MaButtonNative, template } from "./MaButton.native";

export interface MaButtonProps {
  title: string;
  onClick?: () => void;
  children?: ReactNode;
}

export function MaButton(props: MaButtonProps) {
  const [ref, setRef] = useState<(HTMLDivElement & MaButtonNative) | null>(null);

  // keep always one object and only update the values
  const dynamicProps = Object.assign(useRef({} as MaButtonProps).current, props);

  useOnComponentMount(() => {
    define();
  });

  useEffect(() => {
    ref?.onProps(dynamicProps);
  }, [ref]);

  return (
    <ma-button ref={ el => setRef(el as never) } title={ props.title }>
      { template(props) }
    </ma-button>
  );
}

example ...

export default function Showroom() {
  const [clicks, setClicks] = useState(0);

  return (
    <div style={ { width: 640, padding: 20 } }>
      <MaButton title="Decrease" onClick={ () => setClicks(clicks - 1) }>
        Decrease
      </MaButton>
      <div>
        <b>{ clicks }</b>
      </div>
      <MaButton title="Increase" onClick={ () => setClicks(clicks + 1) }>
        Increase
      </MaButton>
    </div>
  );
}

polyfill in root html page

...
  // language=JavaScript
  const polyfillScript = `
    // it's safe to access document here since the page is not evaluated by server, just rendered
    (function attachShadowRoots(root = document) {
      const isShadowRootModeSupported = HTMLTemplateElement.prototype.hasOwnProperty("shadowRootMode");
      if (isShadowRootModeSupported) return;
      
      root
      .querySelectorAll('template[shadowrootmode]')
      .forEach(template => {
        const mode = template.getAttribute('shadowrootmode');
        const shadowRoot = template.parentElement.attachShadow({ mode });
        shadowRoot.appendChild(template.content);
        template.remove();
        attachShadowRoots(shadowRoot);
      });
    })();
  `;

  return (
    <html lang={locale} dir={i18n.dir()}>
      <head>
        ...
      </head>
      <body>
        { children }
        <script dangerouslySetInnerHTML={ { __html: polyfillScript } } />
      </body>
    </html>
  );
...

Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Jul 10, 2024
Copy link

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Stale Automatically closed due to inactivity Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

6 participants