@samitouri / QOS-React / commits / f732fc677b

fix renderToString fails with array type children when react-dom/server render (#10221)

* fix(*): fix renderToString fails with array type children when react-dom/server render * Add an integration test that verify renderToString with array type children * Add integration test to renderToString with array type child * Update integration test to renderToString with array type child * fix renderToString fails with array type children when react-dom/server render * Update integration test to renderToString with array type child * Add to iterate that are not arrays * Add the validation react element * Improve an integration test of server renderToString with array type * prettier * prettier * Make SSR can handle a single array element and nested array * prettier * Change integeration test description * modified: src/renderers/dom/ReactDOMNodeStreamRenderer.js * Make invariants consistent * Gate outdated test itself by feature flag * Change test to make sense both with old and new code * Test more cases

guoyong yi committed Jul 31, 2017 at 23:59 UTC f732fc677b7f3f14ad7809a736d06705e4742347
6 files changed +193 -30
src/renderers/__tests__/ReactComponent-test.js
+54
@@ -13,6 +13,7 @@
13
14 var React;
15 var ReactDOM;
16 +var ReactDOMServer;
17 var ReactDOMFeatureFlags;
18 var ReactTestUtils;
19
@@ -24,6 +25,7 @@ describe('ReactComponent', () => {
25 beforeEach(() => {
26 React = require('react');
27 ReactDOM = require('react-dom');
28 + ReactDOMServer = require('react-dom/server');
29 ReactDOMFeatureFlags = require('ReactDOMFeatureFlags');
30 ReactTestUtils = require('react-dom/test-utils');
31 });
@@ -439,4 +441,56 @@ describe('ReactComponent', () => {
441 ' in Foo (at **)',
442 );
443 });
444 +
445 + it('throws if a plain object is used as a child when using SSR', async () => {
446 + var children = {
447 + x: <span />,
448 + y: <span />,
449 + z: <span />,
450 + };
451 + var element = <div>{[children]}</div>;
452 + var ex;
453 + try {
454 + ReactDOMServer.renderToString(element);
455 + } catch (e) {
456 + ex = e;
457 + }
458 + expect(ex).toBeDefined();
459 + expect(normalizeCodeLocInfo(ex.message)).toBe(
460 + 'Objects are not valid as a React child (found: object with keys ' +
461 + '{x, y, z}). If you meant to render a collection of children, use ' +
462 + 'an array instead.' +
463 + // Fiber gives a slightly better stack with the nearest host components
464 + (ReactDOMFeatureFlags.useFiber ? '\n in div (at **)' : ''),
465 + );
466 + });
467 +
468 + it('throws if a plain object even if it is in an owner when using SSR', async () => {
469 + class Foo extends React.Component {
470 + render() {
471 + var children = {
472 + a: <span />,
473 + b: <span />,
474 + c: <span />,
475 + };
476 + return <div>{[children]}</div>;
477 + }
478 + }
479 + var container = document.createElement('div');
480 + var ex;
481 + try {
482 + ReactDOMServer.renderToString(<Foo />, container);
483 + } catch (e) {
484 + ex = e;
485 + }
486 + expect(ex).toBeDefined();
487 + expect(normalizeCodeLocInfo(ex.message)).toBe(
488 + 'Objects are not valid as a React child (found: object with keys ' +
489 + '{a, b, c}). If you meant to render a collection of children, use ' +
490 + 'an array instead.\n' +
491 + // Fiber gives a slightly better stack with the nearest host components
492 + (ReactDOMFeatureFlags.useFiber ? ' in div (at **)\n' : '') +
493 + ' in Foo (at **)',
494 + );
495 + });
496 });
src/renderers/dom/ReactDOMNodeStreamRenderer.js
+15 -8
@@ -14,6 +14,7 @@
14 var invariant = require('fbjs/lib/invariant');
15 var React = require('react');
16 var ReactPartialRenderer = require('ReactPartialRenderer');
17 +var ReactFeatureFlags = require('ReactFeatureFlags');
18
19 var Readable = require('stream').Readable;
20
@@ -40,10 +41,13 @@ class ReactMarkupReadableStream extends Readable {
41 * See https://facebook.github.io/react/docs/react-dom-stream.html#rendertostream
42 */
43 function renderToStream(element) {
43 - invariant(
44 - React.isValidElement(element),
45 - 'renderToStream(): You must pass a valid ReactElement.',
46 - );
44 + const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures;
45 + if (disableNewFiberFeatures) {
46 + invariant(
47 + React.isValidElement(element),
48 + 'renderToStream(): You must pass a valid ReactElement.',
49 + );
50 + }
51 return new ReactMarkupReadableStream(element, false);
52 }
53
@@ -53,10 +57,13 @@ function renderToStream(element) {
57 * See https://facebook.github.io/react/docs/react-dom-stream.html#rendertostaticstream
58 */
59 function renderToStaticStream(element) {
56 - invariant(
57 - React.isValidElement(element),
58 - 'renderToStaticStream(): You must pass a valid ReactElement.',
59 - );
60 + const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures;
61 + if (disableNewFiberFeatures) {
62 + invariant(
63 + React.isValidElement(element),
64 + 'renderToStaticStream(): You must pass a valid ReactElement.',
65 + );
66 + }
67 return new ReactMarkupReadableStream(element, true);
68 }
69
src/renderers/dom/ReactDOMStringRenderer.js
+15 -8
@@ -14,6 +14,7 @@
14 var invariant = require('fbjs/lib/invariant');
15 var React = require('react');
16 var ReactPartialRenderer = require('ReactPartialRenderer');
17 +var ReactFeatureFlags = require('ReactFeatureFlags');
18
19 /**
20 * Render a ReactElement to its initial HTML. This should only be used on the
@@ -21,10 +22,13 @@ var ReactPartialRenderer = require('ReactPartialRenderer');
22 * See https://facebook.github.io/react/docs/react-dom-server.html#rendertostring
23 */
24 function renderToString(element) {
24 - invariant(
25 - React.isValidElement(element),
26 - 'renderToString(): You must pass a valid ReactElement.',
27 - );
25 + const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures;
26 + if (disableNewFiberFeatures) {
27 + invariant(
28 + React.isValidElement(element),
29 + 'renderToString(): You must pass a valid ReactElement.',
30 + );
31 + }
32 var renderer = new ReactPartialRenderer(element, false);
33 var markup = renderer.read(Infinity);
34 return markup;
@@ -36,10 +40,13 @@ function renderToString(element) {
40 * See https://facebook.github.io/react/docs/react-dom-server.html#rendertostaticmarkup
41 */
42 function renderToStaticMarkup(element) {
39 - invariant(
40 - React.isValidElement(element),
41 - 'renderToStaticMarkup(): You must pass a valid ReactElement.',
42 - );
43 + const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures;
44 + if (disableNewFiberFeatures) {
45 + invariant(
46 + React.isValidElement(element),
47 + 'renderToStaticMarkup(): You must pass a valid ReactElement.',
48 + );
49 + }
50 var renderer = new ReactPartialRenderer(element, true);
51 var markup = renderer.read(Infinity);
52 return markup;
src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js
+77
@@ -332,6 +332,19 @@ describe('ReactDOMServerIntegration', () => {
332 });
333
334 describe('basic rendering', function() {
335 + beforeEach(() => {
336 + onAfterResetModules = () => {
337 + const ReactFeatureFlags = require('ReactFeatureFlags');
338 + ReactFeatureFlags.disableNewFiberFeatures = false;
339 + };
340 +
341 + resetModules();
342 + });
343 +
344 + afterEach(() => {
345 + onAfterResetModules = null;
346 + });
347 +
348 itRenders('a blank div', async render => {
349 const e = await render(<div />);
350 expect(e.tagName).toBe('DIV');
@@ -347,6 +360,70 @@ describe('ReactDOMServerIntegration', () => {
360 expect(e.childNodes.length).toBe(1);
361 expect(e.firstChild.tagName).toBe('BR');
362 });
363 +
364 + if (ReactDOMFeatureFlags.useFiber) {
365 + itRenders('a array type children as a child', async render => {
366 + let Header = props => {
367 + return <p>header</p>;
368 + };
369 + let Footer = props => {
370 + return [<h2 key={1}>footer</h2>, <h3 key={2}>about</h3>];
371 + };
372 + let e = await render([
373 + <div key={1}>text1</div>,
374 + <span key={2}>text2</span>,
375 + <Header key={3} />,
376 + <Footer key={4} />,
377 + ]);
378 + let parent = e.parentNode;
379 + expect(parent.childNodes[0].tagName).toBe('DIV');
380 + expect(parent.childNodes[1].tagName).toBe('SPAN');
381 + expect(parent.childNodes[2].tagName).toBe('P');
382 + expect(parent.childNodes[3].tagName).toBe('H2');
383 + expect(parent.childNodes[4].tagName).toBe('H3');
384 + });
385 +
386 + itRenders('a single array element children as a child', async render => {
387 + let e = await render([<div key={1}>text1</div>]);
388 + let parent = e.parentNode;
389 + expect(parent.childNodes[0].tagName).toBe('DIV');
390 + });
391 +
392 + itRenders('a nested array children as a child', async render => {
393 + let e = await render([
394 + [<div key={1}>text1</div>],
395 + <span key={1}>text2</span>,
396 + [[[null, <p key={1} />], false]],
397 + ]);
398 + let parent = e.parentNode;
399 + expect(parent.childNodes[0].tagName).toBe('DIV');
400 + expect(parent.childNodes[1].tagName).toBe('SPAN');
401 + expect(parent.childNodes[2].tagName).toBe('P');
402 + });
403 +
404 + itRenders('an iterable as a child', async render => {
405 + const threeDivIterable = {
406 + '@@iterator': function() {
407 + var i = 0;
408 + return {
409 + next: function() {
410 + if (i++ < 3) {
411 + return {value: <div key={i} />, done: false};
412 + } else {
413 + return {value: undefined, done: true};
414 + }
415 + },
416 + };
417 + },
418 + };
419 + let e = await render(threeDivIterable);
420 + let parent = e.parentNode;
421 + expect(parent.childNodes.length).toBe(3);
422 + expect(parent.childNodes[0].tagName).toBe('DIV');
423 + expect(parent.childNodes[1].tagName).toBe('DIV');
424 + expect(parent.childNodes[2].tagName).toBe('DIV');
425 + });
426 + }
427 });
428
429 describe('property to attribute mapping', function() {
src/renderers/dom/shared/__tests__/ReactServerRendering-test.js
+14 -7
@@ -20,6 +20,7 @@ var ReactMarkupChecksum;
20 var ReactReconcileTransaction;
21 var ReactTestUtils;
22 var PropTypes;
23 +var ReactFeatureFlags;
24
25 var ID_ATTRIBUTE_NAME;
26 var ROOT_ATTRIBUTE_NAME;
@@ -35,6 +36,9 @@ describe('ReactDOMServer', () => {
36 ReactReconcileTransaction = require('ReactReconcileTransaction');
37 PropTypes = require('prop-types');
38
39 + ReactFeatureFlags = require('ReactFeatureFlags');
40 + ReactFeatureFlags.disableNewFiberFeatures = false;
41 +
42 ExecutionEnvironment = require('fbjs/lib/ExecutionEnvironment');
43 ExecutionEnvironment.canUseDOM = false;
44 ReactDOMServer = require('react-dom/server');
@@ -319,8 +323,12 @@ describe('ReactDOMServer', () => {
323
324 it('should throw with silly args', () => {
325 expect(
322 - ReactDOMServer.renderToString.bind(ReactDOMServer, 'not a component'),
323 - ).toThrowError('renderToString(): You must pass a valid ReactElement.');
326 + ReactDOMServer.renderToString.bind(ReactDOMServer, {x: 123}),
327 + ).toThrowError(
328 + ReactDOMFeatureFlags.useFiber
329 + ? 'Objects are not valid as a React child (found: object with keys {x})'
330 + : 'renderToString(): You must pass a valid ReactElement.',
331 + );
332 });
333 });
334
@@ -431,12 +439,11 @@ describe('ReactDOMServer', () => {
439
440 it('should throw with silly args', () => {
441 expect(
434 - ReactDOMServer.renderToStaticMarkup.bind(
435 - ReactDOMServer,
436 - 'not a component',
437 - ),
442 + ReactDOMServer.renderToStaticMarkup.bind(ReactDOMServer, {x: 123}),
443 ).toThrowError(
439 - 'renderToStaticMarkup(): You must pass a valid ReactElement.',
444 + ReactDOMFeatureFlags.useFiber
445 + ? 'Objects are not valid as a React child (found: object with keys {x})'
446 + : 'renderToStaticMarkup(): You must pass a valid ReactElement.',
447 );
448 });
449
src/renderers/shared/server/ReactPartialRenderer.js
+18 -7
@@ -307,11 +307,6 @@ function createOpenTagMarkup(
307 }
308
309 function resolve(child, context) {
310 - // TODO: We'll need to support Arrays (and strings) after Fiber is rolled out
311 - invariant(
312 - !Array.isArray(child),
313 - 'The server renderer does not implement support for array children yet.',
314 - );
310 while (React.isValidElement(child)) {
311 if (__DEV__) {
312 pushElementToDebugStack(child);
@@ -418,8 +413,9 @@ function resolve(child, context) {
413
414 class ReactDOMServerRenderer {
415 constructor(element, makeStaticMarkup) {
416 + var children = React.isValidElement(element) ? [element] : toArray(element);
417 var topFrame = {
422 - children: [element],
418 + children,
419 childIndex: 0,
420 context: emptyObject,
421 footer: '',
@@ -487,7 +483,22 @@ class ReactDOMServerRenderer {
483 if (child === null || child === false) {
484 return '';
485 } else {
490 - return this.renderDOM(child, context);
486 + if (React.isValidElement(child)) {
487 + return this.renderDOM(child, context);
488 + } else {
489 + var children = toArray(child);
490 + var frame = {
491 + children,
492 + childIndex: 0,
493 + context: context,
494 + footer: '',
495 + };
496 + if (__DEV__) {
497 + frame.debugElementStack = [];
498 + }
499 + this.stack.push(frame);
500 + return '';
501 + }
502 }
503 }
504 }