Skip to content

Commit

Permalink
[Fizz] deterministic text separators
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@gnoff
gnoff committed May 29, 2022
1 parent a276638 commit e85a292
Show file tree
Hide file tree
Showing 9 changed files with 477 additions and 110 deletions.
361 changes: 358 additions & 3 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3795,7 +3795,7 @@ describe('ReactDOMFizzServer', () => {
});

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

const errors = [];
Expand Down Expand Up @@ -3938,7 +3938,7 @@ describe('ReactDOMFizzServer', () => {
});

// @gate experimental
it('(only) includes extraneous text separators in segments that complete before flushing, followed by nothing or a non-Text node', async () => {
it('excludes extraneous text separators in segments that complete before flushing, followed by nothing or a non-Text node', async () => {
function App() {
return (
<div>
Expand Down Expand Up @@ -3970,7 +3970,7 @@ describe('ReactDOMFizzServer', () => {
});

expect(container.innerHTML).toEqual(
'<div><!--$-->hello<!-- -->world<!-- --><!--/$--><!--$-->world<!-- --><!--/$--><!--$-->hello<!-- -->world<!-- --><br><!--/$--><!--$-->world<!-- --><br><!--/$--></div>',
'<div><!--$-->hello<!-- -->world<!--/$--><!--$-->world<!--/$--><!--$-->hello<!-- -->world<br><!--/$--><!--$-->world<br><!--/$--></div>',
);

const errors = [];
Expand Down Expand Up @@ -3998,5 +3998,360 @@ describe('ReactDOMFizzServer', () => {
</div>,
);
});

// @gate experimental
it('handles many serial adjacent segments that resolves in arbitrary order', async () => {
function NineText() {
return (
<>
<ThreeText start={1} />
<ThreeText start={4} />
<ThreeText start={7} />
</>
);
}

function ThreeText({start}) {
return (
<>
<AsyncText text={start} />
<AsyncText text={start + 1} />
<AsyncText text={start + 2} />
</>
);
}

function App() {
return (
<div>
<Suspense>
<NineText />
</Suspense>
</div>
);
}

await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
await act(() => resolveText(1));
await act(() => resolveText(6));
await act(() => resolveText(9));
await afterImmediate();
await act(() => resolveText(2));
await act(() => resolveText(5));
await act(() => resolveText(7));
pipe(writable);
});

expect(container.innerHTML).toEqual(
'<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>',
);

await act(async () => {
resolveText(3);
resolveText(4);
resolveText(8);
});

expect(container.firstElementChild.outerHTML).toEqual(
'<div><!--$-->1<!-- -->2345<!-- -->6<!-- -->789<!--/$--></div>',
);

const errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
errors.push(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
expect(errors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(
<div>
{'1'}
{'2'}
{'3'}
{'4'}
{'5'}
{'6'}
{'7'}
{'8'}
{'9'}
</div>,
);
});

// @gate experimental
it('handles deeply nested segments that resolves in arbitrary order', async () => {
function RecursiveNumber({from, steps, reverse}) {
if (steps === 1) {
return readText(from);
}

const num = readText(from);

return (
<>
{num}
<RecursiveNumber
from={reverse ? from - 1 : from + 1}
steps={steps - 1}
reverse={reverse}
/>
</>
);
}

function App() {
return (
<div>
<Suspense>
<RecursiveNumber from={1} steps={3} />
<RecursiveNumber from={6} steps={3} reverse={true} />
</Suspense>
</div>
);
}

await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
await afterImmediate();
await act(() => resolveText(1));
await act(() => resolveText(2));
await act(() => resolveText(4));

pipe(writable);
});

expect(container.innerHTML).toEqual(
'<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>',
);

await act(async () => {
resolveText(3);
resolveText(5);
resolveText(6);
});

expect(container.firstElementChild.outerHTML).toEqual(
'<div><!--$-->1<!-- -->236<!-- -->5<!-- -->4<!--/$--></div>',
);

const errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
errors.push(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
expect(errors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(
<div>
{'1'}
{'2'}
{'3'}
{'6'}
{'5'}
{'4'}
</div>,
);
});

// @gate experimental
it('handles segments that return null', async () => {
function WrappedAsyncText({outer, text}) {
readText(outer);
return <AsyncText text={text} />;
}

function App() {
return (
<div>
<Suspense>
<div>
<AsyncText text={null} />
<AsyncText text={'hello'} />
<AsyncText text={'world'} />
</div>
<div>
<AsyncText text={'hello'} />
<AsyncText text={null} />
<AsyncText text={'world'} />
</div>
<div>
<AsyncText text={'hello'} />
<AsyncText text={'world'} />
<AsyncText text={null} />
</div>
<div>
<AsyncText text={'hello'} />
<AsyncText text={null} />
<AsyncText text={null} />
<AsyncText text={'world'} />
</div>
<div>
<AsyncText text={'hello'} />
<WrappedAsyncText outer={'outer1'} text={null} />
<AsyncText text={null} />
<AsyncText text={'world'} />
</div>
<div>
<WrappedAsyncText outer={'outer1'} text={'hello'} />
<WrappedAsyncText outer={'outer2'} text={null} />
<AsyncText text={'world'} />
</div>
</Suspense>
</div>
);
}

await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
await afterImmediate();
await act(() => resolveText('outer2'));
await act(() => resolveText('world'));
await act(() => resolveText('outer1'));
await act(() => resolveText(null));
await act(() => resolveText('hello'));

pipe(writable);
});

let helloWorld = '<div>hello<!-- -->world</div>';
let testcases = 6;

expect(container.firstElementChild.outerHTML).toEqual(
'<div><!--$-->' +
new Array(testcases).fill(helloWorld).join('') +
'<!--/$--></div>',
);

const errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
errors.push(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
expect(errors).toEqual([]);
const assertion = () => {
expect(getVisibleChildren(container)).toEqual(
<div>
{new Array(testcases).fill(
<div>
{'hello'}
{'world'}
</div>,
)}
</div>,
);
};
if (__DEV__) {
expect(assertion).toErrorDev([
'Warning: Each child in a list should have a unique "key" prop.',
]);
} else {
assertion();
}
});

// @gate experimental
it('does not add separators when otherwise adjacent text is wrapped in Suspense', async () => {
function App() {
return (
<div>
hello
<Suspense>
<AsyncText text={'world'} />
</Suspense>
</div>
);
}

await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
await afterImmediate();
await act(() => resolveText('world'));

pipe(writable);
});

expect(container.firstElementChild.outerHTML).toEqual(
'<div>hello<!--$-->world<!--/$--></div>',
);

const errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
errors.push(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
expect(errors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(
<div>
{'hello'}
{'world'}
</div>,
);
});

// @gate experimental
it('does not prepend separators for Suspense fallback text but will append them if followed by text', async () => {
function App() {
return (
<div>
<Suspense fallback={'outer'}>
hello
<Suspense
fallback={
<>
<AsyncText text={'world'} />!<AsyncText text={'foo'} />
</>
}>
<AsyncText text={'bar'} />
</Suspense>
!
</Suspense>
</div>
);
}

await act(async () => {
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
await afterImmediate();
await act(() => resolveText('foo'));
pipe(writable);
});

expect(container.innerHTML).toEqual(
'<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>',
);

await act(() => resolveText('world'));

expect(container.children[0].outerHTML).toEqual(
'<div><!--$-->hello<!--$?--><template id="B:1"></template>world!<!-- -->foo<!--/$-->!<!--/$--></div>',
);

const errors = [];
ReactDOMClient.hydrateRoot(container, <App />, {
onRecoverableError(error) {
errors.push(error.message);
},
});
expect(Scheduler).toFlushAndYield([]);
expect(errors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(
<div>
{'hello'}
{/* starting the inner Suspense boundary Fallback */}
{'world'}
{'!'}
{'foo'}
{/* ending the inner Suspense boundary Fallback */}
{'!'}
</div>,
);
});
});
});
4 changes: 1 addition & 3 deletions packages/react-dom/src/__tests__/ReactDOMFizzServerBrowser-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,7 @@ describe('ReactDOMFizzServer', () => {
expect(isComplete).toBe(true);

const result = await readResult(stream);
expect(result).toMatchInlineSnapshot(
`"<div><!--$-->Done<!-- --><!--/$--></div>"`,
);
expect(result).toMatchInlineSnapshot(`"<div><!--$-->Done<!--/$--></div>"`);
});

// @gate experimental
Expand Down
Loading

0 comments on commit e85a292

Please sign in to comment.