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: [18.3.0-canary] renderToString hoists some tags to top(working in 18.2) #27177

Closed
Thiry1 opened this issue Aug 2, 2023 · 1 comment · Fixed by #27269
Closed

Bug: [18.3.0-canary] renderToString hoists some tags to top(working in 18.2) #27177

Thiry1 opened this issue Aug 2, 2023 · 1 comment · Fixed by #27269
Assignees

Comments

@Thiry1
Copy link

Thiry1 commented Aug 2, 2023

React version: 18.3.0-canary-493f72b0a-20230727

Steps To Reproduce

  1. run following code.
import * as ReactDOMServer from "react-dom/server";
const element = (
  <html>
    <head>
      {/* meta and title are hoisted */}
      <meta charSet="utf-8" />
      <title>title</title>
      {/* the script tag is not hoisted */}
      <script src="foo"></script>
      {/* but this is hoisted */}
      <script src="foo" async></script>
    </head>
  </html>
);

console.log(ReactDOMServer.renderToString(element));

Link to code example:
https://codesandbox.io/s/react1830-canary-493f72b0a-20230727-ssr-hoist-bug-lvhj45?file=/src/index.js

The current behavior

console.log outputs <meta charSet="utf-8"/><script src="foo" async=""></script><title>title</title><html><head><script src="foo"></script></head></html>

The expected behavior

console.log outputs <html><head><meta charSet="utf-8"/><title>title</title><script src="foo"></script><script src="foo" async=""></script></head></html>

@Thiry1 Thiry1 added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Aug 2, 2023
@Thiry1 Thiry1 changed the title Bug: [18.3.0-canary] renderToString hoists some tag to top(working in 18.2) Bug: [18.3.0-canary] renderToString hoists some tags to top(working in 18.2) Aug 2, 2023
@Thiry1
Copy link
Author

Thiry1 commented Aug 2, 2023

Maybe it's related to #26106.

@kassens kassens added Type: Bug and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Aug 22, 2023
gnoff added a commit to gnoff/react that referenced this issue Aug 22, 2023
…ing the DOCTYPE included when rendering full documents by setting the root formatcontext to HTML_MODE rather than ROOT_HTML_MODE. Previously this was of little consequence but with Float the Root mode started to be used for things like determining if we could flush hoistable elements yet. In issue facebook#27177 we see that hoisted elements can appear before the <html> tag when using a legacy API `renderToString`.

This change makes the FormatContext responsible for providing the DOCTYPE chunks rather than the insertionMode. Alongside this the legacy mode now uses the ROOT_HTML_MODE insertionMode so that it follows standard logic for hoisting.

The reason I went with this approach is it allows us to avoid any runtime checks for whether we should or should not include a doctype. The only runtime perf consequence is that for legacy APIs when you render <html> there is an extra empty chunk to write which is of tivial consequence
gnoff added a commit to gnoff/react that referenced this issue Aug 22, 2023
…ing the DOCTYPE included when rendering full documents by setting the root formatcontext to HTML_MODE rather than ROOT_HTML_MODE. Previously this was of little consequence but with Float the Root mode started to be used for things like determining if we could flush hoistable elements yet. In issue facebook#27177 we see that hoisted elements can appear before the <html> tag when using a legacy API `renderToString`.

This change exports a DOCTYPE from FizzConfigDOM and FizzConfigDOMLegacy respectively, using an empty chunk in the legacy case. The only runtime perf cost here is that for legacy APIs there is an extra empty chunk to write when rendering a top level <html> tag which is trivial enough
gnoff added a commit to gnoff/react that referenced this issue Aug 22, 2023
…ing the DOCTYPE included when rendering full documents by setting the root formatcontext to HTML_MODE rather than ROOT_HTML_MODE. Previously this was of little consequence but with Float the Root mode started to be used for things like determining if we could flush hoistable elements yet. In issue facebook#27177 we see that hoisted elements can appear before the <html> tag when using a legacy API `renderToString`.

This change exports a DOCTYPE from FizzConfigDOM and FizzConfigDOMLegacy respectively, using an empty chunk in the legacy case. The only runtime perf cost here is that for legacy APIs there is an extra empty chunk to write when rendering a top level <html> tag which is trivial enough
gnoff added a commit to gnoff/react that referenced this issue Aug 22, 2023
…ing the DOCTYPE included when rendering full documents by setting the root formatcontext to HTML_MODE rather than ROOT_HTML_MODE. Previously this was of little consequence but with Float the Root mode started to be used for things like determining if we could flush hoistable elements yet. In issue facebook#27177 we see that hoisted elements can appear before the <html> tag when using a legacy API `renderToString`.

This change exports a DOCTYPE from FizzConfigDOM and FizzConfigDOMLegacy respectively, using an empty chunk in the legacy case. The only runtime perf cost here is that for legacy APIs there is an extra empty chunk to write when rendering a top level <html> tag which is trivial enough
gnoff added a commit that referenced this issue Aug 22, 2023
… in legacy apis such as `renderToString()` (#27269)

renderToString is a legacy server API which used a trick to avoid having
the DOCTYPE included when rendering full documents by setting the root
formatcontext to HTML_MODE rather than ROOT_HTML_MODE. Previously this
was of little consequence but with Float the Root mode started to be
used for things like determining if we could flush hoistable elements
yet. In issue #27177 we see that hoisted elements can appear before the
<html> tag when using a legacy API `renderToString`.

This change exports a DOCTYPE from FizzConfigDOM and FizzConfigDOMLegacy
respectively, using an empty chunk in the legacy case. The only runtime
perf cost here is that for legacy APIs there is an extra empty chunk to
write when rendering a top level <html> tag which is trivial enough

Fixes #27177
github-actions bot pushed a commit that referenced this issue Aug 22, 2023
… in legacy apis such as `renderToString()` (#27269)

renderToString is a legacy server API which used a trick to avoid having
the DOCTYPE included when rendering full documents by setting the root
formatcontext to HTML_MODE rather than ROOT_HTML_MODE. Previously this
was of little consequence but with Float the Root mode started to be
used for things like determining if we could flush hoistable elements
yet. In issue #27177 we see that hoisted elements can appear before the
<html> tag when using a legacy API `renderToString`.

This change exports a DOCTYPE from FizzConfigDOM and FizzConfigDOMLegacy
respectively, using an empty chunk in the legacy case. The only runtime
perf cost here is that for legacy APIs there is an extra empty chunk to
write when rendering a top level <html> tag which is trivial enough

Fixes #27177

DiffTrain build for [86198b9](86198b9)
EdisonVan pushed a commit to EdisonVan/react that referenced this issue Apr 15, 2024
… in legacy apis such as `renderToString()` (facebook#27269)

renderToString is a legacy server API which used a trick to avoid having
the DOCTYPE included when rendering full documents by setting the root
formatcontext to HTML_MODE rather than ROOT_HTML_MODE. Previously this
was of little consequence but with Float the Root mode started to be
used for things like determining if we could flush hoistable elements
yet. In issue facebook#27177 we see that hoisted elements can appear before the
<html> tag when using a legacy API `renderToString`.

This change exports a DOCTYPE from FizzConfigDOM and FizzConfigDOMLegacy
respectively, using an empty chunk in the legacy case. The only runtime
perf cost here is that for legacy APIs there is an extra empty chunk to
write when rendering a top level <html> tag which is trivial enough

Fixes facebook#27177
bigfootjon pushed a commit that referenced this issue Apr 18, 2024
… in legacy apis such as `renderToString()` (#27269)

renderToString is a legacy server API which used a trick to avoid having
the DOCTYPE included when rendering full documents by setting the root
formatcontext to HTML_MODE rather than ROOT_HTML_MODE. Previously this
was of little consequence but with Float the Root mode started to be
used for things like determining if we could flush hoistable elements
yet. In issue #27177 we see that hoisted elements can appear before the
<html> tag when using a legacy API `renderToString`.

This change exports a DOCTYPE from FizzConfigDOM and FizzConfigDOMLegacy
respectively, using an empty chunk in the legacy case. The only runtime
perf cost here is that for legacy APIs there is an extra empty chunk to
write when rendering a top level <html> tag which is trivial enough

Fixes #27177

DiffTrain build for commit 86198b9.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants