Deprecate renderToNodeStream (and fix textarea bug) (facebook#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.

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

@@ -101,7 +101,7 @@ describe('ReactDOMServerIntegration', () => {
         ) {
           // For plain server markup result we have comments between.
           // If we're able to hydrate, they remain.
-          expect(e.childNodes.length).toBe(5);
+          expect(e.childNodes.length).toBe(render === streamRender ? 6 : 5);
           expectTextNode(e.childNodes[0], ' ');
           expectTextNode(e.childNodes[2], ' ');
           expectTextNode(e.childNodes[4], ' ');
@@ -119,8 +119,8 @@ describe('ReactDOMServerIntegration', () => {
             Text<span>More Text</span>
           </div>,
         );
-        expect(e.childNodes.length).toBe(2);
-        const spanNode = e.childNodes[1];
+        expect(e.childNodes.length).toBe(render === streamRender ? 3 : 2);
+        const spanNode = e.childNodes[render === streamRender ? 2 : 1];
         expectTextNode(e.childNodes[0], 'Text');
         expect(spanNode.tagName).toBe('SPAN');
         expect(spanNode.childNodes.length).toBe(1);
@@ -147,19 +147,19 @@ describe('ReactDOMServerIntegration', () => {
       itRenders('a custom element with text', async render => {
         const e = await render(<custom-element>Text</custom-element>);
         expect(e.tagName).toBe('CUSTOM-ELEMENT');
-        expect(e.childNodes.length).toBe(1);
+        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
         expectNode(e.firstChild, TEXT_NODE_TYPE, 'Text');
       });
 
       itRenders('a leading blank child with a text sibling', async render => {
         const e = await render(<div>{''}foo</div>);
-        expect(e.childNodes.length).toBe(1);
+        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
         expectTextNode(e.childNodes[0], '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);
+        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
         expectTextNode(e.childNodes[0], 'foo');
       });
 
@@ -176,7 +176,7 @@ describe('ReactDOMServerIntegration', () => {
           render === streamRender
         ) {
           // In the server render output there's a comment between them.
-          expect(e.childNodes.length).toBe(3);
+          expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
           expectTextNode(e.childNodes[0], 'foo');
           expectTextNode(e.childNodes[2], 'bar');
         } else {
@@ -203,7 +203,7 @@ describe('ReactDOMServerIntegration', () => {
             render === streamRender
           ) {
             // In the server render output there's a comment between them.
-            expect(e.childNodes.length).toBe(5);
+            expect(e.childNodes.length).toBe(render === streamRender ? 6 : 5);
             expectTextNode(e.childNodes[0], 'a');
             expectTextNode(e.childNodes[2], 'b');
             expectTextNode(e.childNodes[4], 'c');
@@ -240,11 +240,7 @@ describe('ReactDOMServerIntegration', () => {
             e
           </div>,
         );
-        if (
-          render === serverRender ||
-          render === clientRenderOnServerString ||
-          render === streamRender
-        ) {
+        if (render === serverRender || render === clientRenderOnServerString) {
           // In the server render output there's comments between text nodes.
           expect(e.childNodes.length).toBe(5);
           expectTextNode(e.childNodes[0], 'a');
@@ -253,6 +249,15 @@ describe('ReactDOMServerIntegration', () => {
           expectTextNode(e.childNodes[3].childNodes[0], 'c');
           expectTextNode(e.childNodes[3].childNodes[2], 'd');
           expectTextNode(e.childNodes[4], 'e');
+        } else if (render === streamRender) {
+          // In the server render output there's comments after each text node.
+          expect(e.childNodes.length).toBe(7);
+          expectTextNode(e.childNodes[0], 'a');
+          expectTextNode(e.childNodes[2], 'b');
+          expect(e.childNodes[4].childNodes.length).toBe(4);
+          expectTextNode(e.childNodes[4].childNodes[0], 'c');
+          expectTextNode(e.childNodes[4].childNodes[2], 'd');
+          expectTextNode(e.childNodes[5], 'e');
         } else {
           expect(e.childNodes.length).toBe(4);
           expectTextNode(e.childNodes[0], 'a');
@@ -291,7 +296,7 @@ describe('ReactDOMServerIntegration', () => {
           render === streamRender
         ) {
           // In the server markup there's a comment between.
-          expect(e.childNodes.length).toBe(3);
+          expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
           expectTextNode(e.childNodes[0], 'foo');
           expectTextNode(e.childNodes[2], '40');
         } else {
@@ -330,13 +335,13 @@ describe('ReactDOMServerIntegration', () => {
 
       itRenders('null children as blank', async render => {
         const e = await render(<div>{null}foo</div>);
-        expect(e.childNodes.length).toBe(1);
+        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
         expectTextNode(e.childNodes[0], 'foo');
       });
 
       itRenders('false children as blank', async render => {
         const e = await render(<div>{false}foo</div>);
-        expect(e.childNodes.length).toBe(1);
+        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
         expectTextNode(e.childNodes[0], 'foo');
       });
 
@@ -348,7 +353,7 @@ describe('ReactDOMServerIntegration', () => {
             {false}
           </div>,
         );
-        expect(e.childNodes.length).toBe(1);
+        expect(e.childNodes.length).toBe(render === streamRender ? 2 : 1);
         expectTextNode(e.childNodes[0], 'foo');
       });
 
@@ -735,10 +740,10 @@ describe('ReactDOMServerIntegration', () => {
             </div>,
           );
           expect(e.id).toBe('parent');
-          expect(e.childNodes.length).toBe(3);
+          expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
           const child1 = e.childNodes[0];
           const textNode = e.childNodes[1];
-          const child2 = e.childNodes[2];
+          const child2 = e.childNodes[render === streamRender ? 3 : 2];
           expect(child1.id).toBe('child1');
           expect(child1.childNodes.length).toBe(0);
           expectTextNode(textNode, ' ');
@@ -752,10 +757,10 @@ describe('ReactDOMServerIntegration', () => {
         async render => {
           // prettier-ignore
           const e = await render(<div id="parent">  <div id="child" />   </div>); // eslint-disable-line no-multi-spaces
-          expect(e.childNodes.length).toBe(3);
+          expect(e.childNodes.length).toBe(render === streamRender ? 5 : 3);
           const textNode1 = e.childNodes[0];
-          const child = e.childNodes[1];
-          const textNode2 = e.childNodes[2];
+          const child = e.childNodes[render === streamRender ? 2 : 1];
+          const textNode2 = e.childNodes[render === streamRender ? 3 : 2];
           expect(e.id).toBe('parent');
           expectTextNode(textNode1, '  ');
           expect(child.id).toBe('child');
@@ -778,7 +783,9 @@ describe('ReactDOMServerIntegration', () => {
         ) {
           // For plain server markup result we have comments between.
           // If we're able to hydrate, they remain.
-          expect(parent.childNodes.length).toBe(5);
+          expect(parent.childNodes.length).toBe(
+            render === streamRender ? 6 : 5,
+          );
           expectTextNode(parent.childNodes[0], 'a');
           expectTextNode(parent.childNodes[2], 'b');
           expectTextNode(parent.childNodes[4], 'c');
@@ -810,7 +817,7 @@ describe('ReactDOMServerIntegration', () => {
           render === clientRenderOnServerString ||
           render === streamRender
         ) {
-          expect(e.childNodes.length).toBe(3);
+          expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
           expectTextNode(e.childNodes[0], '<span>Text1&quot;</span>');
           expectTextNode(e.childNodes[2], '<span>Text2&quot;</span>');
         } else {
@@ -861,7 +868,7 @@ describe('ReactDOMServerIntegration', () => {
           );
           if (render === serverRender || render === streamRender) {
             // We have three nodes because there is a comment between them.
-            expect(e.childNodes.length).toBe(3);
+            expect(e.childNodes.length).toBe(render === streamRender ? 4 : 3);
             // Everything becomes LF when parsed from server HTML.
             // Null character is ignored.
             expectNode(e.childNodes[0], TEXT_NODE_TYPE, 'foo\nbar');

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

@@ -409,12 +409,24 @@ describe('ReactDOMServerIntegration', () => {
         </LoggedInUser.Provider>
       );
 
-      const streamAmy = ReactDOMServer.renderToNodeStream(
-        AppWithUser('Amy'),
-      ).setEncoding('utf8');
-      const streamBob = ReactDOMServer.renderToNodeStream(
-        AppWithUser('Bob'),
-      ).setEncoding('utf8');
+      let streamAmy;
+      let streamBob;
+      expect(() => {
+        streamAmy = ReactDOMServer.renderToNodeStream(
+          AppWithUser('Amy'),
+        ).setEncoding('utf8');
+      }).toErrorDev(
+        'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
+        {withoutStack: true},
+      );
+      expect(() => {
+        streamBob = ReactDOMServer.renderToNodeStream(
+          AppWithUser('Bob'),
+        ).setEncoding('utf8');
+      }).toErrorDev(
+        'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
+        {withoutStack: true},
+      );
 
       // Testing by filling the buffer using internal _read() with a small
       // number of bytes to avoid a test case which needs to align to a
@@ -449,9 +461,14 @@ describe('ReactDOMServerIntegration', () => {
       const streamCount = 34;
 
       for (let i = 0; i < streamCount; i++) {
-        streams[i] = ReactDOMServer.renderToNodeStream(
-          NthRender(i % 2 === 0 ? 'Expected to be recreated' : i),
-        ).setEncoding('utf8');
+        expect(() => {
+          streams[i] = ReactDOMServer.renderToNodeStream(
+            NthRender(i % 2 === 0 ? 'Expected to be recreated' : i),
+          ).setEncoding('utf8');
+        }).toErrorDev(
+          'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
+          {withoutStack: true},
+        );
       }
 
       // Testing by filling the buffer using internal _read() with a small
@@ -468,9 +485,14 @@ describe('ReactDOMServerIntegration', () => {
 
       // Recreate those same streams.
       for (let i = 0; i < streamCount; i += 2) {
-        streams[i] = ReactDOMServer.renderToNodeStream(
-          NthRender(i),
-        ).setEncoding('utf8');
+        expect(() => {
+          streams[i] = ReactDOMServer.renderToNodeStream(
+            NthRender(i),
+          ).setEncoding('utf8');
+        }).toErrorDev(
+          'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
+          {withoutStack: true},
+        );
       }
 
       // Read a bit from all streams again.

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

@@ -49,6 +49,12 @@ describe('ReactDOMServerIntegrationTextarea', () => {
     expect(e.value).toBe('foo');
   });
 
+  itRenders('a textarea with a value of undefined', async render => {
+    const e = await render(<textarea value={undefined} />);
+    expect(e.getAttribute('value')).toBe(null);
+    expect(e.value).toBe('');
+  });
+
   itRenders('a textarea with a value and readOnly', async render => {
     const e = await render(<textarea value="foo" readOnly={true} />);
     // textarea DOM elements don't have a value **attribute**, the text is

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

@@ -569,15 +569,27 @@ describe('ReactDOMServer', () => {
   describe('renderToNodeStream', () => {
     it('should generate simple markup', () => {
       const SuccessfulElement = React.createElement(() => <img />);
-      const response = ReactDOMServer.renderToNodeStream(SuccessfulElement);
+      let response;
+      expect(() => {
+        response = ReactDOMServer.renderToNodeStream(SuccessfulElement);
+      }).toErrorDev(
+        'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
+        {withoutStack: true},
+      );
       expect(response.read().toString()).toMatch(new RegExp('<img' + '/>'));
     });
 
     it('should handle errors correctly', () => {
       const FailingElement = React.createElement(() => {
         throw new Error('An Error');
       });
-      const response = ReactDOMServer.renderToNodeStream(FailingElement);
+      let response;
+      expect(() => {
+        response = ReactDOMServer.renderToNodeStream(FailingElement);
+      }).toErrorDev(
+        'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
+        {withoutStack: true},
+      );
       return new Promise(resolve => {
         response.once('error', () => {
           resolve();

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

@@ -154,8 +154,11 @@ module.exports = function(initModules) {
       () =>
         new Promise((resolve, reject) => {
           const writable = new DrainWritable();
-          const s = ReactDOMServer.renderToNodeStream(reactElement);
-          s.on('error', e => reject(e));
+          const s = ReactDOMServer.renderToPipeableStream(reactElement, {
+            onErrorShell(e) {
+              reject(e);
+            },
+          });
           s.pipe(writable);
           writable.on('finish', () => resolve(writable.buffer));
         }),
@@ -168,7 +171,12 @@ module.exports = function(initModules) {
   // Does not render on client or perform client-side revival.
   async function streamRender(reactElement, errorCount = 0) {
     const markup = await renderIntoStream(reactElement, errorCount);
-    return getContainerFromMarkup(reactElement, markup).firstChild;
+    let firstNode = getContainerFromMarkup(reactElement, markup).firstChild;
+    if (firstNode && firstNode.nodeType === Node.DOCUMENT_TYPE_NODE) {
+      // Skip document type nodes.
+      firstNode = firstNode.nextSibling;
+    }
+    return firstNode;
   }
 
   const clientCleanRender = (element, errorCount = 0) => {

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

@@ -93,6 +93,11 @@ function renderToNodeStream(
   children: ReactNodeList,
   options?: ServerOptions,
 ): Readable {
+  if (__DEV__) {
+    console.error(
+      'renderToNodeStream is deprecated. Use renderToPipeableStream instead.',
+    );
+  }
   return renderToNodeStreamImpl(children, options, false);
 }
 

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

@@ -1001,7 +1001,17 @@ function pushStartTextArea(
     target.push(leadingNewline);
   }
 
-  return value;
+  // ToString and push directly instead of recurse over children.
+  // We don't really support complex children in the value anyway.
+  // This also currently avoids a trailing comment node which breaks textarea.
+  if (value !== null) {
+    if (__DEV__) {
+      checkAttributeStringCoercion(value, 'value');
+    }
+    target.push(stringToChunk(encodeHTMLTextNode('' + value)));
+  }
+
+  return null;
 }
 
 function pushSelfClosing(