Skip to content

Commit d94977a

Browse files
gnoffgnoff
committed
[Fizz] deterministic text separators
This change addresses two related flaws in the current approach to text separators in Fizz. First text separators can be emitted more often than necessary. This is because Segments don't know whether they will be followed by text and so if they end in text they assume they are and insert a separators Second, becuase of the flaw above, Fizz output is not deterministic. If you wait for everything to be ready before flushing you could still end up with different output depending on whether certain segments are created or rendered inline based on the particular behavior of Suspendable components for any given run. If a Segment emits an unecessary text separator in one pass and in another the Segment is never created because that Component did not Suspend the string output would differ for otherwise identical content This change fixes both issues by decorating Segments with additional metadata about what kinds of emitted content is at it's bounds. These are called Edges and can be NODE, TEXT, or a Segment. NODE and TEXT are type identifiers that refer generically to these mutually exclusive concepts. if an edge is a Segment however it is the actual reference to the Segment object, which allows us to make inferrences at flushing time as to whether additional text separators are needed. There are 2 tracked edges per Segment currentEdge - a pointer to type of the prior thing emitted for a Segment. If the segment has not yet been worked on this will be the Edge just before the Segment start. If the Segment is currently being rendered it will be the type or reference of the last thing this Segment emitted. If the Segment is complete it definitionally be the trailing edge of the Segment representing the type emitted at the boundary. followingEdge - this edge is set by later Segments when the currentEdge of that segment is a Segment. If the currentEdge is a Segment and a new Edge is identified it is saved to the followingEdge of the currentEdge segment. Finally, when flushing 2 things happen If the just written Segment ends with text (currentEdge is TEXT_NODE) and if the followingEdge is also text, then a separator is written in between. If the followingEdge is instead another Completed Segment this segment is empty and we walk to that Segment's followingEdge and continue looking for Text.
1 parent a276638 commit d94977a

9 files changed

+479
-110
lines changed

‎packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

+360-3
Original file line numberDiff line numberDiff line change
@@ -3795,7 +3795,7 @@ describe('ReactDOMFizzServer', () => {
37953795
});
37963796

37973797
expect(container.firstElementChild.outerHTML).toEqual(
3798-
'<div>hello<b>world<!-- --></b></div>',
3798+
'<div>hello<b>world</b></div>',
37993799
);
38003800

38013801
const errors = [];
@@ -3938,7 +3938,9 @@ describe('ReactDOMFizzServer', () => {
39383938
});
39393939

39403940
// @gate experimental
3941-
it('(only) includes extraneous text separators in segments that complete before flushing, followed by nothing or a non-Text node', async () => {
3941+
it('excludes extraneous text separators in segments that complete before flushing, followed by nothing or a non-Text node', async () => {
3942+
// @TODO This test no longer makes sense because we do not ever emit extraneous text separators regardless of how things flush. rewrite to assert
3943+
// this new behavior and describe it better
39423944
function App() {
39433945
return (
39443946
<div>
@@ -3970,7 +3972,7 @@ describe('ReactDOMFizzServer', () => {
39703972
});
39713973

39723974
expect(container.innerHTML).toEqual(
3973-
'<div><!--$-->hello<!-- -->world<!-- --><!--/$--><!--$-->world<!-- --><!--/$--><!--$-->hello<!-- -->world<!-- --><br><!--/$--><!--$-->world<!-- --><br><!--/$--></div>',
3975+
'<div><!--$-->hello<!-- -->world<!--/$--><!--$-->world<!--/$--><!--$-->hello<!-- -->world<br><!--/$--><!--$-->world<br><!--/$--></div>',
39743976
);
39753977

39763978
const errors = [];
@@ -3998,5 +4000,360 @@ describe('ReactDOMFizzServer', () => {
39984000
</div>,
39994001
);
40004002
});
4003+
4004+
// @gate experimental
4005+
it('handles many serial adjacent segments that resolves in arbitrary order', async () => {
4006+
function NineText() {
4007+
return (
4008+
<>
4009+
<ThreeText start={1} />
4010+
<ThreeText start={4} />
4011+
<ThreeText start={7} />
4012+
</>
4013+
);
4014+
}
4015+
4016+
function ThreeText({start}) {
4017+
return (
4018+
<>
4019+
<AsyncText text={start} />
4020+
<AsyncText text={start + 1} />
4021+
<AsyncText text={start + 2} />
4022+
</>
4023+
);
4024+
}
4025+
4026+
function App() {
4027+
return (
4028+
<div>
4029+
<Suspense>
4030+
<NineText />
4031+
</Suspense>
4032+
</div>
4033+
);
4034+
}
4035+
4036+
await act(async () => {
4037+
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
4038+
await act(() => resolveText(1));
4039+
await act(() => resolveText(6));
4040+
await act(() => resolveText(9));
4041+
await afterImmediate();
4042+
await act(() => resolveText(2));
4043+
await act(() => resolveText(5));
4044+
await act(() => resolveText(7));
4045+
pipe(writable);
4046+
});
4047+
4048+
expect(container.innerHTML).toEqual(
4049+
'<div><!--$?--><template id="B:0"></template><!--/$--></div><div hidden="" id="S:0">1<!-- -->2<template id="P:1"></template><template id="P:2"></template>5<!-- -->6<!-- -->7<template id="P:3"></template>9</div>',
4050+
);
4051+
4052+
await act(async () => {
4053+
resolveText(3);
4054+
resolveText(4);
4055+
resolveText(8);
4056+
});
4057+
4058+
expect(container.firstElementChild.outerHTML).toEqual(
4059+
'<div><!--$-->1<!-- -->2345<!-- -->6<!-- -->789<!--/$--></div>',
4060+
);
4061+
4062+
const errors = [];
4063+
ReactDOMClient.hydrateRoot(container, <App />, {
4064+
onRecoverableError(error) {
4065+
errors.push(error.message);
4066+
},
4067+
});
4068+
expect(Scheduler).toFlushAndYield([]);
4069+
expect(errors).toEqual([]);
4070+
expect(getVisibleChildren(container)).toEqual(
4071+
<div>
4072+
{'1'}
4073+
{'2'}
4074+
{'3'}
4075+
{'4'}
4076+
{'5'}
4077+
{'6'}
4078+
{'7'}
4079+
{'8'}
4080+
{'9'}
4081+
</div>,
4082+
);
4083+
});
4084+
4085+
// @gate experimental
4086+
it('handles deeply nested segments that resolves in arbitrary order', async () => {
4087+
function RecursiveNumber({from, steps, reverse}) {
4088+
if (steps === 1) {
4089+
return readText(from);
4090+
}
4091+
4092+
const num = readText(from);
4093+
4094+
return (
4095+
<>
4096+
{num}
4097+
<RecursiveNumber
4098+
from={reverse ? from - 1 : from + 1}
4099+
steps={steps - 1}
4100+
reverse={reverse}
4101+
/>
4102+
</>
4103+
);
4104+
}
4105+
4106+
function App() {
4107+
return (
4108+
<div>
4109+
<Suspense>
4110+
<RecursiveNumber from={1} steps={3} />
4111+
<RecursiveNumber from={6} steps={3} reverse={true} />
4112+
</Suspense>
4113+
</div>
4114+
);
4115+
}
4116+
4117+
await act(async () => {
4118+
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
4119+
await afterImmediate();
4120+
await act(() => resolveText(1));
4121+
await act(() => resolveText(2));
4122+
await act(() => resolveText(4));
4123+
4124+
pipe(writable);
4125+
});
4126+
4127+
expect(container.innerHTML).toEqual(
4128+
'<div><!--$?--><template id="B:0"></template><!--/$--></div><div hidden="" id="S:0">1<!-- -->2<template id="P:1"></template><template id="P:2"></template></div>',
4129+
);
4130+
4131+
await act(async () => {
4132+
resolveText(3);
4133+
resolveText(5);
4134+
resolveText(6);
4135+
});
4136+
4137+
expect(container.firstElementChild.outerHTML).toEqual(
4138+
'<div><!--$-->1<!-- -->236<!-- -->5<!-- -->4<!--/$--></div>',
4139+
);
4140+
4141+
const errors = [];
4142+
ReactDOMClient.hydrateRoot(container, <App />, {
4143+
onRecoverableError(error) {
4144+
errors.push(error.message);
4145+
},
4146+
});
4147+
expect(Scheduler).toFlushAndYield([]);
4148+
expect(errors).toEqual([]);
4149+
expect(getVisibleChildren(container)).toEqual(
4150+
<div>
4151+
{'1'}
4152+
{'2'}
4153+
{'3'}
4154+
{'6'}
4155+
{'5'}
4156+
{'4'}
4157+
</div>,
4158+
);
4159+
});
4160+
4161+
// @gate experimental
4162+
it('handles segments that return null', async () => {
4163+
function WrappedAsyncText({outer, text}) {
4164+
readText(outer);
4165+
return <AsyncText text={text} />;
4166+
}
4167+
4168+
function App() {
4169+
return (
4170+
<div>
4171+
<Suspense>
4172+
<div>
4173+
<AsyncText text={null} />
4174+
<AsyncText text={'hello'} />
4175+
<AsyncText text={'world'} />
4176+
</div>
4177+
<div>
4178+
<AsyncText text={'hello'} />
4179+
<AsyncText text={null} />
4180+
<AsyncText text={'world'} />
4181+
</div>
4182+
<div>
4183+
<AsyncText text={'hello'} />
4184+
<AsyncText text={'world'} />
4185+
<AsyncText text={null} />
4186+
</div>
4187+
<div>
4188+
<AsyncText text={'hello'} />
4189+
<AsyncText text={null} />
4190+
<AsyncText text={null} />
4191+
<AsyncText text={'world'} />
4192+
</div>
4193+
<div>
4194+
<AsyncText text={'hello'} />
4195+
<WrappedAsyncText outer={'outer1'} text={null} />
4196+
<AsyncText text={null} />
4197+
<AsyncText text={'world'} />
4198+
</div>
4199+
<div>
4200+
<WrappedAsyncText outer={'outer1'} text={'hello'} />
4201+
<WrappedAsyncText outer={'outer2'} text={null} />
4202+
<AsyncText text={'world'} />
4203+
</div>
4204+
</Suspense>
4205+
</div>
4206+
);
4207+
}
4208+
4209+
await act(async () => {
4210+
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
4211+
await afterImmediate();
4212+
await act(() => resolveText('outer2'));
4213+
await act(() => resolveText('world'));
4214+
await act(() => resolveText('outer1'));
4215+
await act(() => resolveText(null));
4216+
await act(() => resolveText('hello'));
4217+
4218+
pipe(writable);
4219+
});
4220+
4221+
let helloWorld = '<div>hello<!-- -->world</div>';
4222+
let testcases = 6;
4223+
4224+
expect(container.firstElementChild.outerHTML).toEqual(
4225+
'<div><!--$-->' +
4226+
new Array(testcases).fill(helloWorld).join('') +
4227+
'<!--/$--></div>',
4228+
);
4229+
4230+
const errors = [];
4231+
ReactDOMClient.hydrateRoot(container, <App />, {
4232+
onRecoverableError(error) {
4233+
errors.push(error.message);
4234+
},
4235+
});
4236+
expect(Scheduler).toFlushAndYield([]);
4237+
expect(errors).toEqual([]);
4238+
const assertion = () => {
4239+
expect(getVisibleChildren(container)).toEqual(
4240+
<div>
4241+
{new Array(testcases).fill(
4242+
<div>
4243+
{'hello'}
4244+
{'world'}
4245+
</div>,
4246+
)}
4247+
</div>,
4248+
);
4249+
};
4250+
if (__DEV__) {
4251+
expect(assertion).toErrorDev([
4252+
'Warning: Each child in a list should have a unique "key" prop.',
4253+
]);
4254+
} else {
4255+
assertion();
4256+
}
4257+
});
4258+
4259+
// @gate experimental
4260+
it('does not add separators when otherwise adjacent text is wrapped in Suspense', async () => {
4261+
function App() {
4262+
return (
4263+
<div>
4264+
hello
4265+
<Suspense>
4266+
<AsyncText text={'world'} />
4267+
</Suspense>
4268+
</div>
4269+
);
4270+
}
4271+
4272+
await act(async () => {
4273+
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
4274+
await afterImmediate();
4275+
await act(() => resolveText('world'));
4276+
4277+
pipe(writable);
4278+
});
4279+
4280+
expect(container.firstElementChild.outerHTML).toEqual(
4281+
'<div>hello<!--$-->world<!--/$--></div>',
4282+
);
4283+
4284+
const errors = [];
4285+
ReactDOMClient.hydrateRoot(container, <App />, {
4286+
onRecoverableError(error) {
4287+
errors.push(error.message);
4288+
},
4289+
});
4290+
expect(Scheduler).toFlushAndYield([]);
4291+
expect(errors).toEqual([]);
4292+
expect(getVisibleChildren(container)).toEqual(
4293+
<div>
4294+
{'hello'}
4295+
{'world'}
4296+
</div>,
4297+
);
4298+
});
4299+
4300+
// @gate experimental
4301+
it('does not prepend separators for Suspense fallback text but will append them if followed by text', async () => {
4302+
function App() {
4303+
return (
4304+
<div>
4305+
<Suspense fallback={'outer'}>
4306+
hello
4307+
<Suspense
4308+
fallback={
4309+
<>
4310+
<AsyncText text={'world'} />!<AsyncText text={'foo'} />
4311+
</>
4312+
}>
4313+
<AsyncText text={'bar'} />
4314+
</Suspense>
4315+
!
4316+
</Suspense>
4317+
</div>
4318+
);
4319+
}
4320+
4321+
await act(async () => {
4322+
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
4323+
await afterImmediate();
4324+
await act(() => resolveText('foo'));
4325+
pipe(writable);
4326+
});
4327+
4328+
expect(container.innerHTML).toEqual(
4329+
'<div><!--$?--><template id="B:0"></template>outer<!--/$--></div><div hidden="" id="S:0">hello<!--$?--><template id="B:1"></template><template id="P:2"></template>!<!-- -->foo<!--/$-->!</div>',
4330+
);
4331+
4332+
await act(() => resolveText('world'));
4333+
4334+
expect(container.children[0].outerHTML).toEqual(
4335+
'<div><!--$-->hello<!--$?--><template id="B:1"></template>world!<!-- -->foo<!--/$-->!<!--/$--></div>',
4336+
);
4337+
4338+
const errors = [];
4339+
ReactDOMClient.hydrateRoot(container, <App />, {
4340+
onRecoverableError(error) {
4341+
errors.push(error.message);
4342+
},
4343+
});
4344+
expect(Scheduler).toFlushAndYield([]);
4345+
expect(errors).toEqual([]);
4346+
expect(getVisibleChildren(container)).toEqual(
4347+
<div>
4348+
{'hello'}
4349+
{/* starting the inner Suspense boundary Fallback */}
4350+
{'world'}
4351+
{'!'}
4352+
{'foo'}
4353+
{/* ending the inner Suspense boundary Fallback */}
4354+
{'!'}
4355+
</div>,
4356+
);
4357+
});
40014358
});
40024359
});

0 commit comments

Comments
 (0)