Skip to content

Commit

Permalink
treat empty string as null (facebook#22807)
Browse files Browse the repository at this point in the history
  • Loading branch information
@salazarm
salazarm authored Nov 23, 2021
1 parent 09d9b17 commit c1220eb
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 41 deletions.
34 changes: 6 additions & 28 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,8 @@ describe('ReactDOMServerIntegration', () => {
{''}
</div>,
);
if (render === serverRender || render === streamRender) {
// For plain server markup result we should have no text nodes if
// they're all empty.
expect(e.childNodes.length).toBe(0);
expect(e.textContent).toBe('');
} else {
expect(e.childNodes.length).toBe(3);
expectTextNode(e.childNodes[0], '');
expectTextNode(e.childNodes[1], '');
expectTextNode(e.childNodes[2], '');
}
expect(e.childNodes.length).toBe(0);
expect(e.textContent).toBe('');
});

itRenders('a div with multiple whitespace children', async render => {
Expand Down Expand Up @@ -162,27 +153,14 @@ describe('ReactDOMServerIntegration', () => {

itRenders('a leading blank child with a text sibling', async render => {
const e = await render(<div>{''}foo</div>);
if (render === serverRender || render === streamRender) {
expect(e.childNodes.length).toBe(1);
expectTextNode(e.childNodes[0], 'foo');
} else {
expect(e.childNodes.length).toBe(2);
expectTextNode(e.childNodes[0], '');
expectTextNode(e.childNodes[1], 'foo');
}
expect(e.childNodes.length).toBe(1);
expectTextNode(e.childNodes[0], 'foo');
});

itRenders('a trailing blank child with a text sibling', async render => {
const e = await render(<div>foo{''}</div>);
// with Fiber, there are just two text nodes.
if (render === serverRender || render === streamRender) {
expect(e.childNodes.length).toBe(1);
expectTextNode(e.childNodes[0], 'foo');
} else {
expect(e.childNodes.length).toBe(2);
expectTextNode(e.childNodes[0], 'foo');
expectTextNode(e.childNodes[1], '');
}
expect(e.childNodes.length).toBe(1);
expectTextNode(e.childNodes[0], 'foo');
});

itRenders('an element with two text children', async render => {
Expand Down
58 changes: 57 additions & 1 deletion packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

'use strict';

let React;
let React = require('react');
let ReactDOM;
let ReactDOMServer;
let Scheduler;
Expand Down Expand Up @@ -70,6 +70,17 @@ function dispatchMouseEvent(to, from) {
}
}

class TestAppClass extends React.Component {
render() {
return (
<div>
<>{''}</>
<>{'Hello'}</>
</div>
);
}
}

describe('ReactDOMServerPartialHydration', () => {
beforeEach(() => {
jest.resetModuleRegistry();
Expand Down Expand Up @@ -2958,4 +2969,49 @@ describe('ReactDOMServerPartialHydration', () => {
expect(ref.current).toBe(span);
expect(ref.current.innerHTML).toBe('Hidden child');
});

function itHydratesWithoutMismatch(msg, App) {
it('hydrates without mismatch ' + msg, () => {
const container = document.createElement('div');
document.body.appendChild(container);
const finalHTML = ReactDOMServer.renderToString(<App />);
container.innerHTML = finalHTML;

ReactDOM.hydrateRoot(container, <App />);
Scheduler.unstable_flushAll();
});
}

itHydratesWithoutMismatch('an empty string with neighbors', function App() {
return (
<div>
<div id="test">Test</div>
{'' && <div>Test</div>}
{'Test'}
</div>
);
});

itHydratesWithoutMismatch('an empty string', function App() {
return '';
});
itHydratesWithoutMismatch(
'an empty string simple in fragment',
function App() {
return (
<>
{''}
{'sup'}
</>
);
},
);
itHydratesWithoutMismatch(
'an empty string simple in suspense',
function App() {
return <Suspense>{'' && false}</Suspense>;
},
);

itHydratesWithoutMismatch('an empty string in class component', TestAppClass);
});
7 changes: 5 additions & 2 deletions packages/react-dom/src/__tests__/ReactMultiChildText-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ const expectChildren = function(container, children) {
const child = children[i];

if (typeof child === 'string') {
if (child === '') {
continue;
}
textNode = outerNode.childNodes[mountIndex];
expect(textNode.nodeType).toBe(3);
expect(textNode.data).toBe(child);
Expand Down Expand Up @@ -83,7 +86,7 @@ describe('ReactMultiChildText', () => {
true, [],
0, '0',
1.2, '1.2',
'', '',
'', [],
'foo', 'foo',

[], [],
Expand All @@ -93,7 +96,7 @@ describe('ReactMultiChildText', () => {
[true], [],
[0], ['0'],
[1.2], ['1.2'],
[''], [''],
[''], [],
['foo'], ['foo'],
[<div />], [<div />],

Expand Down
20 changes: 16 additions & 4 deletions packages/react-reconciler/src/ReactChildFiber.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,10 @@ function ChildReconciler(shouldTrackSideEffects) {
newChild: any,
lanes: Lanes,
): Fiber | null {
if (typeof newChild === 'string' || typeof newChild === 'number') {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
// Text nodes don't have keys. If the previous node is implicitly keyed
// we can continue to replace it without aborting even if it is not a text
// node.
Expand Down Expand Up @@ -568,7 +571,10 @@ function ChildReconciler(shouldTrackSideEffects) {

const key = oldFiber !== null ? oldFiber.key : null;

if (typeof newChild === 'string' || typeof newChild === 'number') {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
// Text nodes don't have keys. If the previous node is implicitly keyed
// we can continue to replace it without aborting even if it is not a text
// node.
Expand Down Expand Up @@ -630,7 +636,10 @@ function ChildReconciler(shouldTrackSideEffects) {
newChild: any,
lanes: Lanes,
): Fiber | null {
if (typeof newChild === 'string' || typeof newChild === 'number') {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
// Text nodes don't have keys, so we neither have to check the old nor
// new node for the key. If both are text nodes, they match.
const matchedFiber = existingChildren.get(newIdx) || null;
Expand Down Expand Up @@ -1327,7 +1336,10 @@ function ChildReconciler(shouldTrackSideEffects) {
throwOnInvalidObjectType(returnFiber, newChild);
}

if (typeof newChild === 'string' || typeof newChild === 'number') {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
return placeSingleChild(
reconcileSingleTextNode(
returnFiber,
Expand Down
20 changes: 16 additions & 4 deletions packages/react-reconciler/src/ReactChildFiber.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,10 @@ function ChildReconciler(shouldTrackSideEffects) {
newChild: any,
lanes: Lanes,
): Fiber | null {
if (typeof newChild === 'string' || typeof newChild === 'number') {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
// Text nodes don't have keys. If the previous node is implicitly keyed
// we can continue to replace it without aborting even if it is not a text
// node.
Expand Down Expand Up @@ -568,7 +571,10 @@ function ChildReconciler(shouldTrackSideEffects) {

const key = oldFiber !== null ? oldFiber.key : null;

if (typeof newChild === 'string' || typeof newChild === 'number') {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
// Text nodes don't have keys. If the previous node is implicitly keyed
// we can continue to replace it without aborting even if it is not a text
// node.
Expand Down Expand Up @@ -630,7 +636,10 @@ function ChildReconciler(shouldTrackSideEffects) {
newChild: any,
lanes: Lanes,
): Fiber | null {
if (typeof newChild === 'string' || typeof newChild === 'number') {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
// Text nodes don't have keys, so we neither have to check the old nor
// new node for the key. If both are text nodes, they match.
const matchedFiber = existingChildren.get(newIdx) || null;
Expand Down Expand Up @@ -1327,7 +1336,10 @@ function ChildReconciler(shouldTrackSideEffects) {
throwOnInvalidObjectType(returnFiber, newChild);
}

if (typeof newChild === 'string' || typeof newChild === 'number') {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
return placeSingleChild(
reconcileSingleTextNode(
returnFiber,
Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ describe('ReactIncrementalUpdates', () => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded(['Committed: ']);
expect(root).toMatchRenderedOutput('');
expect(root).toMatchRenderedOutput(null);

await act(async () => {
if (gate(flags => flags.enableSyncDefaultUpdates)) {
Expand Down Expand Up @@ -734,7 +734,7 @@ describe('ReactIncrementalUpdates', () => {
root.render(<App />);
});
expect(Scheduler).toHaveYielded([]);
expect(root).toMatchRenderedOutput('');
expect(root).toMatchRenderedOutput(null);

await act(async () => {
if (gate(flags => flags.enableSyncDefaultUpdates)) {
Expand Down

0 comments on commit c1220eb

Please sign in to comment.