Skip to content

Commit c1220eb

Browse files
salazarmsalazarm
authored
treat empty string as null (#22807)
1 parent 09d9b17 commit c1220eb

6 files changed

Lines changed: 102 additions & 41 deletions

File tree

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

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -87,17 +87,8 @@ describe('ReactDOMServerIntegration', () => {
8787
{''}
8888
</div>,
8989
);
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-
}
90+
expect(e.childNodes.length).toBe(0);
91+
expect(e.textContent).toBe('');
10192
});
10293

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

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

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

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

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

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

1010
'use strict';
1111

12-
let React;
12+
let React = require('react');
1313
let ReactDOM;
1414
let ReactDOMServer;
1515
let Scheduler;
@@ -70,6 +70,17 @@ 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+
7384
describe('ReactDOMServerPartialHydration', () => {
7485
beforeEach(() => {
7586
jest.resetModuleRegistry();
@@ -2958,4 +2969,49 @@ describe('ReactDOMServerPartialHydration', () => {
29582969
expect(ref.current).toBe(span);
29592970
expect(ref.current.innerHTML).toBe('Hidden child');
29602971
});
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);
29613017
});

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

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

5555
if (typeof child === 'string') {
56+
if (child === '') {
57+
continue;
58+
}
5659
textNode = outerNode.childNodes[mountIndex];
5760
expect(textNode.nodeType).toBe(3);
5861
expect(textNode.data).toBe(child);
@@ -83,7 +86,7 @@ describe('ReactMultiChildText', () => {
8386
true, [],
8487
0, '0',
8588
1.2, '1.2',
86-
'', '',
89+
'', [],
8790
'foo', 'foo',
8891

8992
[], [],
@@ -93,7 +96,7 @@ describe('ReactMultiChildText', () => {
9396
[true], [],
9497
[0], ['0'],
9598
[1.2], ['1.2'],
96-
[''], [''],
99+
[''], [],
97100
['foo'], ['foo'],
98101
[<div />], [<div />],
99102

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

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

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

571-
if (typeof newChild === 'string' || typeof newChild === 'number') {
574+
if (
575+
(typeof newChild === 'string' && newChild !== '') ||
576+
typeof newChild === 'number'
577+
) {
572578
// Text nodes don't have keys. If the previous node is implicitly keyed
573579
// we can continue to replace it without aborting even if it is not a text
574580
// node.
@@ -630,7 +636,10 @@ function ChildReconciler(shouldTrackSideEffects) {
630636
newChild: any,
631637
lanes: Lanes,
632638
): Fiber | null {
633-
if (typeof newChild === 'string' || typeof newChild === 'number') {
639+
if (
640+
(typeof newChild === 'string' && newChild !== '') ||
641+
typeof newChild === 'number'
642+
) {
634643
// Text nodes don't have keys, so we neither have to check the old nor
635644
// new node for the key. If both are text nodes, they match.
636645
const matchedFiber = existingChildren.get(newIdx) || null;
@@ -1327,7 +1336,10 @@ function ChildReconciler(shouldTrackSideEffects) {
13271336
throwOnInvalidObjectType(returnFiber, newChild);
13281337
}
13291338

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

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

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

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

571-
if (typeof newChild === 'string' || typeof newChild === 'number') {
574+
if (
575+
(typeof newChild === 'string' && newChild !== '') ||
576+
typeof newChild === 'number'
577+
) {
572578
// Text nodes don't have keys. If the previous node is implicitly keyed
573579
// we can continue to replace it without aborting even if it is not a text
574580
// node.
@@ -630,7 +636,10 @@ function ChildReconciler(shouldTrackSideEffects) {
630636
newChild: any,
631637
lanes: Lanes,
632638
): Fiber | null {
633-
if (typeof newChild === 'string' || typeof newChild === 'number') {
639+
if (
640+
(typeof newChild === 'string' && newChild !== '') ||
641+
typeof newChild === 'number'
642+
) {
634643
// Text nodes don't have keys, so we neither have to check the old nor
635644
// new node for the key. If both are text nodes, they match.
636645
const matchedFiber = existingChildren.get(newIdx) || null;
@@ -1327,7 +1336,10 @@ function ChildReconciler(shouldTrackSideEffects) {
13271336
throwOnInvalidObjectType(returnFiber, newChild);
13281337
}
13291338

1330-
if (typeof newChild === 'string' || typeof newChild === 'number') {
1339+
if (
1340+
(typeof newChild === 'string' && newChild !== '') ||
1341+
typeof newChild === 'number'
1342+
) {
13311343
return placeSingleChild(
13321344
reconcileSingleTextNode(
13331345
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('');
676+
expect(root).toMatchRenderedOutput(null);
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('');
737+
expect(root).toMatchRenderedOutput(null);
738738

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

0 commit comments

Comments
 (0)
⚡