Skip to content

Commit 8d0d0e9

Browse files
sebmarkbagesebmarkbage
authored
Deprecate renderToNodeStream (and fix textarea bug) (#23359)
* Deprecate renderToNodeStream * Use renderToPipeableStream in tests instead of renderToNodeStream This is the equivalent API. This means that we have way less test coverage of this API but I feel like that's fine since it has a deprecation warning in it and we have coverage on renderToString that is mostly the same. * Fix textarea bug The test changes revealed a bug with textarea. It happens because we currently always insert trailing comment nodes. We should optimize that away. However, we also don't really support complex children so we should toString it anyway which is what partial renderer used to do. * Update tests that assert number of nodes These tests are unnecessarily specific about number of nodes. I special case these, which these tests already do, because they're good tests to test that the optimization actually works later when we do fix it.
1 parent efe4121 commit 8d0d0e9

7 files changed

+113
-43
lines changed

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

+32-25
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ describe('ReactDOMServerIntegration', () => {
101101
) {
102102
// For plain server markup result we have comments between.
103103
// If we're able to hydrate, they remain.
104-
expect(e.childNodes.length).toBe(5);
104+
expect(e.childNodes.length).toBe(render === streamRender ? 6 : 5);
105105
expectTextNode(e.childNodes[0], ' ');
106106
expectTextNode(e.childNodes[2], ' ');
107107
expectTextNode(e.childNodes[4], ' ');
@@ -119,8 +119,8 @@ describe('ReactDOMServerIntegration', () => {
119119
Text<span>More Text</span>
120120
</div>,
121121
);
122-
expect(e.childNodes.length).toBe(2);
123-
const spanNode = e.childNodes[1];
122+
expect(e.childNodes.length).toBe(render === streamRender ? 3 : 2);
123+
const spanNode = e.childNodes[render === streamRender ? 2 : 1];
124124
expectTextNode(e.childNodes[0], 'Text');
125125
expect(spanNode.tagName).toBe('SPAN');
126126
expect(spanNode.childNodes.length).toBe(1);
@@ -147,19 +147,19 @@ describe('ReactDOMServerIntegration', () => {
147147
itRenders('a custom element with text', async render => {
148148
const e = await render(<custom-element>Text</custom-element>);
149149
expect(e.tagName).toBe('CUSTOM-ELEMENT');
150-
expect(e.childNodes.length).toBe(1);
150+
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
151151
expectNode(e.firstChild, TEXT_NODE_TYPE, 'Text');
152152
});
153153

154154
itRenders('a leading blank child with a text sibling', async render => {
155155
const e = await render(<div>{''}foo</div>);
156-
expect(e.childNodes.length).toBe(1);
156+
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
157157
expectTextNode(e.childNodes[0], 'foo');
158158
});
159159

160160
itRenders('a trailing blank child with a text sibling', async render => {
161161
const e = await render(<div>foo{''}</div>);
162-
expect(e.childNodes.length).toBe(1);
162+
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
163163
expectTextNode(e.childNodes[0], 'foo');
164164
});
165165

@@ -176,7 +176,7 @@ describe('ReactDOMServerIntegration', () => {
176176
render === streamRender
177177
) {
178178
// In the server render output there's a comment between them.
179-
expect(e.childNodes.length).toBe(3);
179+
expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
180180
expectTextNode(e.childNodes[0], 'foo');
181181
expectTextNode(e.childNodes[2], 'bar');
182182
} else {
@@ -203,7 +203,7 @@ describe('ReactDOMServerIntegration', () => {
203203
render === streamRender
204204
) {
205205
// In the server render output there's a comment between them.
206-
expect(e.childNodes.length).toBe(5);
206+
expect(e.childNodes.length).toBe(render === streamRender ? 6 : 5);
207207
expectTextNode(e.childNodes[0], 'a');
208208
expectTextNode(e.childNodes[2], 'b');
209209
expectTextNode(e.childNodes[4], 'c');
@@ -240,11 +240,7 @@ describe('ReactDOMServerIntegration', () => {
240240
e
241241
</div>,
242242
);
243-
if (
244-
render === serverRender ||
245-
render === clientRenderOnServerString ||
246-
render === streamRender
247-
) {
243+
if (render === serverRender || render === clientRenderOnServerString) {
248244
// In the server render output there's comments between text nodes.
249245
expect(e.childNodes.length).toBe(5);
250246
expectTextNode(e.childNodes[0], 'a');
@@ -253,6 +249,15 @@ describe('ReactDOMServerIntegration', () => {
253249
expectTextNode(e.childNodes[3].childNodes[0], 'c');
254250
expectTextNode(e.childNodes[3].childNodes[2], 'd');
255251
expectTextNode(e.childNodes[4], 'e');
252+
} else if (render === streamRender) {
253+
// In the server render output there's comments after each text node.
254+
expect(e.childNodes.length).toBe(7);
255+
expectTextNode(e.childNodes[0], 'a');
256+
expectTextNode(e.childNodes[2], 'b');
257+
expect(e.childNodes[4].childNodes.length).toBe(4);
258+
expectTextNode(e.childNodes[4].childNodes[0], 'c');
259+
expectTextNode(e.childNodes[4].childNodes[2], 'd');
260+
expectTextNode(e.childNodes[5], 'e');
256261
} else {
257262
expect(e.childNodes.length).toBe(4);
258263
expectTextNode(e.childNodes[0], 'a');
@@ -291,7 +296,7 @@ describe('ReactDOMServerIntegration', () => {
291296
render === streamRender
292297
) {
293298
// In the server markup there's a comment between.
294-
expect(e.childNodes.length).toBe(3);
299+
expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
295300
expectTextNode(e.childNodes[0], 'foo');
296301
expectTextNode(e.childNodes[2], '40');
297302
} else {
@@ -330,13 +335,13 @@ describe('ReactDOMServerIntegration', () => {
330335

331336
itRenders('null children as blank', async render => {
332337
const e = await render(<div>{null}foo</div>);
333-
expect(e.childNodes.length).toBe(1);
338+
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
334339
expectTextNode(e.childNodes[0], 'foo');
335340
});
336341

337342
itRenders('false children as blank', async render => {
338343
const e = await render(<div>{false}foo</div>);
339-
expect(e.childNodes.length).toBe(1);
344+
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
340345
expectTextNode(e.childNodes[0], 'foo');
341346
});
342347

@@ -348,7 +353,7 @@ describe('ReactDOMServerIntegration', () => {
348353
{false}
349354
</div>,
350355
);
351-
expect(e.childNodes.length).toBe(1);
356+
expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
352357
expectTextNode(e.childNodes[0], 'foo');
353358
});
354359

@@ -735,10 +740,10 @@ describe('ReactDOMServerIntegration', () => {
735740
</div>,
736741
);
737742
expect(e.id).toBe('parent');
738-
expect(e.childNodes.length).toBe(3);
743+
expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
739744
const child1 = e.childNodes[0];
740745
const textNode = e.childNodes[1];
741-
const child2 = e.childNodes[2];
746+
const child2 = e.childNodes[render === streamRender ? 3 : 2];
742747
expect(child1.id).toBe('child1');
743748
expect(child1.childNodes.length).toBe(0);
744749
expectTextNode(textNode, ' ');
@@ -752,10 +757,10 @@ describe('ReactDOMServerIntegration', () => {
752757
async render => {
753758
// prettier-ignore
754759
const e = await render(<div id="parent"> <div id="child" /> </div>); // eslint-disable-line no-multi-spaces
755-
expect(e.childNodes.length).toBe(3);
760+
expect(e.childNodes.length).toBe(render === streamRender ? 5 : 3);
756761
const textNode1 = e.childNodes[0];
757-
const child = e.childNodes[1];
758-
const textNode2 = e.childNodes[2];
762+
const child = e.childNodes[render === streamRender ? 2 : 1];
763+
const textNode2 = e.childNodes[render === streamRender ? 3 : 2];
759764
expect(e.id).toBe('parent');
760765
expectTextNode(textNode1, ' ');
761766
expect(child.id).toBe('child');
@@ -778,7 +783,9 @@ describe('ReactDOMServerIntegration', () => {
778783
) {
779784
// For plain server markup result we have comments between.
780785
// If we're able to hydrate, they remain.
781-
expect(parent.childNodes.length).toBe(5);
786+
expect(parent.childNodes.length).toBe(
787+
render === streamRender ? 6 : 5,
788+
);
782789
expectTextNode(parent.childNodes[0], 'a');
783790
expectTextNode(parent.childNodes[2], 'b');
784791
expectTextNode(parent.childNodes[4], 'c');
@@ -810,7 +817,7 @@ describe('ReactDOMServerIntegration', () => {
810817
render === clientRenderOnServerString ||
811818
render === streamRender
812819
) {
813-
expect(e.childNodes.length).toBe(3);
820+
expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
814821
expectTextNode(e.childNodes[0], '<span>Text1&quot;</span>');
815822
expectTextNode(e.childNodes[2], '<span>Text2&quot;</span>');
816823
} else {
@@ -861,7 +868,7 @@ describe('ReactDOMServerIntegration', () => {
861868
);
862869
if (render === serverRender || render === streamRender) {
863870
// We have three nodes because there is a comment between them.
864-
expect(e.childNodes.length).toBe(3);
871+
expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
865872
// Everything becomes LF when parsed from server HTML.
866873
// Null character is ignored.
867874
expectNode(e.childNodes[0], TEXT_NODE_TYPE, 'foo\nbar');

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

+34-12
Original file line numberDiff line numberDiff line change
@@ -409,12 +409,24 @@ describe('ReactDOMServerIntegration', () => {
409409
</LoggedInUser.Provider>
410410
);
411411

412-
const streamAmy = ReactDOMServer.renderToNodeStream(
413-
AppWithUser('Amy'),
414-
).setEncoding('utf8');
415-
const streamBob = ReactDOMServer.renderToNodeStream(
416-
AppWithUser('Bob'),
417-
).setEncoding('utf8');
412+
let streamAmy;
413+
let streamBob;
414+
expect(() => {
415+
streamAmy = ReactDOMServer.renderToNodeStream(
416+
AppWithUser('Amy'),
417+
).setEncoding('utf8');
418+
}).toErrorDev(
419+
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
420+
{withoutStack: true},
421+
);
422+
expect(() => {
423+
streamBob = ReactDOMServer.renderToNodeStream(
424+
AppWithUser('Bob'),
425+
).setEncoding('utf8');
426+
}).toErrorDev(
427+
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
428+
{withoutStack: true},
429+
);
418430

419431
// Testing by filling the buffer using internal _read() with a small
420432
// number of bytes to avoid a test case which needs to align to a
@@ -449,9 +461,14 @@ describe('ReactDOMServerIntegration', () => {
449461
const streamCount = 34;
450462

451463
for (let i = 0; i < streamCount; i++) {
452-
streams[i] = ReactDOMServer.renderToNodeStream(
453-
NthRender(i % 2 === 0 ? 'Expected to be recreated' : i),
454-
).setEncoding('utf8');
464+
expect(() => {
465+
streams[i] = ReactDOMServer.renderToNodeStream(
466+
NthRender(i % 2 === 0 ? 'Expected to be recreated' : i),
467+
).setEncoding('utf8');
468+
}).toErrorDev(
469+
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
470+
{withoutStack: true},
471+
);
455472
}
456473

457474
// Testing by filling the buffer using internal _read() with a small
@@ -468,9 +485,14 @@ describe('ReactDOMServerIntegration', () => {
468485

469486
// Recreate those same streams.
470487
for (let i = 0; i < streamCount; i += 2) {
471-
streams[i] = ReactDOMServer.renderToNodeStream(
472-
NthRender(i),
473-
).setEncoding('utf8');
488+
expect(() => {
489+
streams[i] = ReactDOMServer.renderToNodeStream(
490+
NthRender(i),
491+
).setEncoding('utf8');
492+
}).toErrorDev(
493+
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
494+
{withoutStack: true},
495+
);
474496
}
475497

476498
// Read a bit from all streams again.

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

+6
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ describe('ReactDOMServerIntegrationTextarea', () => {
4949
expect(e.value).toBe('foo');
5050
});
5151

52+
itRenders('a textarea with a value of undefined', async render => {
53+
const e = await render(<textarea value={undefined} />);
54+
expect(e.getAttribute('value')).toBe(null);
55+
expect(e.value).toBe('');
56+
});
57+
5258
itRenders('a textarea with a value and readOnly', async render => {
5359
const e = await render(<textarea value="foo" readOnly={true} />);
5460
// textarea DOM elements don't have a value **attribute**, the text is

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

+14-2
Original file line numberDiff line numberDiff line change
@@ -569,15 +569,27 @@ describe('ReactDOMServer', () => {
569569
describe('renderToNodeStream', () => {
570570
it('should generate simple markup', () => {
571571
const SuccessfulElement = React.createElement(() => <img />);
572-
const response = ReactDOMServer.renderToNodeStream(SuccessfulElement);
572+
let response;
573+
expect(() => {
574+
response = ReactDOMServer.renderToNodeStream(SuccessfulElement);
575+
}).toErrorDev(
576+
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
577+
{withoutStack: true},
578+
);
573579
expect(response.read().toString()).toMatch(new RegExp('<img' + '/>'));
574580
});
575581

576582
it('should handle errors correctly', () => {
577583
const FailingElement = React.createElement(() => {
578584
throw new Error('An Error');
579585
});
580-
const response = ReactDOMServer.renderToNodeStream(FailingElement);
586+
let response;
587+
expect(() => {
588+
response = ReactDOMServer.renderToNodeStream(FailingElement);
589+
}).toErrorDev(
590+
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
591+
{withoutStack: true},
592+
);
581593
return new Promise(resolve => {
582594
response.once('error', () => {
583595
resolve();

‎packages/react-dom/src/__tests__/utils/ReactDOMServerIntegrationTestUtils.js

+11-3
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,11 @@ module.exports = function(initModules) {
154154
() =>
155155
new Promise((resolve, reject) => {
156156
const writable = new DrainWritable();
157-
const s = ReactDOMServer.renderToNodeStream(reactElement);
158-
s.on('error', e => reject(e));
157+
const s = ReactDOMServer.renderToPipeableStream(reactElement, {
158+
onErrorShell(e) {
159+
reject(e);
160+
},
161+
});
159162
s.pipe(writable);
160163
writable.on('finish', () => resolve(writable.buffer));
161164
}),
@@ -168,7 +171,12 @@ module.exports = function(initModules) {
168171
// Does not render on client or perform client-side revival.
169172
async function streamRender(reactElement, errorCount = 0) {
170173
const markup = await renderIntoStream(reactElement, errorCount);
171-
return getContainerFromMarkup(reactElement, markup).firstChild;
174+
let firstNode = getContainerFromMarkup(reactElement, markup).firstChild;
175+
if (firstNode && firstNode.nodeType === Node.DOCUMENT_TYPE_NODE) {
176+
// Skip document type nodes.
177+
firstNode = firstNode.nextSibling;
178+
}
179+
return firstNode;
172180
}
173181

174182
const clientCleanRender = (element, errorCount = 0) => {

‎packages/react-dom/src/server/ReactDOMLegacyServerNode.js

+5
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,11 @@ function renderToNodeStream(
9393
children: ReactNodeList,
9494
options?: ServerOptions,
9595
): Readable {
96+
if (__DEV__) {
97+
console.error(
98+
'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
99+
);
100+
}
96101
return renderToNodeStreamImpl(children, options, false);
97102
}
98103

‎packages/react-dom/src/server/ReactDOMServerFormatConfig.js

+11-1
Original file line numberDiff line numberDiff line change
@@ -1001,7 +1001,17 @@ function pushStartTextArea(
10011001
target.push(leadingNewline);
10021002
}
10031003

1004-
return value;
1004+
// ToString and push directly instead of recurse over children.
1005+
// We don't really support complex children in the value anyway.
1006+
// This also currently avoids a trailing comment node which breaks textarea.
1007+
if (value !== null) {
1008+
if (__DEV__) {
1009+
checkAttributeStringCoercion(value, 'value');
1010+
}
1011+
target.push(stringToChunk(encodeHTMLTextNode('' + value)));
1012+
}
1013+
1014+
return null;
10051015
}
10061016

10071017
function pushSelfClosing(

0 commit comments

Comments
 (0)