From 790e65afe9de3ccb7c43229ed5ee6ec8a12cfa29 Mon Sep 17 00:00:00 2001 From: gzhappysky <451417726@qq.com> Date: Thu, 20 Jul 2017 01:51:44 +0800 Subject: [PATCH 01/19] fix(*): fix renderToString fails with array type children when react-dom/server render --- src/renderers/shared/server/ReactPartialRenderer.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index ffd1e408239d8..56bd3328cbc2e 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -473,6 +473,15 @@ class ReactDOMServerRenderer { if (child === null || child === false) { return ''; } else { + if (Array.isArray(child)) { + this.stack.push({ + children: child, + childIndex: 0, + context: context, + footer: '', + }); + return ''; + } return this.renderDOM(child, context); } } From 6fd78e5e49ea6a7d14969f711b0e280bf3438ac9 Mon Sep 17 00:00:00 2001 From: gzhappysky <451417726@qq.com> Date: Thu, 20 Jul 2017 15:35:12 +0800 Subject: [PATCH 02/19] Add an integration test that verify renderToString with array type children --- .../dom/shared/__tests__/ReactDOMServerIntegration-test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 220c71f9aba70..ce6037a887015 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -328,6 +328,7 @@ describe('ReactDOMServerIntegration', () => { expect(e.childNodes.length).toBe(1); expect(e.firstChild.tagName).toBe('BR'); }); + }); describe('property to attribute mapping', function() { From 4b28bc1918d5f2e8d14356b38084c6cb6a2cd67c Mon Sep 17 00:00:00 2001 From: guoyong Date: Fri, 21 Jul 2017 00:36:27 +0800 Subject: [PATCH 03/19] Add integration test to renderToString with array type child --- .../__tests__/ReactDOMServerIntegration-test.js | 17 +++++++++++++++++ .../shared/server/ReactPartialRenderer.js | 13 +++++++++++++ 2 files changed, 30 insertions(+) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 220c71f9aba70..9425ccea4927b 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -20,6 +20,7 @@ let ReactDOM; let ReactDOMServer; let ReactDOMNodeStream; let ReactTestUtils; +let ReactFeatureFlags; const stream = require('stream'); @@ -288,6 +289,8 @@ function expectMarkupMismatch(serverElement, clientElement) { let onAfterResetModules = null; function resetModules() { jest.resetModuleRegistry(); + ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; PropTypes = require('prop-types'); React = require('react'); ReactDOM = require('react-dom'); @@ -328,6 +331,20 @@ describe('ReactDOMServerIntegration', () => { expect(e.childNodes.length).toBe(1); expect(e.firstChild.tagName).toBe('BR'); }); + + if (ReactDOMFeatureFlags.useFiber) { + itRenders('a array type children as a child', async render => { + class AppComponent extends React.Component { + render() { + return [
text1
,
text2
]; + } + } + const e = await render(); + const parentNode = e.parentNode; + expect(parentNode.childNodes[0].tagName).toBe('DIV'); + expect(parentNode.childNodes[1].tagName).toBe('DIV'); + }); + } }); describe('property to attribute mapping', function() { diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index ecb3b0600d04b..951cced31661a 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -484,6 +484,19 @@ class ReactDOMServerRenderer { if (child === null || child === false) { return ''; } else { + if (Array.isArray(child)) { + var frame = { + children: child, + childIndex: 0, + context: context, + footer: '', + }; + if (__DEV__) { + frame.debugElementStack = []; + } + this.stack.push(frame); + return ''; + } return this.renderDOM(child, context); } } From deb11af24fab7b8e0172636ab913987ece67a108 Mon Sep 17 00:00:00 2001 From: guoyong Date: Fri, 21 Jul 2017 01:21:39 +0800 Subject: [PATCH 04/19] Update integration test to renderToString with array type child --- .../dom/shared/__tests__/ReactDOMServerIntegration-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 9425ccea4927b..cd27085c02952 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -336,13 +336,13 @@ describe('ReactDOMServerIntegration', () => { itRenders('a array type children as a child', async render => { class AppComponent extends React.Component { render() { - return [
text1
,
text2
]; + return [
text1
,

text2

]; } } const e = await render(); const parentNode = e.parentNode; expect(parentNode.childNodes[0].tagName).toBe('DIV'); - expect(parentNode.childNodes[1].tagName).toBe('DIV'); + expect(parentNode.childNodes[1].tagName).toBe('P'); }); } }); From 62c042e707402646943fe56524499cc03a419c73 Mon Sep 17 00:00:00 2001 From: guoyong Date: Fri, 21 Jul 2017 03:05:37 +0800 Subject: [PATCH 05/19] fix renderToString fails with array type children when react-dom/server render --- src/renderers/dom/ReactDOMStringRenderer.js | 4 +-- .../ReactDOMServerIntegration-test.js | 25 +++++++++---------- .../shared/server/ReactPartialRenderer.js | 4 +-- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/renderers/dom/ReactDOMStringRenderer.js b/src/renderers/dom/ReactDOMStringRenderer.js index 7995ac3581bcd..6f0363c993d69 100644 --- a/src/renderers/dom/ReactDOMStringRenderer.js +++ b/src/renderers/dom/ReactDOMStringRenderer.js @@ -22,7 +22,7 @@ var ReactPartialRenderer = require('ReactPartialRenderer'); */ function renderToString(element) { invariant( - React.isValidElement(element), + Array.isArray(element) || React.isValidElement(element), 'renderToString(): You must pass a valid ReactElement.', ); var renderer = new ReactPartialRenderer(element, false); @@ -37,7 +37,7 @@ function renderToString(element) { */ function renderToStaticMarkup(element) { invariant( - React.isValidElement(element), + Array.isArray(element) || React.isValidElement(element), 'renderToStaticMarkup(): You must pass a valid ReactElement.', ); var renderer = new ReactPartialRenderer(element, true); diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index cd27085c02952..4ec0375704853 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -20,7 +20,6 @@ let ReactDOM; let ReactDOMServer; let ReactDOMNodeStream; let ReactTestUtils; -let ReactFeatureFlags; const stream = require('stream'); @@ -289,8 +288,6 @@ function expectMarkupMismatch(serverElement, clientElement) { let onAfterResetModules = null; function resetModules() { jest.resetModuleRegistry(); - ReactFeatureFlags = require('ReactFeatureFlags'); - ReactFeatureFlags.disableNewFiberFeatures = false; PropTypes = require('prop-types'); React = require('react'); ReactDOM = require('react-dom'); @@ -333,16 +330,18 @@ describe('ReactDOMServerIntegration', () => { }); if (ReactDOMFeatureFlags.useFiber) { - itRenders('a array type children as a child', async render => { - class AppComponent extends React.Component { - render() { - return [
text1
,

text2

]; - } - } - const e = await render(); - const parentNode = e.parentNode; - expect(parentNode.childNodes[0].tagName).toBe('DIV'); - expect(parentNode.childNodes[1].tagName).toBe('P'); + it('a array type children as a child', () => { + spyOn(console, 'error'); + let markup = ReactDOMServer.renderToString([ +
text1
, +

text2

, + ]); + + const root = document.createElement('div'); + root.innerHTML = markup; + expect(root.childNodes[0].tagName).toBe('DIV'); + expect(root.childNodes[1].tagName).toBe('P'); + expectDev(console.error.calls.count()).toBe(0); }); } }); diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index 951cced31661a..25773a6523437 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -307,8 +307,6 @@ function createOpenTagMarkup( } function resolve(child, context) { - // TODO: We'll need to support Arrays (and strings) after Fiber is rolled out - invariant(!Array.isArray(child), 'Did not expect to receive an Array child'); while (React.isValidElement(child)) { if (__DEV__) { pushElementToDebugStack(child); @@ -416,7 +414,7 @@ function resolve(child, context) { class ReactDOMServerRenderer { constructor(element, makeStaticMarkup) { var topFrame = { - children: [element], + children: Array.isArray() ? element : [element], childIndex: 0, context: emptyObject, footer: '', From 6b4eb0e32a5e6692a14b2748f981bb900638f7be Mon Sep 17 00:00:00 2001 From: guoyong Date: Fri, 21 Jul 2017 10:17:05 +0800 Subject: [PATCH 06/19] Update integration test to renderToString with array type child --- .../dom/ReactDOMNodeStreamRenderer.js | 4 --- .../ReactDOMServerIntegration-test.js | 30 +++++++++++-------- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/src/renderers/dom/ReactDOMNodeStreamRenderer.js b/src/renderers/dom/ReactDOMNodeStreamRenderer.js index 890538bb6fbd4..689d339d81ef9 100644 --- a/src/renderers/dom/ReactDOMNodeStreamRenderer.js +++ b/src/renderers/dom/ReactDOMNodeStreamRenderer.js @@ -40,10 +40,6 @@ class ReactMarkupReadableStream extends Readable { * See https://facebook.github.io/react/docs/react-dom-stream.html#rendertostream */ function renderToStream(element) { - invariant( - React.isValidElement(element), - 'renderToStream(): You must pass a valid ReactElement.', - ); return new ReactMarkupReadableStream(element, false); } diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 4ec0375704853..3b47d070bd099 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -313,6 +313,19 @@ describe('ReactDOMServerIntegration', () => { }); describe('basic rendering', function() { + beforeEach(() => { + onAfterResetModules = () => { + const ReactFeatureFlags = require('ReactFeatureFlags'); + ReactFeatureFlags.disableNewFiberFeatures = false; + }; + + resetModules(); + }); + + afterEach(() => { + onAfterResetModules = null; + }); + itRenders('a blank div', async render => { const e = await render(
); expect(e.tagName).toBe('DIV'); @@ -330,18 +343,11 @@ describe('ReactDOMServerIntegration', () => { }); if (ReactDOMFeatureFlags.useFiber) { - it('a array type children as a child', () => { - spyOn(console, 'error'); - let markup = ReactDOMServer.renderToString([ -
text1
, -

text2

, - ]); - - const root = document.createElement('div'); - root.innerHTML = markup; - expect(root.childNodes[0].tagName).toBe('DIV'); - expect(root.childNodes[1].tagName).toBe('P'); - expectDev(console.error.calls.count()).toBe(0); + itRenders('a array type children as a child', async render => { + let e = await render([
text1
,

text2

]); + let parent = e.parentNode; + expect(parent.childNodes[0].tagName).toBe('DIV'); + expect(parent.childNodes[1].tagName).toBe('P'); }); } }); From e53e5526244d71274a1b04eb42652308d1621b66 Mon Sep 17 00:00:00 2001 From: guoyong Date: Fri, 21 Jul 2017 11:54:18 +0800 Subject: [PATCH 07/19] Add to iterate that are not arrays --- src/renderers/shared/server/ReactPartialRenderer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index 25773a6523437..dda2462ab84ab 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -414,7 +414,7 @@ function resolve(child, context) { class ReactDOMServerRenderer { constructor(element, makeStaticMarkup) { var topFrame = { - children: Array.isArray() ? element : [element], + children: toArray(element), childIndex: 0, context: emptyObject, footer: '', From eafb4bdf4c5723230d12eb3061054ffa3230f998 Mon Sep 17 00:00:00 2001 From: guoyong Date: Sat, 22 Jul 2017 03:51:46 +0800 Subject: [PATCH 08/19] Add the validation react element --- src/renderers/dom/ReactDOMStringRenderer.js | 23 ++- .../__tests__/ReactServerRendering-test.js | 164 +++++++++--------- 2 files changed, 100 insertions(+), 87 deletions(-) diff --git a/src/renderers/dom/ReactDOMStringRenderer.js b/src/renderers/dom/ReactDOMStringRenderer.js index 6f0363c993d69..7461c3297c837 100644 --- a/src/renderers/dom/ReactDOMStringRenderer.js +++ b/src/renderers/dom/ReactDOMStringRenderer.js @@ -14,6 +14,7 @@ var invariant = require('fbjs/lib/invariant'); var React = require('react'); var ReactPartialRenderer = require('ReactPartialRenderer'); +var ReactFeatureFlags = require('ReactFeatureFlags'); /** * Render a ReactElement to its initial HTML. This should only be used on the @@ -21,10 +22,13 @@ var ReactPartialRenderer = require('ReactPartialRenderer'); * See https://facebook.github.io/react/docs/react-dom-server.html#rendertostring */ function renderToString(element) { - invariant( - Array.isArray(element) || React.isValidElement(element), - 'renderToString(): You must pass a valid ReactElement.', - ); + const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures; + if (disableNewFiberFeatures) { + invariant( + React.isValidElement(element), + 'renderToString(): You must pass a valid ReactElement.', + ); + } var renderer = new ReactPartialRenderer(element, false); var markup = renderer.read(Infinity); return markup; @@ -36,10 +40,13 @@ function renderToString(element) { * See https://facebook.github.io/react/docs/react-dom-server.html#rendertostaticmarkup */ function renderToStaticMarkup(element) { - invariant( - Array.isArray(element) || React.isValidElement(element), - 'renderToStaticMarkup(): You must pass a valid ReactElement.', - ); + const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures; + if (disableNewFiberFeatures) { + invariant( + React.isValidElement(element), + 'renderToStaticMarkup(): You must pass a valid ReactElement.', + ); + } var renderer = new ReactPartialRenderer(element, true); var markup = renderer.read(Infinity); return markup; diff --git a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js index b768f3bef2bb2..53d50e85ff048 100644 --- a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js @@ -20,6 +20,7 @@ var ReactMarkupChecksum; var ReactReconcileTransaction; var ReactTestUtils; var PropTypes; +var ReactFeatureFlags; var ID_ATTRIBUTE_NAME; var ROOT_ATTRIBUTE_NAME; @@ -34,6 +35,7 @@ describe('ReactDOMServer', () => { ReactMarkupChecksum = require('ReactMarkupChecksum'); ReactReconcileTransaction = require('ReactReconcileTransaction'); PropTypes = require('prop-types'); + ReactFeatureFlags = require('ReactFeatureFlags'); ExecutionEnvironment = require('fbjs/lib/ExecutionEnvironment'); ExecutionEnvironment.canUseDOM = false; @@ -50,14 +52,14 @@ describe('ReactDOMServer', () => { expect(response).toMatch( new RegExp( 'hello world', + ROOT_ATTRIBUTE_NAME + + '="" ' + + ID_ATTRIBUTE_NAME + + '="[^"]*"' + + (ReactDOMFeatureFlags.useFiber + ? '' + : ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+"') + + '>hello world', ), ); }); @@ -67,14 +69,14 @@ describe('ReactDOMServer', () => { expect(response).toMatch( new RegExp( '', + ROOT_ATTRIBUTE_NAME + + '="" ' + + ID_ATTRIBUTE_NAME + + '="[^"]*"' + + (ReactDOMFeatureFlags.useFiber + ? '' + : ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+"') + + '/>', ), ); }); @@ -84,14 +86,14 @@ describe('ReactDOMServer', () => { expect(response).toMatch( new RegExp( '', + ROOT_ATTRIBUTE_NAME + + '="" ' + + ID_ATTRIBUTE_NAME + + '="[^"]*"' + + (ReactDOMFeatureFlags.useFiber + ? '' + : ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+"') + + '/>', ), ); }); @@ -130,23 +132,23 @@ describe('ReactDOMServer', () => { expect(response).toMatch( new RegExp( '
' + - '' + - (ReactDOMFeatureFlags.useFiber - ? 'My name is child' - : 'My name is ' + - 'child') + - '' + - '
', + ROOT_ATTRIBUTE_NAME + + '="" ' + + ID_ATTRIBUTE_NAME + + '="[^"]*"' + + (ReactDOMFeatureFlags.useFiber + ? '' + : ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+"') + + '>' + + '' + + (ReactDOMFeatureFlags.useFiber + ? 'My name is child' + : 'My name is ' + + 'child') + + '' + + '
', ), ); }); @@ -159,7 +161,7 @@ describe('ReactDOMServer', () => { constructor(props) { super(props); lifecycle.push('getInitialState'); - this.state = {name: 'TestComponent'}; + this.state = { name: 'TestComponent' }; } componentWillMount() { @@ -201,19 +203,19 @@ describe('ReactDOMServer', () => { expect(response).toMatch( new RegExp( '' + - (ReactDOMFeatureFlags.useFiber - ? 'Component name: TestComponent' - : 'Component name: ' + - 'TestComponent') + - '', + ROOT_ATTRIBUTE_NAME + + '="" ' + + ID_ATTRIBUTE_NAME + + '="[^"]*"' + + (ReactDOMFeatureFlags.useFiber + ? '' + : ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+"') + + '>' + + (ReactDOMFeatureFlags.useFiber + ? 'Component name: TestComponent' + : 'Component name: ' + + 'TestComponent') + + '', ), ); expect(lifecycle).toEqual([ @@ -375,7 +377,7 @@ describe('ReactDOMServer', () => { constructor(props) { super(props); lifecycle.push('getInitialState'); - this.state = {name: 'TestComponent'}; + this.state = { name: 'TestComponent' }; } componentWillMount() { @@ -429,21 +431,25 @@ describe('ReactDOMServer', () => { runTest(); }); + it('should throw with silly args', () => { - expect( - ReactDOMServer.renderToStaticMarkup.bind( - ReactDOMServer, - 'not a component', - ), - ).toThrowError( - 'renderToStaticMarkup(): You must pass a valid ReactElement.', - ); + const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures; + if (disableNewFiberFeatures) { + expect( + ReactDOMServer.renderToStaticMarkup.bind( + ReactDOMServer, + 'not a component', + ), + ).toThrowError( + 'renderToStaticMarkup(): You must pass a valid ReactElement.', + ); + } }); it('allows setState in componentWillMount without using DOM', () => { class Component extends React.Component { componentWillMount() { - this.setState({text: 'hello, world'}); + this.setState({ text: 'hello, world' }); } render() { @@ -451,7 +457,7 @@ describe('ReactDOMServer', () => { } } - ReactReconcileTransaction.prototype.perform = function() { + ReactReconcileTransaction.prototype.perform = function () { // We shouldn't ever be calling this on the server throw new Error('Browser reconcile transaction should not be used'); }; @@ -463,11 +469,11 @@ describe('ReactDOMServer', () => { class Component extends React.Component { constructor() { super(); - this.state = {text: 'default state'}; + this.state = { text: 'default state' }; } componentWillMount() { - this.setState({text: 'hello, world'}); + this.setState({ text: 'hello, world' }); } render() { @@ -475,7 +481,7 @@ describe('ReactDOMServer', () => { } } - ReactReconcileTransaction.prototype.perform = function() { + ReactReconcileTransaction.prototype.perform = function () { // We shouldn't ever be calling this on the server throw new Error('Browser reconcile transaction should not be used'); }; @@ -545,13 +551,13 @@ describe('ReactDOMServer', () => { , ); - return
; + return
; } } class Component extends React.Component { componentWillMount() { - this.setState({text: 'hello, world'}); + this.setState({ text: 'hello, world' }); } render() { @@ -574,13 +580,13 @@ describe('ReactDOMServer', () => { it('warns with a no-op when an async setState is triggered', () => { class Foo extends React.Component { componentWillMount() { - this.setState({text: 'hello'}); + this.setState({ text: 'hello' }); setTimeout(() => { - this.setState({text: 'error'}); + this.setState({ text: 'error' }); }); } render() { - return
{}}>{this.state.text}
; + return
{ }}>{this.state.text}
; } } @@ -590,8 +596,8 @@ describe('ReactDOMServer', () => { expectDev(console.error.calls.count()).toBe(1); expectDev(console.error.calls.mostRecent().args[0]).toBe( 'Warning: setState(...): Can only update a mounting component.' + - ' This usually means you called setState() outside componentWillMount() on the server.' + - ' This is a no-op.\n\nPlease check the code for the Foo component.', + ' This usually means you called setState() outside componentWillMount() on the server.' + + ' This is a no-op.\n\nPlease check the code for the Foo component.', ); var markup = ReactDOMServer.renderToStaticMarkup(); expect(markup).toBe('
hello
'); @@ -607,7 +613,7 @@ describe('ReactDOMServer', () => { } render() { - return
{}} />; + return
{ }} />; } } @@ -617,8 +623,8 @@ describe('ReactDOMServer', () => { expectDev(console.error.calls.count()).toBe(1); expectDev(console.error.calls.mostRecent().args[0]).toBe( 'Warning: forceUpdate(...): Can only update a mounting component. ' + - 'This usually means you called forceUpdate() outside componentWillMount() on the server. ' + - 'This is a no-op.\n\nPlease check the code for the Baz component.', + 'This usually means you called forceUpdate() outside componentWillMount() on the server. ' + + 'This is a no-op.\n\nPlease check the code for the Baz component.', ); var markup = ReactDOMServer.renderToStaticMarkup(); expect(markup).toBe('
'); From 3df964dea7bd95e26254296a71395be003903de9 Mon Sep 17 00:00:00 2001 From: guoyong Date: Sat, 22 Jul 2017 11:29:35 +0800 Subject: [PATCH 09/19] Improve an integration test of server renderToString with array type --- .../__tests__/ReactDOMServerIntegration-test.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 3b47d070bd099..83363f951c68b 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -344,10 +344,18 @@ describe('ReactDOMServerIntegration', () => { if (ReactDOMFeatureFlags.useFiber) { itRenders('a array type children as a child', async render => { - let e = await render([
text1
,

text2

]); + let Header = props => { + return

header

; + }; + let e = await render([ +
text1
, + text2, +
, + ]); let parent = e.parentNode; expect(parent.childNodes[0].tagName).toBe('DIV'); - expect(parent.childNodes[1].tagName).toBe('P'); + expect(parent.childNodes[1].tagName).toBe('SPAN'); + expect(parent.childNodes[2].tagName).toBe('P'); }); } }); From 02101d5d4acccce66b303535aeea5bb07d9a384f Mon Sep 17 00:00:00 2001 From: guoyong Date: Sat, 22 Jul 2017 13:38:25 +0800 Subject: [PATCH 10/19] prettier --- .../__tests__/ReactServerRendering-test.js | 143 +++++++++--------- 1 file changed, 71 insertions(+), 72 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js index 53d50e85ff048..9caf9196b23d7 100644 --- a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js @@ -52,14 +52,14 @@ describe('ReactDOMServer', () => { expect(response).toMatch( new RegExp( 'hello world', + ROOT_ATTRIBUTE_NAME + + '="" ' + + ID_ATTRIBUTE_NAME + + '="[^"]*"' + + (ReactDOMFeatureFlags.useFiber + ? '' + : ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+"') + + '>hello world', ), ); }); @@ -69,14 +69,14 @@ describe('ReactDOMServer', () => { expect(response).toMatch( new RegExp( '', + ROOT_ATTRIBUTE_NAME + + '="" ' + + ID_ATTRIBUTE_NAME + + '="[^"]*"' + + (ReactDOMFeatureFlags.useFiber + ? '' + : ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+"') + + '/>', ), ); }); @@ -86,14 +86,14 @@ describe('ReactDOMServer', () => { expect(response).toMatch( new RegExp( '', + ROOT_ATTRIBUTE_NAME + + '="" ' + + ID_ATTRIBUTE_NAME + + '="[^"]*"' + + (ReactDOMFeatureFlags.useFiber + ? '' + : ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+"') + + '/>', ), ); }); @@ -132,23 +132,23 @@ describe('ReactDOMServer', () => { expect(response).toMatch( new RegExp( '
' + - '' + - (ReactDOMFeatureFlags.useFiber - ? 'My name is child' - : 'My name is ' + - 'child') + - '' + - '
', + ROOT_ATTRIBUTE_NAME + + '="" ' + + ID_ATTRIBUTE_NAME + + '="[^"]*"' + + (ReactDOMFeatureFlags.useFiber + ? '' + : ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+"') + + '>' + + '' + + (ReactDOMFeatureFlags.useFiber + ? 'My name is child' + : 'My name is ' + + 'child') + + '' + + '
', ), ); }); @@ -161,7 +161,7 @@ describe('ReactDOMServer', () => { constructor(props) { super(props); lifecycle.push('getInitialState'); - this.state = { name: 'TestComponent' }; + this.state = {name: 'TestComponent'}; } componentWillMount() { @@ -203,19 +203,19 @@ describe('ReactDOMServer', () => { expect(response).toMatch( new RegExp( '' + - (ReactDOMFeatureFlags.useFiber - ? 'Component name: TestComponent' - : 'Component name: ' + - 'TestComponent') + - '', + ROOT_ATTRIBUTE_NAME + + '="" ' + + ID_ATTRIBUTE_NAME + + '="[^"]*"' + + (ReactDOMFeatureFlags.useFiber + ? '' + : ' ' + ReactMarkupChecksum.CHECKSUM_ATTR_NAME + '="[^"]+"') + + '>' + + (ReactDOMFeatureFlags.useFiber + ? 'Component name: TestComponent' + : 'Component name: ' + + 'TestComponent') + + '', ), ); expect(lifecycle).toEqual([ @@ -377,7 +377,7 @@ describe('ReactDOMServer', () => { constructor(props) { super(props); lifecycle.push('getInitialState'); - this.state = { name: 'TestComponent' }; + this.state = {name: 'TestComponent'}; } componentWillMount() { @@ -431,7 +431,6 @@ describe('ReactDOMServer', () => { runTest(); }); - it('should throw with silly args', () => { const disableNewFiberFeatures = ReactFeatureFlags.disableNewFiberFeatures; if (disableNewFiberFeatures) { @@ -449,7 +448,7 @@ describe('ReactDOMServer', () => { it('allows setState in componentWillMount without using DOM', () => { class Component extends React.Component { componentWillMount() { - this.setState({ text: 'hello, world' }); + this.setState({text: 'hello, world'}); } render() { @@ -457,7 +456,7 @@ describe('ReactDOMServer', () => { } } - ReactReconcileTransaction.prototype.perform = function () { + ReactReconcileTransaction.prototype.perform = function() { // We shouldn't ever be calling this on the server throw new Error('Browser reconcile transaction should not be used'); }; @@ -469,11 +468,11 @@ describe('ReactDOMServer', () => { class Component extends React.Component { constructor() { super(); - this.state = { text: 'default state' }; + this.state = {text: 'default state'}; } componentWillMount() { - this.setState({ text: 'hello, world' }); + this.setState({text: 'hello, world'}); } render() { @@ -481,7 +480,7 @@ describe('ReactDOMServer', () => { } } - ReactReconcileTransaction.prototype.perform = function () { + ReactReconcileTransaction.prototype.perform = function() { // We shouldn't ever be calling this on the server throw new Error('Browser reconcile transaction should not be used'); }; @@ -551,13 +550,13 @@ describe('ReactDOMServer', () => {
, ); - return
; + return
; } } class Component extends React.Component { componentWillMount() { - this.setState({ text: 'hello, world' }); + this.setState({text: 'hello, world'}); } render() { @@ -580,13 +579,13 @@ describe('ReactDOMServer', () => { it('warns with a no-op when an async setState is triggered', () => { class Foo extends React.Component { componentWillMount() { - this.setState({ text: 'hello' }); + this.setState({text: 'hello'}); setTimeout(() => { - this.setState({ text: 'error' }); + this.setState({text: 'error'}); }); } render() { - return
{ }}>{this.state.text}
; + return
{}}>{this.state.text}
; } } @@ -596,8 +595,8 @@ describe('ReactDOMServer', () => { expectDev(console.error.calls.count()).toBe(1); expectDev(console.error.calls.mostRecent().args[0]).toBe( 'Warning: setState(...): Can only update a mounting component.' + - ' This usually means you called setState() outside componentWillMount() on the server.' + - ' This is a no-op.\n\nPlease check the code for the Foo component.', + ' This usually means you called setState() outside componentWillMount() on the server.' + + ' This is a no-op.\n\nPlease check the code for the Foo component.', ); var markup = ReactDOMServer.renderToStaticMarkup(); expect(markup).toBe('
hello
'); @@ -613,7 +612,7 @@ describe('ReactDOMServer', () => { } render() { - return
{ }} />; + return
{}} />; } } @@ -623,8 +622,8 @@ describe('ReactDOMServer', () => { expectDev(console.error.calls.count()).toBe(1); expectDev(console.error.calls.mostRecent().args[0]).toBe( 'Warning: forceUpdate(...): Can only update a mounting component. ' + - 'This usually means you called forceUpdate() outside componentWillMount() on the server. ' + - 'This is a no-op.\n\nPlease check the code for the Baz component.', + 'This usually means you called forceUpdate() outside componentWillMount() on the server. ' + + 'This is a no-op.\n\nPlease check the code for the Baz component.', ); var markup = ReactDOMServer.renderToStaticMarkup(); expect(markup).toBe('
'); From 5f8a2ddaaa927733b1cb589cab80cfef739597ec Mon Sep 17 00:00:00 2001 From: guoyong Date: Fri, 28 Jul 2017 11:33:50 +0800 Subject: [PATCH 11/19] prettier --- src/renderers/shared/server/ReactPartialRenderer.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/renderers/shared/server/ReactPartialRenderer.js b/src/renderers/shared/server/ReactPartialRenderer.js index afefefabdd8f4..4e8f33ec6f329 100644 --- a/src/renderers/shared/server/ReactPartialRenderer.js +++ b/src/renderers/shared/server/ReactPartialRenderer.js @@ -306,7 +306,7 @@ function createOpenTagMarkup( return ret; } -function resolve(child, context) { +function resolve(child, context) { while (React.isValidElement(child)) { if (__DEV__) { pushElementToDebugStack(child); @@ -482,9 +482,10 @@ class ReactDOMServerRenderer { if (child === null || child === false) { return ''; } else { - if (Array.isArray(child)) { + var children = toArray(child); + if (children.length > 1) { var frame = { - children: child, + children, childIndex: 0, context: context, footer: '', From cf005dd11f6b9dd3d9d75852bfbd40e86c272591 Mon Sep 17 00:00:00 2001 From: guoyong Date: Sat, 29 Jul 2017 18:58:04 +0800 Subject: [PATCH 12/19] Make SSR can handle a single array element and nested array --- .../ReactDOMServerIntegration-test.js | 265 +++++++++++------- .../shared/server/ReactPartialRenderer.js | 10 +- 2 files changed, 174 insertions(+), 101 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 408f63c64f5c7..77c7b1302efe0 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -25,6 +25,9 @@ const stream = require('stream'); // Helper functions for rendering tests // ==================================== +function normalizeCodeLocInfo(str) { + return str && str.replace(/\(at .+?:\d+\)/g, '(at **)'); +} // promisified version of ReactDOM.render() function asyncReactDOMRender(reactElement, domElement) { @@ -149,7 +152,7 @@ const clientRenderOnServerString = async (element, errorCount = 0) => { return clientElement; }; -function BadMarkupExpected() {} +function BadMarkupExpected() { } const clientRenderOnBadMarkup = async (element, errorCount = 0) => { // First we render the top of bad mark up. @@ -232,8 +235,8 @@ function itThrows(desc, testFn) { return testFn() .then(() => expect(false).toBe('The promise resolved and should not have.'), - ) - .catch(() => {}); + ) + .catch(() => { }); }); } @@ -331,7 +334,7 @@ describe('ReactDOMServerIntegration', () => { ExecutionEnvironment.canUseDOM = false; }); - describe('basic rendering', function() { + describe('basic rendering', function () { beforeEach(() => { onAfterResetModules = () => { const ReactFeatureFlags = require('ReactFeatureFlags'); @@ -376,11 +379,79 @@ describe('ReactDOMServerIntegration', () => { expect(parent.childNodes[1].tagName).toBe('SPAN'); expect(parent.childNodes[2].tagName).toBe('P'); }); + + itRenders('a single array element children as a child', async render => { + let e = await render([
text1
]); + let parent = e.parentNode; + expect(parent.childNodes[0].tagName).toBe('DIV'); + }); + + itRenders('a nested array children as a child', async render => { + let e = await render([[
text1
],text2]); + let parent = e.parentNode; + expect(parent.childNodes[0].tagName).toBe('DIV'); + expect(parent.childNodes[1].tagName).toBe('SPAN'); + }); + + itRenders( + 'throws if a plain object is used as a child on SSR', + async () => { + var children = { + x: , + y: , + z: , + }; + var element =
{[children]}
; + var ex; + try { + ReactDOMServer.renderToString(element); + } catch (e) { + ex = e; + } + expect(ex).toBeDefined(); + expect(normalizeCodeLocInfo(ex.message)).toBe( + 'Objects are not valid as a React child (found: object with keys ' + + '{x, y, z}). If you meant to render a collection of children, use ' + + 'an array instead.' + + // Fiber gives a slightly better stack with the nearest host components + (ReactDOMFeatureFlags.useFiber ? '\n in div (at **)' : ''), + ); + }, + ); + + it('throws if a plain object even if it is in an owner on SSR', async () => { + class Foo extends React.Component { + render() { + var children = { + a: , + b: , + c: , + }; + return
{[children]}
; + } + } + var container = document.createElement('div'); + var ex; + try { + ReactDOMServer.renderToString(, container); + } catch (e) { + ex = e; + } + expect(ex).toBeDefined(); + expect(normalizeCodeLocInfo(ex.message)).toBe( + 'Objects are not valid as a React child (found: object with keys ' + + '{a, b, c}). If you meant to render a collection of children, use ' + + 'an array instead.\n' + + // Fiber gives a slightly better stack with the nearest host components + (ReactDOMFeatureFlags.useFiber ? ' in div (at **)\n' : '') + + ' in Foo (at **)', + ); + }); } }); - describe('property to attribute mapping', function() { - describe('string properties', function() { + describe('property to attribute mapping', function () { + describe('string properties', function () { itRenders('simple numbers', async render => { const e = await render(
); expect(e.getAttribute('width')).toBe('30'); @@ -409,7 +480,7 @@ describe('ReactDOMServerIntegration', () => { }); }); - describe('boolean properties', function() { + describe('boolean properties', function () { itRenders('boolean prop with true value', async render => { const e = await render(