@samitouri / QOS-React / commits / 8d0d0e9a8a

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.

Sebastian Markbåge committed Feb 24, 2022 at 20:09 UTC 8d0d0e9a8aadc4bdddff3a40871dbc54c63264f3

7 files changed +113 -43

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

+32 -25

@@ -101,7 +101,7 @@ describe('ReactDOMServerIntegration', () => {

101 ) {

102 // For plain server markup result we have comments between.

103 // 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);

105 expectTextNode(e.childNodes[0], ' ');

106 expectTextNode(e.childNodes[2], ' ');

107 expectTextNode(e.childNodes[4], ' ');

@@ -119,8 +119,8 @@ describe('ReactDOMServerIntegration', () => {

119 Text<span>More Text</span>

120 </div>,

121 );

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];

124 expectTextNode(e.childNodes[0], 'Text');

125 expect(spanNode.tagName).toBe('SPAN');

126 expect(spanNode.childNodes.length).toBe(1);

@@ -147,19 +147,19 @@ describe('ReactDOMServerIntegration', () => {

147 itRenders('a custom element with text', async render => {

148 const e = await render(<custom-element>Text</custom-element>);

149 expect(e.tagName).toBe('CUSTOM-ELEMENT');

150 - expect(e.childNodes.length).toBe(1);

150 + expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);

151 expectNode(e.firstChild, TEXT_NODE_TYPE, 'Text');

152 });

153

154 itRenders('a leading blank child with a text sibling', async render => {

155 const e = await render(<div>{''}foo</div>);

156 - expect(e.childNodes.length).toBe(1);

156 + expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);

157 expectTextNode(e.childNodes[0], 'foo');

158 });

159

160 itRenders('a trailing blank child with a text sibling', async render => {

161 const e = await render(<div>foo{''}</div>);

162 - expect(e.childNodes.length).toBe(1);

162 + expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);

163 expectTextNode(e.childNodes[0], 'foo');

164 });

165

@@ -176,7 +176,7 @@ describe('ReactDOMServerIntegration', () => {

176 render === streamRender

177 ) {

178 // 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);

180 expectTextNode(e.childNodes[0], 'foo');

181 expectTextNode(e.childNodes[2], 'bar');

182 } else {

@@ -203,7 +203,7 @@ describe('ReactDOMServerIntegration', () => {

203 render === streamRender

204 ) {

205 // 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);

207 expectTextNode(e.childNodes[0], 'a');

208 expectTextNode(e.childNodes[2], 'b');

209 expectTextNode(e.childNodes[4], 'c');

@@ -240,11 +240,7 @@ describe('ReactDOMServerIntegration', () => {

240 e

241 </div>,

242 );

243 - if (

244 - render === serverRender ||

245 - render === clientRenderOnServerString ||

246 - render === streamRender

247 - ) {

243 + if (render === serverRender || render === clientRenderOnServerString) {

244 // In the server render output there's comments between text nodes.

245 expect(e.childNodes.length).toBe(5);

246 expectTextNode(e.childNodes[0], 'a');

@@ -253,6 +249,15 @@ describe('ReactDOMServerIntegration', () => {

249 expectTextNode(e.childNodes[3].childNodes[0], 'c');

250 expectTextNode(e.childNodes[3].childNodes[2], 'd');

251 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');

261 } else {

262 expect(e.childNodes.length).toBe(4);

263 expectTextNode(e.childNodes[0], 'a');

@@ -291,7 +296,7 @@ describe('ReactDOMServerIntegration', () => {

296 render === streamRender

297 ) {

298 // 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);

300 expectTextNode(e.childNodes[0], 'foo');

301 expectTextNode(e.childNodes[2], '40');

302 } else {

@@ -330,13 +335,13 @@ describe('ReactDOMServerIntegration', () => {

335

336 itRenders('null children as blank', async render => {

337 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);

339 expectTextNode(e.childNodes[0], 'foo');

340 });

341

342 itRenders('false children as blank', async render => {

343 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);

345 expectTextNode(e.childNodes[0], 'foo');

346 });

347

@@ -348,7 +353,7 @@ describe('ReactDOMServerIntegration', () => {

353 {false}

354 </div>,

355 );

351 - expect(e.childNodes.length).toBe(1);

356 + expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);

357 expectTextNode(e.childNodes[0], 'foo');

358 });

359

@@ -735,10 +740,10 @@ describe('ReactDOMServerIntegration', () => {

740 </div>,

741 );

742 expect(e.id).toBe('parent');

738 - expect(e.childNodes.length).toBe(3);

743 + expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);

744 const child1 = e.childNodes[0];

745 const textNode = e.childNodes[1];

741 - const child2 = e.childNodes[2];

746 + const child2 = e.childNodes[render === streamRender ? 3 : 2];

747 expect(child1.id).toBe('child1');

748 expect(child1.childNodes.length).toBe(0);

749 expectTextNode(textNode, ' ');

@@ -752,10 +757,10 @@ describe('ReactDOMServerIntegration', () => {

757 async render => {

758 // prettier-ignore

759 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);

761 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];

764 expect(e.id).toBe('parent');

765 expectTextNode(textNode1, ' ');

766 expect(child.id).toBe('child');

@@ -778,7 +783,9 @@ describe('ReactDOMServerIntegration', () => {

783 ) {

784 // For plain server markup result we have comments between.

785 // 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 + );

789 expectTextNode(parent.childNodes[0], 'a');

790 expectTextNode(parent.childNodes[2], 'b');

791 expectTextNode(parent.childNodes[4], 'c');

@@ -810,7 +817,7 @@ describe('ReactDOMServerIntegration', () => {

817 render === clientRenderOnServerString ||

818 render === streamRender

819 ) {

813 - expect(e.childNodes.length).toBe(3);

820 + expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);

821 expectTextNode(e.childNodes[0], '<span>Text1"</span>');

822 expectTextNode(e.childNodes[2], '<span>Text2"</span>');

823 } else {

@@ -861,7 +868,7 @@ describe('ReactDOMServerIntegration', () => {

868 );

869 if (render === serverRender || render === streamRender) {

870 // 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);

872 // Everything becomes LF when parsed from server HTML.

873 // Null character is ignored.

874 expectNode(e.childNodes[0], TEXT_NODE_TYPE, 'foo\nbar');

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

+34 -12

@@ -409,12 +409,24 @@ describe('ReactDOMServerIntegration', () => {

409 </LoggedInUser.Provider>

410 );

411

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 + );

430

431 // Testing by filling the buffer using internal _read() with a small

432 // number of bytes to avoid a test case which needs to align to a

@@ -449,9 +461,14 @@ describe('ReactDOMServerIntegration', () => {

461 const streamCount = 34;

462

463 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 + );

472 }

473

474 // Testing by filling the buffer using internal _read() with a small

@@ -468,9 +485,14 @@ describe('ReactDOMServerIntegration', () => {

485

486 // Recreate those same streams.

487 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 + );

496 }

497

498 // Read a bit from all streams again.

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

@@ -49,6 +49,12 @@ describe('ReactDOMServerIntegrationTextarea', () => {

49 expect(e.value).toBe('foo');

50 });

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 +

58 itRenders('a textarea with a value and readOnly', async render => {

59 const e = await render(<textarea value="foo" readOnly={true} />);

60 // textarea DOM elements don't have a value **attribute**, the text is

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

+14 -2

@@ -569,7 +569,13 @@ describe('ReactDOMServer', () => {

569 describe('renderToNodeStream', () => {

570 it('should generate simple markup', () => {

571 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 + );

579 expect(response.read().toString()).toMatch(new RegExp('<img' + '/>'));

580 });

581

@@ -577,7 +583,13 @@ describe('ReactDOMServer', () => {

583 const FailingElement = React.createElement(() => {

584 throw new Error('An Error');

585 });

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 + );

593 return new Promise(resolve => {

594 response.once('error', () => {

595 resolve();

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

+11 -3

@@ -154,8 +154,11 @@ module.exports = function(initModules) {

154 () =>

155 new Promise((resolve, reject) => {

156 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 + });

162 s.pipe(writable);

163 writable.on('finish', () => resolve(writable.buffer));

164 }),

@@ -168,7 +171,12 @@ module.exports = function(initModules) {

171 // Does not render on client or perform client-side revival.

172 async function streamRender(reactElement, errorCount = 0) {

173 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;

180 }

181

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

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

@@ -93,6 +93,11 @@ function renderToNodeStream(

93 children: ReactNodeList,

94 options?: ServerOptions,

95 ): Readable {

96 + if (__DEV__) {

97 + console.error(

98 + 'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',

99 + );

100 + }

101 return renderToNodeStreamImpl(children, options, false);

102 }

103

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

+11 -1

@@ -1001,7 +1001,17 @@ function pushStartTextArea(

1001 target.push(leadingNewline);

1002 }

1003

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;

1015 }

1016

1017 function pushSelfClosing(