@samitouri / QOS-React-2 / commits / a2766387ef

[Fizz] Improve text separator byte efficiency (#24630)

* [Fizz] Improve text separator byte efficiency Previously text separators were inserted following any Text node in Fizz. This increases bytes sent when streaming and in some cases such as title elements these separators are not interpreted as comment nodes and leak into the visual aspects of a page as escaped text. The reason simple tracking on the last pushed type doesn't work is that Segments can be filled in asynchronously later and so you cannot know in a single pass whether the preceding content was a text node or not. This commit adds a concept of TextEmbedding which provides a best effort signal to Segments on whether they are embedded within text. This allows the later resolution of that Segment to add text separators when possibly necessary but avoid them when they are surely not. The current implementation can only "peek" head if the segment is a the Root Segment or a Suspense Boundary Segment. In these cases we know there is no trailing text embedding and we can eliminate the separator at the end of the segment if the last emitted element was Text. In normal Segments we cannot peek and thus have to assume there might be a trailing text embedding and we issue a separator defensively. This should be rare in practice as it is assumed most components that will cause segment creation will also emit some markup at the edges. * [Fizz] Improve separator efficiency when flushing delayed segments The method by which we get segment markup into the DOM differs depending on when the Segment resolves. If a Segment resolves before flushing begins for it's parent it will be emitted inline with the parent markup. In these cases separators may be necessary because they are how we clue the browser into breakup up text into distinct nodes that will later match up with what will be hydrated on the client. If a Segment resolves after flushing has happened a script will be used to patch up the DOM in the client. when this happens if there are any text nodes on the boundary of the patch they won't be "merged" and thus will continue to have distinct representation as Nodes in the DOM. Thus we can avoid doing any separators at the boundaries in these cases. After applying these changes the only time you will get text separators as follows * in between serial text nodes that emit at the same time - these are necessary and cannot be eliminated unless we stop relying on the browser to automatically parse the correct text nodes when processing this HTML * after a final text node in a non-boundary segment that resolves before it's parent has flushed - these are sometimes extraneous, like when the next emitted thing is a non-Text node. In all other cases text separators should be omitted which means the general byte efficiency of this approach should be pretty good

Josh Story committed May 28, 2022 at 08:30 UTC a2766387efe68b318b23d8c35c70b850d1e6a250
11 files changed +579 -59
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+422
@@ -234,6 +234,11 @@ describe('ReactDOMFizzServer', () => {
234 return readText(text);
235 }
236
237 + function AsyncTextWrapped({as, text}) {
238 + const As = as;
239 + return <As>{readText(text)}</As>;
240 + }
241 +
242 // @gate experimental
243 it('should asynchronously load a lazy component', async () => {
244 let resolveA;
@@ -3577,4 +3582,421 @@ describe('ReactDOMFizzServer', () => {
3582 </div>,
3583 );
3584 });
3585 +
3586 + describe('text separators', () => {
3587 + // To force performWork to start before resolving AsyncText but before piping we need to wait until
3588 + // after scheduleWork which currently uses setImmediate to delay performWork
3589 + function afterImmediate() {
3590 + return new Promise(resolve => {
3591 + setImmediate(resolve);
3592 + });
3593 + }
3594 +
3595 + // @gate experimental
3596 + it('it only includes separators between adjacent text nodes', async () => {
3597 + function App({name}) {
3598 + return (
3599 + <div>
3600 + hello<b>world, {name}</b>!
3601 + </div>
3602 + );
3603 + }
3604 +
3605 + await act(async () => {
3606 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
3607 + <App name="Foo" />,
3608 + );
3609 + pipe(writable);
3610 + });
3611 +
3612 + expect(container.innerHTML).toEqual(
3613 + '<div>hello<b>world, <!-- -->Foo</b>!</div>',
3614 + );
3615 + const errors = [];
3616 + ReactDOMClient.hydrateRoot(container, <App name="Foo" />, {
3617 + onRecoverableError(error) {
3618 + errors.push(error.message);
3619 + },
3620 + });
3621 + expect(Scheduler).toFlushAndYield([]);
3622 + expect(errors).toEqual([]);
3623 + expect(getVisibleChildren(container)).toEqual(
3624 + <div>
3625 + hello<b>world, {'Foo'}</b>!
3626 + </div>,
3627 + );
3628 + });
3629 +
3630 + // @gate experimental
3631 + it('it does not insert text separators even when adjacent text is in a delayed segment', async () => {
3632 + function App({name}) {
3633 + return (
3634 + <Suspense fallback={'loading...'}>
3635 + <div id="app-div">
3636 + hello
3637 + <b>
3638 + world, <AsyncText text={name} />
3639 + </b>
3640 + !
3641 + </div>
3642 + </Suspense>
3643 + );
3644 + }
3645 +
3646 + await act(async () => {
3647 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
3648 + <App name="Foo" />,
3649 + );
3650 + pipe(writable);
3651 + });
3652 +
3653 + expect(document.getElementById('app-div').outerHTML).toEqual(
3654 + '<div id="app-div">hello<b>world, <template id="P:1"></template></b>!</div>',
3655 + );
3656 +
3657 + await act(() => resolveText('Foo'));
3658 +
3659 + expect(container.firstElementChild.outerHTML).toEqual(
3660 + '<div id="app-div">hello<b>world, Foo</b>!</div>',
3661 + );
3662 + // there are extra script nodes at the end of container
3663 + expect(container.childNodes.length).toBe(5);
3664 + const div = container.childNodes[1];
3665 + expect(div.childNodes.length).toBe(3);
3666 + const b = div.childNodes[1];
3667 + expect(b.childNodes.length).toBe(2);
3668 + expect(b.childNodes[0]).toMatchInlineSnapshot('world, ');
3669 + expect(b.childNodes[1]).toMatchInlineSnapshot('Foo');
3670 +
3671 + const errors = [];
3672 + ReactDOMClient.hydrateRoot(container, <App name="Foo" />, {
3673 + onRecoverableError(error) {
3674 + errors.push(error.message);
3675 + },
3676 + });
3677 + expect(Scheduler).toFlushAndYield([]);
3678 + expect(errors).toEqual([]);
3679 + expect(getVisibleChildren(container)).toEqual(
3680 + <div id="app-div">
3681 + hello<b>world, {'Foo'}</b>!
3682 + </div>,
3683 + );
3684 + });
3685 +
3686 + // @gate experimental
3687 + it('it works with multiple adjacent segments', async () => {
3688 + function App() {
3689 + return (
3690 + <Suspense fallback={'loading...'}>
3691 + <div id="app-div">
3692 + h<AsyncText text={'ello'} />
3693 + w<AsyncText text={'orld'} />
3694 + </div>
3695 + </Suspense>
3696 + );
3697 + }
3698 +
3699 + await act(async () => {
3700 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
3701 + pipe(writable);
3702 + });
3703 +
3704 + expect(document.getElementById('app-div').outerHTML).toEqual(
3705 + '<div id="app-div">h<template id="P:1"></template>w<template id="P:2"></template></div>',
3706 + );
3707 +
3708 + await act(() => resolveText('orld'));
3709 +
3710 + expect(document.getElementById('app-div').outerHTML).toEqual(
3711 + '<div id="app-div">h<template id="P:1"></template>world</div>',
3712 + );
3713 +
3714 + await act(() => resolveText('ello'));
3715 + expect(container.firstElementChild.outerHTML).toEqual(
3716 + '<div id="app-div">helloworld</div>',
3717 + );
3718 +
3719 + const errors = [];
3720 + ReactDOMClient.hydrateRoot(container, <App name="Foo" />, {
3721 + onRecoverableError(error) {
3722 + errors.push(error.message);
3723 + },
3724 + });
3725 + expect(Scheduler).toFlushAndYield([]);
3726 + expect(errors).toEqual([]);
3727 + expect(getVisibleChildren(container)).toEqual(
3728 + <div id="app-div">{['h', 'ello', 'w', 'orld']}</div>,
3729 + );
3730 + });
3731 +
3732 + // @gate experimental
3733 + it('it works when some segments are flushed and others are patched', async () => {
3734 + function App() {
3735 + return (
3736 + <Suspense fallback={'loading...'}>
3737 + <div id="app-div">
3738 + h<AsyncText text={'ello'} />
3739 + w<AsyncText text={'orld'} />
3740 + </div>
3741 + </Suspense>
3742 + );
3743 + }
3744 +
3745 + await act(async () => {
3746 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
3747 + await afterImmediate();
3748 + await act(() => resolveText('ello'));
3749 + pipe(writable);
3750 + });
3751 +
3752 + expect(document.getElementById('app-div').outerHTML).toEqual(
3753 + '<div id="app-div">h<!-- -->ello<!-- -->w<template id="P:1"></template></div>',
3754 + );
3755 +
3756 + await act(() => resolveText('orld'));
3757 +
3758 + expect(container.firstElementChild.outerHTML).toEqual(
3759 + '<div id="app-div">h<!-- -->ello<!-- -->world</div>',
3760 + );
3761 +
3762 + const errors = [];
3763 + ReactDOMClient.hydrateRoot(container, <App />, {
3764 + onRecoverableError(error) {
3765 + errors.push(error.message);
3766 + },
3767 + });
3768 + expect(Scheduler).toFlushAndYield([]);
3769 + expect(errors).toEqual([]);
3770 + expect(getVisibleChildren(container)).toEqual(
3771 + <div id="app-div">{['h', 'ello', 'w', 'orld']}</div>,
3772 + );
3773 + });
3774 +
3775 + // @gate experimental
3776 + it('it does not prepend a text separators if the segment follows a non-Text Node', async () => {
3777 + function App() {
3778 + return (
3779 + <Suspense fallback={'loading...'}>
3780 + <div>
3781 + hello
3782 + <b>
3783 + <AsyncText text={'world'} />
3784 + </b>
3785 + </div>
3786 + </Suspense>
3787 + );
3788 + }
3789 +
3790 + await act(async () => {
3791 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
3792 + await afterImmediate();
3793 + await act(() => resolveText('world'));
3794 + pipe(writable);
3795 + });
3796 +
3797 + expect(container.firstElementChild.outerHTML).toEqual(
3798 + '<div>hello<b>world<!-- --></b></div>',
3799 + );
3800 +
3801 + const errors = [];
3802 + ReactDOMClient.hydrateRoot(container, <App />, {
3803 + onRecoverableError(error) {
3804 + errors.push(error.message);
3805 + },
3806 + });
3807 + expect(Scheduler).toFlushAndYield([]);
3808 + expect(errors).toEqual([]);
3809 + expect(getVisibleChildren(container)).toEqual(
3810 + <div>
3811 + hello<b>world</b>
3812 + </div>,
3813 + );
3814 + });
3815 +
3816 + // @gate experimental
3817 + it('it does not prepend a text separators if the segments first emission is a non-Text Node', async () => {
3818 + function App() {
3819 + return (
3820 + <Suspense fallback={'loading...'}>
3821 + <div>
3822 + hello
3823 + <AsyncTextWrapped as={'b'} text={'world'} />
3824 + </div>
3825 + </Suspense>
3826 + );
3827 + }
3828 +
3829 + await act(async () => {
3830 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
3831 + await afterImmediate();
3832 + await act(() => resolveText('world'));
3833 + pipe(writable);
3834 + });
3835 +
3836 + expect(container.firstElementChild.outerHTML).toEqual(
3837 + '<div>hello<b>world</b></div>',
3838 + );
3839 +
3840 + const errors = [];
3841 + ReactDOMClient.hydrateRoot(container, <App />, {
3842 + onRecoverableError(error) {
3843 + errors.push(error.message);
3844 + },
3845 + });
3846 + expect(Scheduler).toFlushAndYield([]);
3847 + expect(errors).toEqual([]);
3848 + expect(getVisibleChildren(container)).toEqual(
3849 + <div>
3850 + hello<b>world</b>
3851 + </div>,
3852 + );
3853 + });
3854 +
3855 + // @gate experimental
3856 + it('should not insert separators for text inside Suspense boundaries even if they would otherwise be considered text-embedded', async () => {
3857 + function App() {
3858 + return (
3859 + <Suspense fallback={'loading...'}>
3860 + <div id="app-div">
3861 + start
3862 + <Suspense fallback={'[loading first]'}>
3863 + firststart
3864 + <AsyncText text={'first suspended'} />
3865 + firstend
3866 + </Suspense>
3867 + <Suspense fallback={'[loading second]'}>
3868 + secondstart
3869 + <b>
3870 + <AsyncText text={'second suspended'} />
3871 + </b>
3872 + </Suspense>
3873 + end
3874 + </div>
3875 + </Suspense>
3876 + );
3877 + }
3878 +
3879 + await act(async () => {
3880 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
3881 + await afterImmediate();
3882 + await act(() => resolveText('world'));
3883 + pipe(writable);
3884 + });
3885 +
3886 + expect(document.getElementById('app-div').outerHTML).toEqual(
3887 + '<div id="app-div">start<!--$?--><template id="B:0"></template>[loading first]<!--/$--><!--$?--><template id="B:1"></template>[loading second]<!--/$-->end</div>',
3888 + );
3889 +
3890 + await act(async () => {
3891 + resolveText('first suspended');
3892 + });
3893 +
3894 + expect(document.getElementById('app-div').outerHTML).toEqual(
3895 + '<div id="app-div">start<!--$-->firststartfirst suspendedfirstend<!--/$--><!--$?--><template id="B:1"></template>[loading second]<!--/$-->end</div>',
3896 + );
3897 +
3898 + const errors = [];
3899 + ReactDOMClient.hydrateRoot(container, <App />, {
3900 + onRecoverableError(error) {
3901 + errors.push(error.message);
3902 + },
3903 + });
3904 + expect(Scheduler).toFlushAndYield([]);
3905 + expect(errors).toEqual([]);
3906 + expect(getVisibleChildren(container)).toEqual(
3907 + <div id="app-div">
3908 + {'start'}
3909 + {'firststart'}
3910 + {'first suspended'}
3911 + {'firstend'}
3912 + {'[loading second]'}
3913 + {'end'}
3914 + </div>,
3915 + );
3916 +
3917 + await act(async () => {
3918 + resolveText('second suspended');
3919 + });
3920 +
3921 + expect(container.firstElementChild.outerHTML).toEqual(
3922 + '<div id="app-div">start<!--$-->firststartfirst suspendedfirstend<!--/$--><!--$-->secondstart<b>second suspended</b><!--/$-->end</div>',
3923 + );
3924 +
3925 + expect(Scheduler).toFlushAndYield([]);
3926 + expect(errors).toEqual([]);
3927 + expect(getVisibleChildren(container)).toEqual(
3928 + <div id="app-div">
3929 + {'start'}
3930 + {'firststart'}
3931 + {'first suspended'}
3932 + {'firstend'}
3933 + {'secondstart'}
3934 + <b>second suspended</b>
3935 + {'end'}
3936 + </div>,
3937 + );
3938 + });
3939 +
3940 + // @gate experimental
3941 + it('(only) includes extraneous text separators in segments that complete before flushing, followed by nothing or a non-Text node', async () => {
3942 + function App() {
3943 + return (
3944 + <div>
3945 + <Suspense fallback={'text before, nothing after...'}>
3946 + hello
3947 + <AsyncText text="world" />
3948 + </Suspense>
3949 + <Suspense fallback={'nothing before or after...'}>
3950 + <AsyncText text="world" />
3951 + </Suspense>
3952 + <Suspense fallback={'text before, element after...'}>
3953 + hello
3954 + <AsyncText text="world" />
3955 + <br />
3956 + </Suspense>
3957 + <Suspense fallback={'nothing before, element after...'}>
3958 + <AsyncText text="world" />
3959 + <br />
3960 + </Suspense>
3961 + </div>
3962 + );
3963 + }
3964 +
3965 + await act(async () => {
3966 + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
3967 + await afterImmediate();
3968 + await act(() => resolveText('world'));
3969 + pipe(writable);
3970 + });
3971 +
3972 + expect(container.innerHTML).toEqual(
3973 + '<div><!--$-->hello<!-- -->world<!-- --><!--/$--><!--$-->world<!-- --><!--/$--><!--$-->hello<!-- -->world<!-- --><br><!--/$--><!--$-->world<!-- --><br><!--/$--></div>',
3974 + );
3975 +
3976 + const errors = [];
3977 + ReactDOMClient.hydrateRoot(container, <App />, {
3978 + onRecoverableError(error) {
3979 + errors.push(error.message);
3980 + },
3981 + });
3982 + expect(Scheduler).toFlushAndYield([]);
3983 + expect(errors).toEqual([]);
3984 + expect(getVisibleChildren(container)).toEqual(
3985 + <div>
3986 + {/* first boundary */}
3987 + {'hello'}
3988 + {'world'}
3989 + {/* second boundary */}
3990 + {'world'}
3991 + {/* third boundary */}
3992 + {'hello'}
3993 + {'world'}
3994 + <br />
3995 + {/* fourth boundary */}
3996 + {'world'}
3997 + <br />
3998 + </div>,
3999 + );
4000 + });
4001 + });
4002 });
packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js
+25 -32
@@ -101,7 +101,7 @@ describe('ReactDOMServerIntegration', () => {
101 ) {
102 // For plain server markup result we have comments between.
103 // If we're able to hydrate, they remain.
104 - expect(e.childNodes.length).toBe(render === streamRender ? 6 : 5);
104 + expect(e.childNodes.length).toBe(5);
105 expectTextNode(e.childNodes[0], ' ');
106 expectTextNode(e.childNodes[2], ' ');
107 expectTextNode(e.childNodes[4], ' ');
@@ -119,8 +119,8 @@ describe('ReactDOMServerIntegration', () => {
119 Text<span>More Text</span>
120 </div>,
121 );
122 - expect(e.childNodes.length).toBe(render === streamRender ? 3 : 2);
123 - const spanNode = e.childNodes[render === streamRender ? 2 : 1];
122 + expect(e.childNodes.length).toBe(2);
123 + const spanNode = e.childNodes[1];
124 expectTextNode(e.childNodes[0], 'Text');
125 expect(spanNode.tagName).toBe('SPAN');
126 expect(spanNode.childNodes.length).toBe(1);
@@ -147,19 +147,19 @@ describe('ReactDOMServerIntegration', () => {
147 itRenders('a custom element with text', async render => {
148 const e = await render(<custom-element>Text</custom-element>);
149 expect(e.tagName).toBe('CUSTOM-ELEMENT');
150 - expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
150 + expect(e.childNodes.length).toBe(1);
151 expectNode(e.firstChild, TEXT_NODE_TYPE, 'Text');
152 });
153
154 itRenders('a leading blank child with a text sibling', async render => {
155 const e = await render(<div>{''}foo</div>);
156 - expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
156 + expect(e.childNodes.length).toBe(1);
157 expectTextNode(e.childNodes[0], 'foo');
158 });
159
160 itRenders('a trailing blank child with a text sibling', async render => {
161 const e = await render(<div>foo{''}</div>);
162 - expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
162 + expect(e.childNodes.length).toBe(1);
163 expectTextNode(e.childNodes[0], 'foo');
164 });
165
@@ -176,7 +176,7 @@ describe('ReactDOMServerIntegration', () => {
176 render === streamRender
177 ) {
178 // In the server render output there's a comment between them.
179 - expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
179 + expect(e.childNodes.length).toBe(3);
180 expectTextNode(e.childNodes[0], 'foo');
181 expectTextNode(e.childNodes[2], 'bar');
182 } else {
@@ -203,7 +203,7 @@ describe('ReactDOMServerIntegration', () => {
203 render === streamRender
204 ) {
205 // In the server render output there's a comment between them.
206 - expect(e.childNodes.length).toBe(render === streamRender ? 6 : 5);
206 + expect(e.childNodes.length).toBe(5);
207 expectTextNode(e.childNodes[0], 'a');
208 expectTextNode(e.childNodes[2], 'b');
209 expectTextNode(e.childNodes[4], 'c');
@@ -240,7 +240,11 @@ describe('ReactDOMServerIntegration', () => {
240 e
241 </div>,
242 );
243 - if (render === serverRender || render === clientRenderOnServerString) {
243 + if (
244 + render === serverRender ||
245 + render === streamRender ||
246 + render === clientRenderOnServerString
247 + ) {
248 // In the server render output there's comments between text nodes.
249 expect(e.childNodes.length).toBe(5);
250 expectTextNode(e.childNodes[0], 'a');
@@ -249,15 +253,6 @@ describe('ReactDOMServerIntegration', () => {
253 expectTextNode(e.childNodes[3].childNodes[0], 'c');
254 expectTextNode(e.childNodes[3].childNodes[2], 'd');
255 expectTextNode(e.childNodes[4], 'e');
252 - } else if (render === streamRender) {
253 - // In the server render output there's comments after each text node.
254 - expect(e.childNodes.length).toBe(7);
255 - expectTextNode(e.childNodes[0], 'a');
256 - expectTextNode(e.childNodes[2], 'b');
257 - expect(e.childNodes[4].childNodes.length).toBe(4);
258 - expectTextNode(e.childNodes[4].childNodes[0], 'c');
259 - expectTextNode(e.childNodes[4].childNodes[2], 'd');
260 - expectTextNode(e.childNodes[5], 'e');
256 } else {
257 expect(e.childNodes.length).toBe(4);
258 expectTextNode(e.childNodes[0], 'a');
@@ -296,7 +291,7 @@ describe('ReactDOMServerIntegration', () => {
291 render === streamRender
292 ) {
293 // In the server markup there's a comment between.
299 - expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
294 + expect(e.childNodes.length).toBe(3);
295 expectTextNode(e.childNodes[0], 'foo');
296 expectTextNode(e.childNodes[2], '40');
297 } else {
@@ -335,13 +330,13 @@ describe('ReactDOMServerIntegration', () => {
330
331 itRenders('null children as blank', async render => {
332 const e = await render(<div>{null}foo</div>);
338 - expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
333 + expect(e.childNodes.length).toBe(1);
334 expectTextNode(e.childNodes[0], 'foo');
335 });
336
337 itRenders('false children as blank', async render => {
338 const e = await render(<div>{false}foo</div>);
344 - expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
339 + expect(e.childNodes.length).toBe(1);
340 expectTextNode(e.childNodes[0], 'foo');
341 });
342
@@ -353,7 +348,7 @@ describe('ReactDOMServerIntegration', () => {
348 {false}
349 </div>,
350 );
356 - expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
351 + expect(e.childNodes.length).toBe(1);
352 expectTextNode(e.childNodes[0], 'foo');
353 });
354
@@ -740,10 +735,10 @@ describe('ReactDOMServerIntegration', () => {
735 </div>,
736 );
737 expect(e.id).toBe('parent');
743 - expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
738 + expect(e.childNodes.length).toBe(3);
739 const child1 = e.childNodes[0];
740 const textNode = e.childNodes[1];
746 - const child2 = e.childNodes[render === streamRender ? 3 : 2];
741 + const child2 = e.childNodes[2];
742 expect(child1.id).toBe('child1');
743 expect(child1.childNodes.length).toBe(0);
744 expectTextNode(textNode, ' ');
@@ -757,10 +752,10 @@ describe('ReactDOMServerIntegration', () => {
752 async render => {
753 // prettier-ignore
754 const e = await render(<div id="parent"> <div id="child" /> </div>); // eslint-disable-line no-multi-spaces
760 - expect(e.childNodes.length).toBe(render === streamRender ? 5 : 3);
755 + expect(e.childNodes.length).toBe(3);
756 const textNode1 = e.childNodes[0];
762 - const child = e.childNodes[render === streamRender ? 2 : 1];
763 - const textNode2 = e.childNodes[render === streamRender ? 3 : 2];
757 + const child = e.childNodes[1];
758 + const textNode2 = e.childNodes[2];
759 expect(e.id).toBe('parent');
760 expectTextNode(textNode1, ' ');
761 expect(child.id).toBe('child');
@@ -783,9 +778,7 @@ describe('ReactDOMServerIntegration', () => {
778 ) {
779 // For plain server markup result we have comments between.
780 // If we're able to hydrate, they remain.
786 - expect(parent.childNodes.length).toBe(
787 - render === streamRender ? 6 : 5,
788 - );
781 + expect(parent.childNodes.length).toBe(5);
782 expectTextNode(parent.childNodes[0], 'a');
783 expectTextNode(parent.childNodes[2], 'b');
784 expectTextNode(parent.childNodes[4], 'c');
@@ -817,7 +810,7 @@ describe('ReactDOMServerIntegration', () => {
810 render === clientRenderOnServerString ||
811 render === streamRender
812 ) {
820 - expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
813 + expect(e.childNodes.length).toBe(3);
814 expectTextNode(e.childNodes[0], '<span>Text1&quot;</span>');
815 expectTextNode(e.childNodes[2], '<span>Text2&quot;</span>');
816 } else {
@@ -868,7 +861,7 @@ describe('ReactDOMServerIntegration', () => {
861 );
862 if (render === serverRender || render === streamRender) {
863 // We have three nodes because there is a comment between them.
871 - expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
864 + expect(e.childNodes.length).toBe(3);
865 // Everything becomes LF when parsed from server HTML.
866 // Null character is ignored.
867 expectNode(e.childNodes[0], TEXT_NODE_TYPE, 'foo\nbar');
packages/react-dom/src/__tests__/ReactDOMUseId-test.js
-2
@@ -343,7 +343,6 @@ describe('useId', () => {
343 id="container"
344 >
345 :R0:, :R0H1:, :R0H2:
346 - <!-- -->
346 </div>
347 `);
348 });
@@ -369,7 +368,6 @@ describe('useId', () => {
368 id="container"
369 >
370 :R0:
372 - <!-- -->
371 </div>
372 `);
373 });
packages/react-dom/src/server/ReactDOMLegacyServerStreamConfig.js
-11
@@ -24,7 +24,6 @@ export function flushBuffered(destination: Destination) {}
24
25 export function beginWriting(destination: Destination) {}
26
27 -let prevWasCommentSegmenter = false;
27 export function writeChunk(
28 destination: Destination,
29 chunk: Chunk | PrecomputedChunk,
@@ -36,16 +35,6 @@ export function writeChunkAndReturn(
35 destination: Destination,
36 chunk: Chunk | PrecomputedChunk,
37 ): boolean {
39 - if (prevWasCommentSegmenter) {
40 - prevWasCommentSegmenter = false;
41 - if (chunk[0] !== '<') {
42 - destination.push('<!-- -->');
43 - }
44 - }
45 - if (chunk === '<!-- -->') {
46 - prevWasCommentSegmenter = true;
47 - return true;
48 - }
38 return destination.push(chunk);
39 }
40
packages/react-dom/src/server/ReactDOMServerFormatConfig.js
+21 -4
@@ -284,13 +284,30 @@ export function pushTextInstance(
284 target: Array<Chunk | PrecomputedChunk>,
285 text: string,
286 responseState: ResponseState,
287 -): void {
287 + textEmbedded: boolean,
288 +): boolean {
289 if (text === '') {
290 // Empty text doesn't have a DOM node representation and the hydration is aware of this.
290 - return;
291 + return textEmbedded;
292 + }
293 + if (textEmbedded) {
294 + target.push(textSeparator);
295 + }
296 + target.push(stringToChunk(encodeHTMLTextNode(text)));
297 + return true;
298 +}
299 +
300 +// Called when Fizz is done with a Segment. Currently the only purpose is to conditionally
301 +// emit a text separator when we don't know for sure it is safe to omit
302 +export function pushSegmentFinale(
303 + target: Array<Chunk | PrecomputedChunk>,
304 + responseState: ResponseState,
305 + lastPushedText: boolean,
306 + textEmbedded: boolean,
307 +): void {
308 + if (lastPushedText && textEmbedded) {
309 + target.push(textSeparator);
310 }
292 - // TODO: Avoid adding a text separator in common cases.
293 - target.push(stringToChunk(encodeHTMLTextNode(text)), textSeparator);
311 }
312
313 const styleNameCache: Map<string, PrecomputedChunk> = new Map();
packages/react-dom/src/server/ReactDOMServerLegacyFormatConfig.js
+23 -2
@@ -12,6 +12,7 @@ import type {FormatContext} from './ReactDOMServerFormatConfig';
12 import {
13 createResponseState as createResponseStateImpl,
14 pushTextInstance as pushTextInstanceImpl,
15 + pushSegmentFinale as pushSegmentFinaleImpl,
16 writeStartCompletedSuspenseBoundary as writeStartCompletedSuspenseBoundaryImpl,
17 writeStartClientRenderedSuspenseBoundary as writeStartClientRenderedSuspenseBoundaryImpl,
18 writeEndCompletedSuspenseBoundary as writeEndCompletedSuspenseBoundaryImpl,
@@ -105,11 +106,31 @@ export function pushTextInstance(
106 target: Array<Chunk | PrecomputedChunk>,
107 text: string,
108 responseState: ResponseState,
108 -): void {
109 + textEmbedded: boolean,
110 +): boolean {
111 if (responseState.generateStaticMarkup) {
112 target.push(stringToChunk(escapeTextForBrowser(text)));
113 + return false;
114 + } else {
115 + return pushTextInstanceImpl(target, text, responseState, textEmbedded);
116 + }
117 +}
118 +
119 +export function pushSegmentFinale(
120 + target: Array<Chunk | PrecomputedChunk>,
121 + responseState: ResponseState,
122 + lastPushedText: boolean,
123 + textEmbedded: boolean,
124 +): void {
125 + if (responseState.generateStaticMarkup) {
126 + return;
127 } else {
112 - pushTextInstanceImpl(target, text, responseState);
128 + return pushSegmentFinaleImpl(
129 + target,
130 + responseState,
131 + lastPushedText,
132 + textEmbedded,
133 + );
134 }
135 }
136
packages/react-native-renderer/src/server/ReactNativeServerFormatConfig.js
+12 -1
@@ -122,7 +122,9 @@ export function pushTextInstance(
122 target: Array<Chunk | PrecomputedChunk>,
123 text: string,
124 responseState: ResponseState,
125 -): void {
125 + // This Renderer does not use this argument
126 + textEmbedded: boolean,
127 +): boolean {
128 target.push(
129 INSTANCE,
130 RAW_TEXT, // Type
@@ -130,6 +132,7 @@ export function pushTextInstance(
132 // TODO: props { text: text }
133 END, // End of children
134 );
135 + return false;
136 }
137
138 export function pushStartInstance(
@@ -156,6 +159,14 @@ export function pushEndInstance(
159 target.push(END);
160 }
161
162 +// In this Renderer this is a noop
163 +export function pushSegmentFinale(
164 + target: Array<Chunk | PrecomputedChunk>,
165 + responseState: ResponseState,
166 + lastPushedText: boolean,
167 + textEmbedded: boolean,
168 +): void {}
169 +
170 export function writeCompletedRoot(
171 destination: Destination,
172 responseState: ResponseState,
packages/react-noop-renderer/src/ReactNoopServer.js
+15 -1
@@ -98,12 +98,18 @@ const ReactNoopServer = ReactFizzServer({
98 return null;
99 },
100
101 - pushTextInstance(target: Array<Uint8Array>, text: string): void {
101 + pushTextInstance(
102 + target: Array<Uint8Array>,
103 + text: string,
104 + responseState: ResponseState,
105 + textEmbedded: boolean,
106 + ): boolean {
107 const textInstance: TextInstance = {
108 text,
109 hidden: false,
110 };
111 target.push(Buffer.from(JSON.stringify(textInstance), 'utf8'), POP);
112 + return false;
113 },
114 pushStartInstance(
115 target: Array<Uint8Array>,
@@ -128,6 +134,14 @@ const ReactNoopServer = ReactFizzServer({
134 target.push(POP);
135 },
136
137 + // This is a noop in ReactNoop
138 + pushSegmentFinale(
139 + target: Array<Uint8Array>,
140 + responseState: ResponseState,
141 + lastPushedText: boolean,
142 + textEmbedded: boolean,
143 + ): void {},
144 +
145 writeCompletedRoot(
146 destination: Destination,
147 responseState: ResponseState,
packages/react-server-dom-relay/src/__tests__/ReactDOMServerFB-test.internal.js
+1 -3
@@ -93,9 +93,7 @@ describe('ReactDOMServerFB', () => {
93 await jest.runAllTimers();
94
95 const result = readResult(stream);
96 - expect(result).toMatchInlineSnapshot(
97 - `"<div><!--$-->Done<!-- --><!--/$--></div>"`,
98 - );
96 + expect(result).toMatchInlineSnapshot(`"<div><!--$-->Done<!--/$--></div>"`);
97 });
98
99 it('should throw an error when an error is thrown at the root', () => {
packages/react-server/src/ReactFizzServer.js
+59 -3
@@ -56,6 +56,7 @@ import {
56 pushEndInstance,
57 pushStartCompletedSuspenseBoundary,
58 pushEndCompletedSuspenseBoundary,
59 + pushSegmentFinale,
60 UNINITIALIZED_SUSPENSE_BOUNDARY_ID,
61 assignSuspenseBoundaryID,
62 getChildFormatContext,
@@ -169,6 +170,9 @@ type Segment = {
170 formatContext: FormatContext,
171 // If this segment represents a fallback, this is the content that will replace that fallback.
172 +boundary: null | SuspenseBoundary,
173 + // used to discern when text separator boundaries are needed
174 + lastPushedText: boolean,
175 + textEmbedded: boolean,
176 };
177
178 const OPEN = 0;
@@ -267,7 +271,15 @@ export function createRequest(
271 onFatalError: onFatalError === undefined ? noop : onFatalError,
272 };
273 // This segment represents the root fallback.
270 - const rootSegment = createPendingSegment(request, 0, null, rootFormatContext);
274 + const rootSegment = createPendingSegment(
275 + request,
276 + 0,
277 + null,
278 + rootFormatContext,
279 + // Root segments are never embedded in Text on either edge
280 + false,
281 + false,
282 + );
283 // There is no parent so conceptually, we're unblocked to flush this segment.
284 rootSegment.parentFlushed = true;
285 const rootTask = createTask(
@@ -346,6 +358,8 @@ function createPendingSegment(
358 index: number,
359 boundary: null | SuspenseBoundary,
360 formatContext: FormatContext,
361 + lastPushedText: boolean,
362 + textEmbedded: boolean,
363 ): Segment {
364 return {
365 status: PENDING,
@@ -356,6 +370,8 @@ function createPendingSegment(
370 children: [],
371 formatContext,
372 boundary,
373 + lastPushedText,
374 + textEmbedded,
375 };
376 }
377
@@ -459,8 +475,13 @@ function renderSuspenseBoundary(
475 insertionIndex,
476 newBoundary,
477 parentSegment.formatContext,
478 + // boundaries never require text embedding at their edges because comment nodes bound them
479 + false,
480 + false,
481 );
482 parentSegment.children.push(boundarySegment);
483 + // The parentSegment has a child Segment at this index so we reset the lastPushedText marker on the parent
484 + parentSegment.lastPushedText = false;
485
486 // This segment is the actual child content. We can start rendering that immediately.
487 const contentRootSegment = createPendingSegment(
@@ -468,6 +489,9 @@ function renderSuspenseBoundary(
489 0,
490 null,
491 parentSegment.formatContext,
492 + // boundaries never require text embedding at their edges because comment nodes bound them
493 + false,
494 + false,
495 );
496 // We mark the root segment as having its parent flushed. It's not really flushed but there is
497 // no parent segment so there's nothing to wait on.
@@ -486,6 +510,12 @@ function renderSuspenseBoundary(
510 try {
511 // We use the safe form because we don't handle suspending here. Only error handling.
512 renderNode(request, task, content);
513 + pushSegmentFinale(
514 + contentRootSegment.chunks,
515 + request.responseState,
516 + contentRootSegment.lastPushedText,
517 + contentRootSegment.textEmbedded,
518 + );
519 contentRootSegment.status = COMPLETED;
520 queueCompletedSegment(newBoundary, contentRootSegment);
521 if (newBoundary.pendingTasks === 0) {
@@ -561,15 +591,18 @@ function renderHostElement(
591 request.responseState,
592 segment.formatContext,
593 );
594 + segment.lastPushedText = false;
595 const prevContext = segment.formatContext;
596 segment.formatContext = getChildFormatContext(prevContext, type, props);
597 // We use the non-destructive form because if something suspends, we still
598 // need to pop back up and finish this subtree of HTML.
599 renderNode(request, task, children);
600 +
601 // We expect that errors will fatal the whole task and that we don't need
602 // the correct context. Therefore this is not in a finally.
603 segment.formatContext = prevContext;
604 pushEndInstance(segment.chunks, type, props);
605 + segment.lastPushedText = false;
606 popComponentStackInDEV(task);
607 }
608
@@ -1216,15 +1249,23 @@ function renderNodeDestructive(
1249 }
1250
1251 if (typeof node === 'string') {
1219 - pushTextInstance(task.blockedSegment.chunks, node, request.responseState);
1252 + const segment = task.blockedSegment;
1253 + segment.lastPushedText = pushTextInstance(
1254 + task.blockedSegment.chunks,
1255 + node,
1256 + request.responseState,
1257 + segment.lastPushedText,
1258 + );
1259 return;
1260 }
1261
1262 if (typeof node === 'number') {
1224 - pushTextInstance(
1263 + const segment = task.blockedSegment;
1264 + segment.lastPushedText = pushTextInstance(
1265 task.blockedSegment.chunks,
1266 '' + node,
1267 request.responseState,
1268 + segment.lastPushedText,
1269 );
1270 return;
1271 }
@@ -1268,8 +1309,14 @@ function spawnNewSuspendedTask(
1309 insertionIndex,
1310 null,
1311 segment.formatContext,
1312 + // Adopt the parent segment's leading text embed
1313 + segment.lastPushedText,
1314 + // Assume we are text embedded at the trailing edge
1315 + true,
1316 );
1317 segment.children.push(newSegment);
1318 + // Reset lastPushedText for current Segment since the new Segment "consumed" it
1319 + segment.lastPushedText = false;
1320 const newTask = createTask(
1321 request,
1322 task.node,
@@ -1545,6 +1592,12 @@ function retryTask(request: Request, task: Task): void {
1592 // We call the destructive form that mutates this task. That way if something
1593 // suspends again, we can reuse the same task instead of spawning a new one.
1594 renderNodeDestructive(request, task, task.node);
1595 + pushSegmentFinale(
1596 + segment.chunks,
1597 + request.responseState,
1598 + segment.lastPushedText,
1599 + segment.textEmbedded,
1600 + );
1601
1602 task.abortSet.delete(task);
1603 segment.status = COMPLETED;
@@ -1625,6 +1678,9 @@ function flushSubtree(
1678 // We're emitting a placeholder for this segment to be filled in later.
1679 // Therefore we'll need to assign it an ID - to refer to it by.
1680 const segmentID = (segment.id = request.nextSegmentId++);
1681 + // When this segment finally completes it won't be embedded in text since it will flush separately
1682 + segment.lastPushedText = false;
1683 + segment.textEmbedded = false;
1684 return writePlaceholder(destination, request.responseState, segmentID);
1685 }
1686 case COMPLETED: {
packages/react-server/src/forks/ReactServerFormatConfig.custom.js
+1
@@ -43,6 +43,7 @@ export const pushStartCompletedSuspenseBoundary =
43 $$$hostConfig.pushStartCompletedSuspenseBoundary;
44 export const pushEndCompletedSuspenseBoundary =
45 $$$hostConfig.pushEndCompletedSuspenseBoundary;
46 +export const pushSegmentFinale = $$$hostConfig.pushSegmentFinale;
47 export const writeCompletedRoot = $$$hostConfig.writeCompletedRoot;
48 export const writePlaceholder = $$$hostConfig.writePlaceholder;
49 export const writeStartCompletedSuspenseBoundary =