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

[Float][Fizz][Legacy] hoisted elements no longer emit before <html> in legacy apis such as renderToString() #27269

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

gnoff
Copy link
Collaborator

@gnoff gnoff commented Aug 22, 2023

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 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 tag which is trivial enough

Fixes #27177

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 22, 2023
@react-sizebot
Copy link

react-sizebot commented Aug 22, 2023

Comparing: b277259...1120c0e

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 165.67 kB 165.67 kB = 51.99 kB 51.99 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 173.16 kB 173.16 kB = 54.21 kB 54.21 kB
facebook-www/ReactDOM-prod.classic.js = 570.50 kB 570.50 kB = 100.49 kB 100.49 kB
facebook-www/ReactDOM-prod.modern.js = 554.30 kB 554.30 kB = 97.66 kB 97.66 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 1120c0e

…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 gnoff merged commit 86198b9 into facebook:main Aug 22, 2023
2 checks passed
@gnoff gnoff deleted the bugfix-hoisting branch August 22, 2023 17:54
github-actions bot pushed a commit that referenced this pull request 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 pull request 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 pull request 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
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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