Skip to content

Commit

Permalink
Revert "treat empty string as null (facebook#22807)"
Browse files Browse the repository at this point in the history 
This reverts commit c1220eb.
  • Loading branch information
@acdlite
acdlite committed Jan 19, 2022
1 parent 1a80d59 commit 36e73f7
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 102 deletions.
34 changes: 28 additions & 6 deletions packages/react-dom/src/__tests__/ReactDOMServerIntegrationElements-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,17 @@ describe('ReactDOMServerIntegration', () => {
{''}
</div>,
);
expect(e.childNodes.length).toBe(0);
expect(e.textContent).toBe('');
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], '');
}
});

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

itRenders('a leading blank child with a text sibling', async render => {
const e = await render(<div>{''}foo</div>);
expect(e.childNodes.length).toBe(1);
expectTextNode(e.childNodes[0], 'foo');
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');
}
});

itRenders('a trailing blank child with a text sibling', async render => {
const e = await render(<div>foo{''}</div>);
expect(e.childNodes.length).toBe(1);
expectTextNode(e.childNodes[0], 'foo');
// 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], '');
}
});

itRenders('an element with two text children', async render => {
Expand Down
58 changes: 1 addition & 57 deletions 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 = require('react');
let React;
let ReactDOM;
let ReactDOMServer;
let Scheduler;
Expand Down Expand Up @@ -70,17 +70,6 @@ function dispatchMouseEvent(to, from) {
}
}

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

describe('ReactDOMServerPartialHydration', () => {
beforeEach(() => {
jest.resetModuleRegistry();
Expand Down Expand Up @@ -2969,49 +2958,4 @@ 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: 2 additions & 5 deletions packages/react-dom/src/__tests__/ReactMultiChildText-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ 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 @@ -86,7 +83,7 @@ describe('ReactMultiChildText', () => {
true, [],
0, '0',
1.2, '1.2',
'', [],
'', '',
'foo', 'foo',

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

Expand Down
20 changes: 4 additions & 16 deletions packages/react-reconciler/src/ReactChildFiber.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,10 +492,7 @@ function ChildReconciler(shouldTrackSideEffects) {
newChild: any,
lanes: Lanes,
): Fiber | null {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
if (typeof newChild === 'string' || 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 @@ -571,10 +568,7 @@ function ChildReconciler(shouldTrackSideEffects) {

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

if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
if (typeof newChild === 'string' || 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 @@ -636,10 +630,7 @@ function ChildReconciler(shouldTrackSideEffects) {
newChild: any,
lanes: Lanes,
): Fiber | null {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
if (typeof newChild === 'string' || 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 @@ -1336,10 +1327,7 @@ function ChildReconciler(shouldTrackSideEffects) {
throwOnInvalidObjectType(returnFiber, newChild);
}

if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
if (typeof newChild === 'string' || typeof newChild === 'number') {
return placeSingleChild(
reconcileSingleTextNode(
returnFiber,
Expand Down
20 changes: 4 additions & 16 deletions packages/react-reconciler/src/ReactChildFiber.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,10 +492,7 @@ function ChildReconciler(shouldTrackSideEffects) {
newChild: any,
lanes: Lanes,
): Fiber | null {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
if (typeof newChild === 'string' || 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 @@ -571,10 +568,7 @@ function ChildReconciler(shouldTrackSideEffects) {

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

if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
if (typeof newChild === 'string' || 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 @@ -636,10 +630,7 @@ function ChildReconciler(shouldTrackSideEffects) {
newChild: any,
lanes: Lanes,
): Fiber | null {
if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
if (typeof newChild === 'string' || 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 @@ -1336,10 +1327,7 @@ function ChildReconciler(shouldTrackSideEffects) {
throwOnInvalidObjectType(returnFiber, newChild);
}

if (
(typeof newChild === 'string' && newChild !== '') ||
typeof newChild === 'number'
) {
if (typeof newChild === 'string' || 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(null);
expect(root).toMatchRenderedOutput('');

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(null);
expect(root).toMatchRenderedOutput('');

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

0 comments on commit 36e73f7

Please sign in to comment.