facebook
[ServerRenderer][draft] Add option to send instructions as data attributes
Would love to get some feedback on this before writing tests.
Changes made:
- Running with enableFizzExternalRuntime (feature flag) and unstable_externalRuntimeSrc (param) will generate the following
<div
hidden data-react-server-instruction="$INSTR"
data-react-server-arg0="param0"
data-react-server-arg1="param1"
></div>
What's left (done)
- ~~Add
disableInstructionExecutionas an entrypoint parameter~~ - ~~Add a MutationObserver implementation~~
- ~~Add ^ to test setup (when using Meta's feature flags)~~
Summary
How did you test this change?
Comparing: e98225485a124e35abc4cea82e6da944472ce7c7...f35926fdbe4c415467a7622b80b3ea83e03dbd78
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 | = | 154.55 kB | 154.55 kB | = | 49.05 kB | 49.04 kB |
| oss-experimental/react-dom/cjs/react-dom.production.min.js | = | 156.47 kB | 156.47 kB | = | 49.67 kB | 49.67 kB |
| facebook-www/ReactDOM-prod.classic.js | = | 533.67 kB | 533.67 kB | = | 94.54 kB | 94.54 kB |
| facebook-www/ReactDOM-prod.modern.js | = | 518.77 kB | 518.77 kB | = | 92.35 kB | 92.35 kB |
| facebook-www/ReactDOMForked-prod.classic.js | = | 533.67 kB | 533.67 kB | = | 94.54 kB | 94.54 kB |
| oss-experimental/react-dom/unstable_server-external-runtime.js | +65.66% | 1.52 kB | 2.51 kB | +55.60% | 0.73 kB | 1.14 kB |
| oss-stable-semver/react-dom/unstable_server-external-runtime.js | +65.66% | 1.52 kB | 2.51 kB | +55.60% | 0.73 kB | 1.14 kB |
| oss-stable/react-dom/unstable_server-external-runtime.js | +65.66% | 1.52 kB | 2.51 kB | +55.60% | 0.73 kB | 1.14 kB |
| facebook-www/ReactDOMServerStreaming-prod.modern.js | +5.57% | 121.77 kB | 128.56 kB | +2.84% | 24.02 kB | 24.70 kB |
| facebook-www/ReactDOMServer-prod.modern.js | +5.43% | 117.08 kB | 123.43 kB | +2.87% | 22.73 kB | 23.39 kB |
| facebook-www/ReactDOMServer-prod.classic.js | +5.29% | 120.22 kB | 126.57 kB | +2.73% | 23.37 kB | 24.01 kB |
| facebook-www/ReactDOMServer-dev.modern.js | +3.19% | 326.42 kB | 336.82 kB | +2.15% | 73.39 kB | 74.96 kB |
| facebook-www/ReactDOMServerStreaming-dev.modern.js | +3.16% | 321.90 kB | 332.07 kB | +2.10% | 72.36 kB | 73.88 kB |
| facebook-www/ReactDOMServer-dev.classic.js | +3.12% | 333.11 kB | 343.52 kB | +2.10% | 74.82 kB | 76.39 kB |
Significant size changes
Includes any change greater than 0.2%:
Expand to show
Generated by :no_entry_sign: dangerJS against f35926fdbe4c415467a7622b80b3ea83e03dbd78
@acdlite @gnoff @sebmarkbage Thanks for all the feedback so far! All suggested changes have been made, and I'd love to get some feedback on this again 🙂
Thanks @sebmarkbage @gnoff, I really appreciate all the feedback you've given - I definitely learned a lot writing this PR! Sorry for the long wait (I wanted to confirm that these changes worked for www).
Would love one more (possibly last?) pass 🙂
Also had a question about https://github.com/facebook/react/pull/25437#discussion_r1018398657
I think going forward our suggestion for rendering React only to part of the tree will be an API that treats and existing ID as the target for the shell but assume that the streamed content gets concatenated to the body.
Why do we assume that streamed content gets concatenated to the body? Trying to understand what could go wrong with concatenating streamed content to a div container instead
(Sorry for the back and forth, and thanks again for the review! Once again putting this back on your queue @gnoff )
(Sorry for the back and forth, and thanks again for the review! Once again putting this back on your queue @gnoff )