Skip to content

Commit 36e73f7

Browse files
acdliteacdlite
committed
Revert "treat empty string as null (facebook#22807)"
This reverts commit c1220eb.
1 parent 1a80d59 commit 36e73f7

File tree

6 files changed

+41
-102
lines changed

6 files changed

+41
-102
lines changed

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

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,17 @@ describe('ReactDOMServerIntegration', () => {
8787
{''}
8888
</div>,
8989
);
90-
expect(e.childNodes.length).toBe(0);
91-
expect(e.textContent).toBe('');
90+
if (render === serverRender || render === streamRender) {
91+
// For plain server markup result we should have no text nodes if
92+
// they're all empty.
93+
expect(e.childNodes.length).toBe(0);
94+
expect(e.textContent).toBe('');
95+
} else {
96+
expect(e.childNodes.length).toBe(3);
97+
expectTextNode(e.childNodes[0], '');
98+
expectTextNode(e.childNodes[1], '');
99+
expectTextNode(e.childNodes[2], '');
100+
}
92101
});
93102

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

154163
itRenders('a leading blank child with a text sibling', async render => {
155164
const e = await render(<div>{''}foo</div>);
156-
expect(e.childNodes.length).toBe(1);
157-
expectTextNode(e.childNodes[0], 'foo');
165+
if (render === serverRender || render === streamRender) {
166+
expect(e.childNodes.length).toBe(1);
167+
expectTextNode(e.childNodes[0], 'foo');
168+
} else {
169+
expect(e.childNodes.length).toBe(2);
170+
expectTextNode(e.childNodes[0], '');
171+
expectTextNode(e.childNodes[1], 'foo');
172+
}
158173
});
159174

160175
itRenders('a trailing blank child with a text sibling', async render => {
161176
const e = await render(<div>foo{''}</div>);
162-
expect(e.childNodes.length).toBe(1);
163-
expectTextNode(e.childNodes[0], 'foo');
177+
// with Fiber, there are just two text nodes.
178+
if (render === serverRender || render === streamRender) {
179+
expect(e.childNodes.length).toBe(1);
180+
expectTextNode(e.childNodes[0], 'foo');
181+
} else {
182+
expect(e.childNodes.length).toBe(2);
183+
expectTextNode(e.childNodes[0], 'foo');
184+
expectTextNode(e.childNodes[1], '');
185+
}
164186
});
165187

166188
itRenders('an element with two text children', async render => {

‎packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 1 addition & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
'use strict';
1111

12-
let React = require('react');
12+
let React;
1313
let ReactDOM;
1414
let ReactDOMServer;
1515
let Scheduler;
@@ -70,17 +70,6 @@ function dispatchMouseEvent(to, from) {
7070
}
7171
}
7272

73-
class TestAppClass extends React.Component {
74-
render() {
75-
return (
76-
<div>
77-
<>{''}</>
78-
<>{'Hello'}</>
79-
</div>
80-
);
81-
}
82-
}
83-
8473
describe('ReactDOMServerPartialHydration', () => {
8574
beforeEach(() => {
8675
jest.resetModuleRegistry();
@@ -2969,49 +2958,4 @@ describe('ReactDOMServerPartialHydration', () => {
29692958
expect(ref.current).toBe(span);
29702959
expect(ref.current.innerHTML).toBe('Hidden child');
29712960
});
2972-
2973-
function itHydratesWithoutMismatch(msg, App) {
2974-
it('hydrates without mismatch ' + msg, () => {
2975-
const container = document.createElement('div');
2976-
document.body.appendChild(container);
2977-
const finalHTML = ReactDOMServer.renderToString(<App />);
2978-
container.innerHTML = finalHTML;
2979-
2980-
ReactDOM.hydrateRoot(container, <App />);
2981-
Scheduler.unstable_flushAll();
2982-
});
2983-
}
2984-
2985-
itHydratesWithoutMismatch('an empty string with neighbors', function App() {
2986-
return (
2987-
<div>
2988-
<div id="test">Test</div>
2989-
{'' && <div>Test</div>}
2990-
{'Test'}
2991-
</div>
2992-
);
2993-
});
2994-
2995-
itHydratesWithoutMismatch('an empty string', function App() {
2996-
return '';
2997-
});
2998-
itHydratesWithoutMismatch(
2999-
'an empty string simple in fragment',
3000-
function App() {
3001-
return (
3002-
<>
3003-
{''}
3004-
{'sup'}
3005-
</>
3006-
);
3007-
},
3008-
);
3009-
itHydratesWithoutMismatch(
3010-
'an empty string simple in suspense',
3011-
function App() {
3012-
return <Suspense>{'' && false}</Suspense>;
3013-
},
3014-
);
3015-
3016-
itHydratesWithoutMismatch('an empty string in class component', TestAppClass);
30172961
});

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

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ const expectChildren = function(container, children) {
5353
const child = children[i];
5454

5555
if (typeof child === 'string') {
56-
if (child === '') {
57-
continue;
58-
}
5956
textNode = outerNode.childNodes[mountIndex];
6057
expect(textNode.nodeType).toBe(3);
6158
expect(textNode.data).toBe(child);
@@ -86,7 +83,7 @@ describe('ReactMultiChildText', () => {
8683
true, [],
8784
0, '0',
8885
1.2, '1.2',
89-
'', [],
86+
'', '',
9087
'foo', 'foo',
9188

9289
[], [],
@@ -96,7 +93,7 @@ describe('ReactMultiChildText', () => {
9693
[true], [],
9794
[0], ['0'],
9895
[1.2], ['1.2'],
99-
[''], [],
96+
[''], [''],
10097
['foo'], ['foo'],
10198
[<div />], [<div />],
10299

‎packages/react-reconciler/src/ReactChildFiber.new.js

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -492,10 +492,7 @@ function ChildReconciler(shouldTrackSideEffects) {
492492
newChild: any,
493493
lanes: Lanes,
494494
): Fiber | null {
495-
if (
496-
(typeof newChild === 'string' && newChild !== '') ||
497-
typeof newChild === 'number'
498-
) {
495+
if (typeof newChild === 'string' || typeof newChild === 'number') {
499496
// Text nodes don't have keys. If the previous node is implicitly keyed
500497
// we can continue to replace it without aborting even if it is not a text
501498
// node.
@@ -571,10 +568,7 @@ function ChildReconciler(shouldTrackSideEffects) {
571568

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

574-
if (
575-
(typeof newChild === 'string' && newChild !== '') ||
576-
typeof newChild === 'number'
577-
) {
571+
if (typeof newChild === 'string' || typeof newChild === 'number') {
578572
// Text nodes don't have keys. If the previous node is implicitly keyed
579573
// we can continue to replace it without aborting even if it is not a text
580574
// node.
@@ -636,10 +630,7 @@ function ChildReconciler(shouldTrackSideEffects) {
636630
newChild: any,
637631
lanes: Lanes,
638632
): Fiber | null {
639-
if (
640-
(typeof newChild === 'string' && newChild !== '') ||
641-
typeof newChild === 'number'
642-
) {
633+
if (typeof newChild === 'string' || typeof newChild === 'number') {
643634
// Text nodes don't have keys, so we neither have to check the old nor
644635
// new node for the key. If both are text nodes, they match.
645636
const matchedFiber = existingChildren.get(newIdx) || null;
@@ -1336,10 +1327,7 @@ function ChildReconciler(shouldTrackSideEffects) {
13361327
throwOnInvalidObjectType(returnFiber, newChild);
13371328
}
13381329

1339-
if (
1340-
(typeof newChild === 'string' && newChild !== '') ||
1341-
typeof newChild === 'number'
1342-
) {
1330+
if (typeof newChild === 'string' || typeof newChild === 'number') {
13431331
return placeSingleChild(
13441332
reconcileSingleTextNode(
13451333
returnFiber,

‎packages/react-reconciler/src/ReactChildFiber.old.js

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -492,10 +492,7 @@ function ChildReconciler(shouldTrackSideEffects) {
492492
newChild: any,
493493
lanes: Lanes,
494494
): Fiber | null {
495-
if (
496-
(typeof newChild === 'string' && newChild !== '') ||
497-
typeof newChild === 'number'
498-
) {
495+
if (typeof newChild === 'string' || typeof newChild === 'number') {
499496
// Text nodes don't have keys. If the previous node is implicitly keyed
500497
// we can continue to replace it without aborting even if it is not a text
501498
// node.
@@ -571,10 +568,7 @@ function ChildReconciler(shouldTrackSideEffects) {
571568

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

574-
if (
575-
(typeof newChild === 'string' && newChild !== '') ||
576-
typeof newChild === 'number'
577-
) {
571+
if (typeof newChild === 'string' || typeof newChild === 'number') {
578572
// Text nodes don't have keys. If the previous node is implicitly keyed
579573
// we can continue to replace it without aborting even if it is not a text
580574
// node.
@@ -636,10 +630,7 @@ function ChildReconciler(shouldTrackSideEffects) {
636630
newChild: any,
637631
lanes: Lanes,
638632
): Fiber | null {
639-
if (
640-
(typeof newChild === 'string' && newChild !== '') ||
641-
typeof newChild === 'number'
642-
) {
633+
if (typeof newChild === 'string' || typeof newChild === 'number') {
643634
// Text nodes don't have keys, so we neither have to check the old nor
644635
// new node for the key. If both are text nodes, they match.
645636
const matchedFiber = existingChildren.get(newIdx) || null;
@@ -1336,10 +1327,7 @@ function ChildReconciler(shouldTrackSideEffects) {
13361327
throwOnInvalidObjectType(returnFiber, newChild);
13371328
}
13381329

1339-
if (
1340-
(typeof newChild === 'string' && newChild !== '') ||
1341-
typeof newChild === 'number'
1342-
) {
1330+
if (typeof newChild === 'string' || typeof newChild === 'number') {
13431331
return placeSingleChild(
13441332
reconcileSingleTextNode(
13451333
returnFiber,

‎packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -673,7 +673,7 @@ describe('ReactIncrementalUpdates', () => {
673673
root.render(<App />);
674674
});
675675
expect(Scheduler).toHaveYielded(['Committed: ']);
676-
expect(root).toMatchRenderedOutput(null);
676+
expect(root).toMatchRenderedOutput('');
677677

678678
await act(async () => {
679679
if (gate(flags => flags.enableSyncDefaultUpdates)) {
@@ -734,7 +734,7 @@ describe('ReactIncrementalUpdates', () => {
734734
root.render(<App />);
735735
});
736736
expect(Scheduler).toHaveYielded([]);
737-
expect(root).toMatchRenderedOutput(null);
737+
expect(root).toMatchRenderedOutput('');
738738

739739
await act(async () => {
740740
if (gate(flags => flags.enableSyncDefaultUpdates)) {

0 commit comments

Comments
 (0)