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

Uncaught DOMException when having <!DOCTYPE html> and setting the parentDom to document when rendering #3794

Open
1 task
roj1512 opened this issue Nov 13, 2022 · 13 comments
Labels
11.x compat enhancement known issue The issue is known and may be left as-is. workaround

Comments

@roj1512
Copy link

roj1512 commented Nov 13, 2022

  • Check if updating to the latest Preact version resolves the issue

Describe the bug
If we have <!DOCTYPE html> in our HTML document where we use Preact, and try to render or hydrate a component in the document, we get an Uncaught DOMException error, and the component does not render/hydrate. Although, it'll be fine if we remove <!DOCTYPE html>. This is very a strange behavior.

To Reproduce

<!DOCTYPE html>
<html>
  <body>
  </body>
</html>
<script type="module">
  import { h, Component, render } from 'https://unpkg.com/preact?module';

  const app = h('html', null, h('body', null, h('h1', null, 'Hello World!')));

  render(app, document);
</script>

Steps to reproduce the behavior:

  1. Open the above HTML in your browser.
  2. You will face the error.
  3. Try removing the first line, <!DOCTYPE html>.
  4. See how the error is gone.

Expected behavior
Should act just fine :)

@marvinhagemeister
Copy link
Member

That is the correct behavior and not a bug as the document is owned by the browser.

The second argument passed to render is a DOM node that's expected to be fully owned by Preact. When Preact detects that there are existing nodes there that doesn't match the virtual tree that it was given, we'll try to insert nodes before that. One of those nodes in the doctype and inserting nodes before that is not allowed like the error states.

Even in theory, rendering into the document itself is a very brittle approach as you're relying on the frameworks logic to not blow away nodes that are not present in the virtual tree, like for example the <script> tag in this scenario. It's not worth it fighting the way HTML works.

Instead, create a container element inside <body> that is exclusively owned by Preact. That's the recommended approach.

  <!DOCTYPE html>
  <html>
    <body>
+    <div id="app"></div>
+    <script type="module">
+      import { h, Component, render } from 'https://unpkg.com/preact?module';
+
+      const app = h('h1', null, 'Hello World!');
+
+      render(app, document.getElementById("app"));
+    </script>
    </body>
  </html>
-  <script type="module">
-    import { h, Component, render } from 'https://unpkg.com/preact?module';
-
-    const app = h('html', null, h('body', null, h('h1', null, 'Hello World!')));
-
-    render(app, document);
-  </script>

@roj1512
Copy link
Author

roj1512 commented Nov 14, 2022

@marvinhagemeister What about this:

  • I want to SSR.
  • And then hydrate in the browser to the body.
  • Rerender the head?

Rendering in the document is a common practice. It works with React DOM. React DOM Server will even include the <!DOCTYPE html>.

@marvinhagemeister
Copy link
Member

marvinhagemeister commented Nov 14, 2022

Rendering in the document is a common practice. It works with React DOM. React DOM Server will even include the <!DOCTYPE html>.

You don't get this error in React because it aggressively removes any nodes not matching the virtual tree. In this case it drops the doctype node completely which makes the HTML invalid. It works regardless of that because browsers are pretty good with dealing with invalid HTML documents due to quirks mode.

The common and recommended approach to doing SSR in both React and Preact is to stitch the document manually. Nodes supposed to be included in the <head> section are usually generated via react-helmet, react-head or something custom.

const html = `<!DOCTYPE html>
<html>
<head>
  ${headNodes}
</head>
<body>
  <div id="app>${app}</div>
  <script type="module">
    // ...app code + hydration here
  </script>
</body>
</html>
`

@roj1512
Copy link
Author

roj1512 commented Nov 14, 2022

I know about this way of rendering the head, but what if we wanted to use a state in the head?

@marvinhagemeister
Copy link
Member

Can you share more about your use case why you need state in the <head> portion of the document?

@roj1512
Copy link
Author

roj1512 commented Nov 14, 2022

I don't have anything in mind right now. It's just super weird to not be able to hydrate the whole document. Everything does that nowadays.

@marvinhagemeister
Copy link
Member

Everything does that nowadays.

That doesn't match my experience, but I'm curious to learn more. Can you share some links to projects which do that in the way you're describing?

@roj1512
Copy link
Author

roj1512 commented Nov 14, 2022

Ultra

This even supports both React and Preact, but for the Preact support it does not use preact-render-to-string because of this issue, instead it uses ReactDOMServer with preact/compat.

https://github.com/exhibitionist-digital/ultra/blob/main/examples/with-react-router/client.tsx#L6

@roj1512
Copy link
Author

roj1512 commented Nov 16, 2022

@marvinhagemeister Can you please reopen this?

@roj1512
Copy link
Author

roj1512 commented Nov 23, 2022

@marvinhagemeister @JoviDeCroock @andrewiggins

Since you're not reopening this, should I open a new issue?

@developit
Copy link
Member

Shouldn't this be rendering into document.documentElement?

I'd like to support this in Preact, since Remix is already relying on it.

@developit developit reopened this Dec 2, 2022
@developit developit added 11.x compat enhancement known issue The issue is known and may be left as-is. workaround and removed invalid labels Dec 2, 2022
@roj1512

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
11.x compat enhancement known issue The issue is known and may be left as-is. workaround
Projects
None yet
Development

No branches or pull requests

3 participants