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

[React 19] Regression when using createPortal with DOM element created by dangerouslySetInnerHTML #31600

Open
jonathanhefner opened this issue Nov 20, 2024 · 2 comments
Labels

Comments

@jonathanhefner
Copy link

In React 18, it was possible to use createPortal with a DOM element created by dangerouslySetInnerHTML.

Example (adapted from this Stack Overflow answer):

import { useCallback, useState } from "react";
import { createPortal } from "react-dom";

export default function App() {
  const htmlFromElsewhere = `foo <span class="portal-container"></span> bar`;

  return <InnerHtmlWithPortals html={htmlFromElsewhere} />
}

function InnerHtmlWithPortals({ html }: { html: string }) {
  const [portalContainer, setPortalContainer] = useState<Element | null>(null)

  const refCallback = useCallback((el: HTMLDivElement) => {
    setPortalContainer(el?.querySelector(".portal-container"))
  })

  return <>
    <div ref={refCallback} dangerouslySetInnerHTML={{ __html: html }} />
    {portalContainer && createPortal(<Cake />, portalContainer)}
  </>
}

function Cake() {
  return <strong>cake</strong>
}

React 18 CodeSandbox: https://codesandbox.io/p/sandbox/optimistic-kowalevski-73sk5w

In React 19, this no longer works. React appears to be re-rendering the inner HTML after calling refCallback. Thus, createPortal succeeds, but the portalContainer element that it uses is no longer part of the DOM.

React 19 CodeSandbox: https://codesandbox.io/p/sandbox/vibrant-cloud-gd8yzr

It is possible to work around the issue by setting innerHTML directly instead of using dangerouslySetInnerHTML:

   const [portalContainer, setPortalContainer] = useState<Element | null>(null)

   const refCallback = useCallback((el: HTMLDivElement) => {
+    if (el) el.innerHTML = html
     setPortalContainer(el?.querySelector(".portal-container"))
-  })
+  }, [html])

   return <>
-    <div ref={refCallback} dangerouslySetInnerHTML={{ __html: html }} />
+    <div ref={refCallback} />
     {portalContainer && createPortal(<Cake />, portalContainer)}
   </>

But I'm not sure whether that is a reliable solution.

@denk0403
Copy link

I'm speculating here, but I think what's going on is that in the same render pass as when the portal container state is initialized, the "new" { __html: html} object creates completely new inner HTML elements. So by the time the portal tries to render, the portal container in state no longer matches the one actually in the DOM.

You can work around this by memoizing the dangerous HTML object with useMemo(). This seems to tell React to reuse the existing HTML elements in the DOM:

function InnerHtmlWithPortals({ html }: { html: string }) {
  const [portalContainer, setPortalContainer] = useState<Element | null>(null);

  const htmlMemo = useMemo(() => ({ __html: html }), [html]);

  const refCallback = useCallback((el: HTMLDivElement) => {
    setPortalContainer(el?.querySelector(".portal-container"));
  }, []);

  return (
    <>
      <div ref={refCallback} dangerouslySetInnerHTML={htmlMemo} />
      {portalContainer && createPortal(<Cake />, portalContainer)}
    </>
  );
}

I'm not sure what the actual cause for the regression is though.

@agriffis
Copy link

I ran into this bug without the involvement of a portal. Specifically:

import {useEffect, useRef, useState} from 'react'

const seen = new WeakSet()

function App() {
  const ref = useRef()
  const [, setX] = useState(false)

  useEffect(() => {
    console.log('in effect')

    const div = ref.current
    if (!seen.has(div)) {
      console.log('new div', div)
      seen.add(div)
    }

    const p = ref.current.firstChild
    if (!seen.has(p)) {
      console.log('new p', p)
      seen.add(p)
    }
  })

  useEffect(() => setX(true), [])

  return <div ref={ref} dangerouslySetInnerHTML={{__html: '<p>hello</p>'}} />
}

export default App

This will generate in the console:

new div
new p
new p

The second "new p" is when the paragraph element was replaced in the DOM, even though the HTML didn't change.

I've been using the same useMemo() trick in my code to work around the bug.

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

No branches or pull requests

3 participants