From 437d0299fa79ab053ec8c21a5071ecb8afb1f823 Mon Sep 17 00:00:00 2001 From: Sasha Aickin Date: Thu, 23 Feb 2017 13:02:32 -0800 Subject: [PATCH 1/9] Added a handful of SSR unit tests, ported from a previous pull request. --- .../__tests__/ReactServerRendering-test.js | 158 ++++++++++++++++-- 1 file changed, 146 insertions(+), 12 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js index 296bb572b576b..add5e0a49aad2 100644 --- a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js @@ -23,20 +23,132 @@ var ReactTestUtils; var ID_ATTRIBUTE_NAME; var ROOT_ATTRIBUTE_NAME; +// performs fn asynchronously and expects count errors logged to console.error. +// will fail the test if the count of errors logged is not equal to count. +function expectErrors(fn, count) { + if (console.error.calls && console.error.calls.reset) { + console.error.calls.reset(); + } else { + spyOn(console, "error"); + } + + return fn().then((result) => { + expect(console.error.calls.count()).toBe(count); + if (console.error.calls.count() !== count) { + console.log(`We expected ${count} warning(s), but saw ${console.calls.count().length} warning(s).`); + if (console.error.calls.count() > 0) { + console.log(`We saw these warnings:`); + for (var i = 0; i < console.error.calls.count(); i++) { + console.log(console.error.calls.argsFor(i)[0]); + } + } + } + return result; + }); +} + +// renders the reactElement into domElement, and expects a certain number of errors. +// returns a Promise that resolves when the render is complete. +function renderIntoDom(reactElement, domElement, errorCount = 0) { + return expectErrors( + () => new Promise((resolve) => ReactDOM.render(reactElement, domElement, () => resolve(domElement.firstChild))), + errorCount + ); +} + +// Renders text using SSR and then stuffs it into a DOM node; returns the DOM +// element that corresponds with the reactElement. +// Does not render on client or perform client-side revival. +function serverRender(reactElement, errorCount = 0) { + return expectErrors( + () => Promise.resolve(ReactDOMServer.renderToString(reactElement)), + errorCount) + .then((markup) => { + var domElement = document.createElement('div'); + domElement.innerHTML = markup; + return domElement.firstChild; + }); +} + +const clientCleanRender = (element, errorCount = 0) => { + const div = document.createElement('div'); + return renderIntoDom(element, div, errorCount); +}; + +const clientRenderOnServerString = (element, errorCount = 0) => { + return serverRender(element, errorCount).then((markup) => { + resetModules(); + var domElement = document.createElement('div'); + domElement.innerHTML = markup; + return renderIntoDom(element, domElement, errorCount); + }) +}; + +const clientRenderOnBadMarkup = (element, errorCount = 0) => { + var domElement = document.createElement('div'); + domElement.innerHTML = '
'; + return renderIntoDom(element, domElement, errorCount + 1); +}; + +// runs a DOM rendering test as four different tests, with four different rendering +// scenarios: +// -- render to string on server +// -- render on client without any server markup "clean client render" +// -- render on client on top of good server-generated string markup +// -- render on client on top of bad server-generated markup +// +// testFn is a test that has one arg, which is a render function. the render +// function takes in a ReactElement and an optional expected error count and +// returns a promise of a DOM Element. +// +// You should only perform tests that examine the DOM of the results of +// render; you should not depend on the interactivity of the returned DOM element, +// as that will not work in the server string scenario. +function itRenders(desc, testFn) { + it(`${desc} with server string render`, + () => testFn(serverRender)); + itClientRenders(desc, testFn); +} + +// run testFn in three different rendering scenarios: +// -- render on client without any server markup "clean client render" +// -- render on client on top of good server-generated string markup +// -- render on client on top of bad server-generated markup +// +// testFn is a test that has one arg, which is a render function. the render +// function takes in a ReactElement and an optional expected error count and +// returns a promise of a DOM Element. +// +// Since all of the renders in this function are on the client, you can test interactivity, +// unlike with itRenders. +function itClientRenders(desc, testFn) { + it(`${desc} with clean client render`, + () => testFn(clientCleanRender)); + it(`${desc} with client render on top of good server markup`, + () => testFn(clientRenderOnServerString)); + it(`${desc} with client render on top of bad server markup`, + () => testFn(clientRenderOnBadMarkup)); +} + +function resetModules() { + jest.resetModuleRegistry(); + React = require('React'); + ReactDOM = require('ReactDOM'); + ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); + ReactMarkupChecksum = require('ReactMarkupChecksum'); + ReactTestUtils = require('ReactTestUtils'); + ReactReconcileTransaction = require('ReactReconcileTransaction'); + + ExecutionEnvironment = require('ExecutionEnvironment'); + ExecutionEnvironment.canUseDOM = false; + ReactDOMServer = require('ReactDOMServer'); + + +} + describe('ReactDOMServer', () => { beforeEach(() => { - jest.resetModules(); - React = require('React'); - ReactDOM = require('ReactDOM'); - ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); - ReactMarkupChecksum = require('ReactMarkupChecksum'); - ReactTestUtils = require('ReactTestUtils'); - ReactReconcileTransaction = require('ReactReconcileTransaction'); - - ExecutionEnvironment = require('ExecutionEnvironment'); - ExecutionEnvironment.canUseDOM = false; - ReactDOMServer = require('ReactDOMServer'); - + resetModules(); var DOMProperty = require('DOMProperty'); ID_ATTRIBUTE_NAME = DOMProperty.ID_ATTRIBUTE_NAME; ROOT_ATTRIBUTE_NAME = DOMProperty.ROOT_ATTRIBUTE_NAME; @@ -279,6 +391,28 @@ describe('ReactDOMServer', () => { expect(numClicks).toEqual(2); }); + describe('basic rendering', function() { + itRenders('should render a blank div', render => + render(
).then(e => expect(e.tagName.toLowerCase()).toBe('div'))); + + itRenders('should render a div with inline styles', render => + render(
).then(e => { + expect(e.style.color).toBe('red'); + expect(e.style.width).toBe('30px'); + }) + ); + + itRenders('should render a self-closing tag', render => + render(
).then(e => expect(e.tagName.toLowerCase()).toBe('br'))); + + itRenders('should render a self-closing tag as a child', render => + render(

).then(e => { + expect(e.childNodes.length).toBe(1); + expect(e.firstChild.tagName.toLowerCase()).toBe('br'); + }) + ); + }); + it('should throw with silly args', () => { expect( ReactDOMServer.renderToString.bind( From 7c85408323104f9de5a2ccb352c2334e31ae3495 Mon Sep 17 00:00:00 2001 From: Sasha Aickin Date: Thu, 23 Feb 2017 13:05:37 -0800 Subject: [PATCH 2/9] Fixing linting errors --- .../dom/shared/__tests__/ReactServerRendering-test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js index add5e0a49aad2..5466f95f49c58 100644 --- a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js @@ -29,7 +29,7 @@ function expectErrors(fn, count) { if (console.error.calls && console.error.calls.reset) { console.error.calls.reset(); } else { - spyOn(console, "error"); + spyOn(console, 'error'); } return fn().then((result) => { @@ -71,8 +71,8 @@ function serverRender(reactElement, errorCount = 0) { } const clientCleanRender = (element, errorCount = 0) => { - const div = document.createElement('div'); - return renderIntoDom(element, div, errorCount); + const div = document.createElement('div'); + return renderIntoDom(element, div, errorCount); }; const clientRenderOnServerString = (element, errorCount = 0) => { @@ -81,7 +81,7 @@ const clientRenderOnServerString = (element, errorCount = 0) => { var domElement = document.createElement('div'); domElement.innerHTML = markup; return renderIntoDom(element, domElement, errorCount); - }) + }); }; const clientRenderOnBadMarkup = (element, errorCount = 0) => { From c1d9787f493bf338be125ebb9f31f820d9495a4d Mon Sep 17 00:00:00 2001 From: Sasha Aickin Date: Thu, 23 Feb 2017 15:30:30 -0800 Subject: [PATCH 3/9] Fixed a test helper function to properly report errors. --- .../dom/shared/__tests__/ReactServerRendering-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js index 5466f95f49c58..93d698bc4ff9d 100644 --- a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js @@ -33,9 +33,8 @@ function expectErrors(fn, count) { } return fn().then((result) => { - expect(console.error.calls.count()).toBe(count); if (console.error.calls.count() !== count) { - console.log(`We expected ${count} warning(s), but saw ${console.calls.count().length} warning(s).`); + console.log(`We expected ${count} warning(s), but saw ${console.error.calls.count()} warning(s).`); if (console.error.calls.count() > 0) { console.log(`We saw these warnings:`); for (var i = 0; i < console.error.calls.count(); i++) { @@ -43,6 +42,7 @@ function expectErrors(fn, count) { } } } + expect(console.error.calls.count()).toBe(count); return result; }); } From 9257660e816e6124c7dddde45bbb6244cc829809 Mon Sep 17 00:00:00 2001 From: Sasha Aickin Date: Mon, 27 Feb 2017 22:46:24 -0800 Subject: [PATCH 4/9] Un-nested the new rendering tests. Updated the fiber test passing/not passing lists. --- scripts/fiber/tests-passing-except-dev.txt | 4 ++ scripts/fiber/tests-passing.txt | 12 +++++ .../__tests__/ReactServerRendering-test.js | 47 ++++++++++--------- 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index db9abfa455fa7..028abb813a6cd 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -144,3 +144,7 @@ src/renderers/dom/shared/__tests__/ReactMountDestruction-test.js src/renderers/dom/shared/__tests__/ReactServerRendering-test.js * should have the correct mounting behavior +* should render a blank div with client render on top of bad server markup +* should render a div with inline styles with client render on top of bad server markup +* should render a self-closing tag with client render on top of bad server markup +* should render a self-closing tag as a child with client render on top of bad server markup diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index c5ff7cee6acb5..8632b799ef946 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1161,6 +1161,18 @@ src/renderers/dom/shared/__tests__/ReactServerRendering-test.js * warns with a no-op when an async replaceState is triggered * warns with a no-op when an async forceUpdate is triggered * should warn when children are mutated during render +* should render a blank div with server string render +* should render a blank div with clean client render +* should render a blank div with client render on top of good server markup +* should render a div with inline styles with server string render +* should render a div with inline styles with clean client render +* should render a div with inline styles with client render on top of good server markup +* should render a self-closing tag with server string render +* should render a self-closing tag with clean client render +* should render a self-closing tag with client render on top of good server markup +* should render a self-closing tag as a child with server string render +* should render a self-closing tag as a child with clean client render +* should render a self-closing tag as a child with client render on top of good server markup src/renderers/dom/shared/__tests__/escapeTextContentForBrowser-test.js * should escape boolean to string diff --git a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js index 93d698bc4ff9d..31aa7ec2a5fb9 100644 --- a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js @@ -42,7 +42,7 @@ function expectErrors(fn, count) { } } } - expect(console.error.calls.count()).toBe(count); + expectDev(console.error.calls.count()).toBe(count); return result; }); } @@ -391,28 +391,6 @@ describe('ReactDOMServer', () => { expect(numClicks).toEqual(2); }); - describe('basic rendering', function() { - itRenders('should render a blank div', render => - render(
).then(e => expect(e.tagName.toLowerCase()).toBe('div'))); - - itRenders('should render a div with inline styles', render => - render(
).then(e => { - expect(e.style.color).toBe('red'); - expect(e.style.width).toBe('30px'); - }) - ); - - itRenders('should render a self-closing tag', render => - render(
).then(e => expect(e.tagName.toLowerCase()).toBe('br'))); - - itRenders('should render a self-closing tag as a child', render => - render(

).then(e => { - expect(e.childNodes.length).toBe(1); - expect(e.firstChild.tagName.toLowerCase()).toBe('br'); - }) - ); - }); - it('should throw with silly args', () => { expect( ReactDOMServer.renderToString.bind( @@ -683,4 +661,27 @@ describe('ReactDOMServer', () => { ); }).toThrowError(/Cannot assign to read only property.*/); }); + + describe('basic rendering', function() { + itRenders('should render a blank div', render => + render(
).then(e => expect(e.tagName.toLowerCase()).toBe('div'))); + + itRenders('should render a div with inline styles', render => + render(
).then(e => { + expect(e.style.color).toBe('red'); + expect(e.style.width).toBe('30px'); + }) + ); + + itRenders('should render a self-closing tag', render => + render(
).then(e => expect(e.tagName.toLowerCase()).toBe('br'))); + + itRenders('should render a self-closing tag as a child', render => + render(

).then(e => { + expect(e.childNodes.length).toBe(1); + expect(e.firstChild.tagName.toLowerCase()).toBe('br'); + }) + ); + }); + }); From 08b4936b26f511c4b65ededaa8bf045c3bce8fe2 Mon Sep 17 00:00:00 2001 From: Sasha Aickin Date: Mon, 27 Feb 2017 23:03:45 -0800 Subject: [PATCH 5/9] Edited to comply with the react/jsx-space-before-closing eslint rule, which will soon be merged into master. --- .../dom/shared/__tests__/ReactServerRendering-test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js index 31aa7ec2a5fb9..143035fcbaf56 100644 --- a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js @@ -664,20 +664,20 @@ describe('ReactDOMServer', () => { describe('basic rendering', function() { itRenders('should render a blank div', render => - render(
).then(e => expect(e.tagName.toLowerCase()).toBe('div'))); + render(
).then(e => expect(e.tagName.toLowerCase()).toBe('div'))); itRenders('should render a div with inline styles', render => - render(
).then(e => { + render(
).then(e => { expect(e.style.color).toBe('red'); expect(e.style.width).toBe('30px'); }) ); itRenders('should render a self-closing tag', render => - render(
).then(e => expect(e.tagName.toLowerCase()).toBe('br'))); + render(
).then(e => expect(e.tagName.toLowerCase()).toBe('br'))); itRenders('should render a self-closing tag as a child', render => - render(

).then(e => { + render(

).then(e => { expect(e.childNodes.length).toBe(1); expect(e.firstChild.tagName.toLowerCase()).toBe('br'); }) From a91c836cc347c3b2a790b613721eaea28d0e5062 Mon Sep 17 00:00:00 2001 From: Sasha Aickin Date: Wed, 1 Mar 2017 09:40:50 -0800 Subject: [PATCH 6/9] Response to code review from @spicyj. Moved tests to separate file, reworked wording of test names, corrected use of canUseDom, and simplified tests against tagName. Thanks for the help, @spicyj! --- scripts/fiber/tests-passing-except-dev.txt | 10 +- scripts/fiber/tests-passing.txt | 26 +-- .../ReactDOMServerIntegration-test.js | 166 ++++++++++++++++++ .../__tests__/ReactServerRendering-test.js | 159 ++--------------- 4 files changed, 198 insertions(+), 163 deletions(-) create mode 100644 src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index 028abb813a6cd..bd22db6cbaa9b 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -134,6 +134,12 @@ src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js * should warn about class (ssr) * should suggest property name if available (ssr) +src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +* renders a blank div with client render on top of bad server markup +* renders a div with inline styles with client render on top of bad server markup +* renders a self-closing tag with client render on top of bad server markup +* renders a self-closing tag as a child with client render on top of bad server markup + src/renderers/dom/shared/__tests__/ReactMount-test.js * should warn if mounting into dirty rendered markup * should account for escaping on a checksum mismatch @@ -144,7 +150,3 @@ src/renderers/dom/shared/__tests__/ReactMountDestruction-test.js src/renderers/dom/shared/__tests__/ReactServerRendering-test.js * should have the correct mounting behavior -* should render a blank div with client render on top of bad server markup -* should render a div with inline styles with client render on top of bad server markup -* should render a self-closing tag with client render on top of bad server markup -* should render a self-closing tag as a child with client render on top of bad server markup diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 8632b799ef946..525163ec8962f 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1109,6 +1109,20 @@ src/renderers/dom/shared/__tests__/ReactDOMSVG-test.js * can render SVG into a non-React SVG tree * can render HTML into a foreignObject in non-React SVG tree +src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +* renders a blank div with server string render +* renders a blank div with clean client render +* renders a blank div with client render on top of good server markup +* renders a div with inline styles with server string render +* renders a div with inline styles with clean client render +* renders a div with inline styles with client render on top of good server markup +* renders a self-closing tag with server string render +* renders a self-closing tag with clean client render +* renders a self-closing tag with client render on top of good server markup +* renders a self-closing tag as a child with server string render +* renders a self-closing tag as a child with clean client render +* renders a self-closing tag as a child with client render on top of good server markup + src/renderers/dom/shared/__tests__/ReactDOMTextComponent-test.js * updates a mounted text component in place * can be toggled in and out of the markup @@ -1161,18 +1175,6 @@ src/renderers/dom/shared/__tests__/ReactServerRendering-test.js * warns with a no-op when an async replaceState is triggered * warns with a no-op when an async forceUpdate is triggered * should warn when children are mutated during render -* should render a blank div with server string render -* should render a blank div with clean client render -* should render a blank div with client render on top of good server markup -* should render a div with inline styles with server string render -* should render a div with inline styles with clean client render -* should render a div with inline styles with client render on top of good server markup -* should render a self-closing tag with server string render -* should render a self-closing tag with clean client render -* should render a self-closing tag with client render on top of good server markup -* should render a self-closing tag as a child with server string render -* should render a self-closing tag as a child with clean client render -* should render a self-closing tag as a child with client render on top of good server markup src/renderers/dom/shared/__tests__/escapeTextContentForBrowser-test.js * should escape boolean to string diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js new file mode 100644 index 0000000000000..daf84a2f219a4 --- /dev/null +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -0,0 +1,166 @@ +/** + * Copyright 2013-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +'use strict'; + +let ExecutionEnvironment; +let React; +let ReactDOM; +let ReactDOMServer; + +// Helper functions for rendering tests +// ==================================== + +// performs fn asynchronously and expects count errors logged to console.error. +// will fail the test if the count of errors logged is not equal to count. +function expectErrors(fn, count) { + if (console.error.calls && console.error.calls.reset) { + console.error.calls.reset(); + } else { + spyOn(console, 'error'); + } + + return fn().then((result) => { + if (console.error.calls.count() !== count) { + console.log(`We expected ${count} warning(s), but saw ${console.error.calls.count()} warning(s).`); + if (console.error.calls.count() > 0) { + console.log(`We saw these warnings:`); + for (var i = 0; i < console.error.calls.count(); i++) { + console.log(console.error.calls.argsFor(i)[0]); + } + } + } + expectDev(console.error.calls.count()).toBe(count); + return result; + }); +} + +// renders the reactElement into domElement, and expects a certain number of errors. +// returns a Promise that resolves when the render is complete. +function renderIntoDom(reactElement, domElement, errorCount = 0) { + return expectErrors( + () => new Promise((resolve) => { + ExecutionEnvironment.canUseDOM = true; + ReactDOM.render(reactElement, domElement, () => { + ExecutionEnvironment.canUseDOM = false; + resolve(domElement.firstChild); + }); + }), + errorCount + ); +} + +// Renders text using SSR and then stuffs it into a DOM node; returns the DOM +// element that corresponds with the reactElement. +// Does not render on client or perform client-side revival. +function serverRender(reactElement, errorCount = 0) { + return expectErrors( + () => Promise.resolve(ReactDOMServer.renderToString(reactElement)), + errorCount) + .then((markup) => { + var domElement = document.createElement('div'); + domElement.innerHTML = markup; + return domElement.firstChild; + }); +} + +const clientCleanRender = (element, errorCount = 0) => { + const div = document.createElement('div'); + return renderIntoDom(element, div, errorCount); +}; + +const clientRenderOnServerString = (element, errorCount = 0) => { + return serverRender(element, errorCount).then((markup) => { + var domElement = document.createElement('div'); + domElement.innerHTML = markup; + return renderIntoDom(element, domElement, errorCount); + }); +}; + +const clientRenderOnBadMarkup = (element, errorCount = 0) => { + var domElement = document.createElement('div'); + domElement.innerHTML = '
'; + return renderIntoDom(element, domElement, errorCount + 1); +}; + +// runs a DOM rendering test as four different tests, with four different rendering +// scenarios: +// -- render to string on server +// -- render on client without any server markup "clean client render" +// -- render on client on top of good server-generated string markup +// -- render on client on top of bad server-generated markup +// +// testFn is a test that has one arg, which is a render function. the render +// function takes in a ReactElement and an optional expected error count and +// returns a promise of a DOM Element. +// +// You should only perform tests that examine the DOM of the results of +// render; you should not depend on the interactivity of the returned DOM element, +// as that will not work in the server string scenario. +function itRenders(desc, testFn) { + it(`renders ${desc} with server string render`, + () => testFn(serverRender)); + itClientRenders(desc, testFn); +} + +// run testFn in three different rendering scenarios: +// -- render on client without any server markup "clean client render" +// -- render on client on top of good server-generated string markup +// -- render on client on top of bad server-generated markup +// +// testFn is a test that has one arg, which is a render function. the render +// function takes in a ReactElement and an optional expected error count and +// returns a promise of a DOM Element. +// +// Since all of the renders in this function are on the client, you can test interactivity, +// unlike with itRenders. +function itClientRenders(desc, testFn) { + it(`renders ${desc} with clean client render`, + () => testFn(clientCleanRender)); + it(`renders ${desc} with client render on top of good server markup`, + () => testFn(clientRenderOnServerString)); + it(`renders ${desc} with client render on top of bad server markup`, + () => testFn(clientRenderOnBadMarkup)); +} + +describe('ReactDOMServerIntegration', () => { + beforeEach(() => { + jest.resetModuleRegistry(); + React = require('React'); + ReactDOM = require('ReactDOM'); + ReactDOMServer = require('ReactDOMServer'); + + ExecutionEnvironment = require('ExecutionEnvironment'); + ExecutionEnvironment.canUseDOM = false; + }); + + describe('basic rendering', function() { + itRenders('a blank div', render => + render(
).then(e => expect(e.tagName).toBe('DIV'))); + + itRenders('a div with inline styles', render => + render(
).then(e => { + expect(e.style.color).toBe('red'); + expect(e.style.width).toBe('30px'); + }) + ); + + itRenders('a self-closing tag', render => + render(
).then(e => expect(e.tagName).toBe('BR'))); + + itRenders('a self-closing tag as a child', render => + render(

).then(e => { + expect(e.childNodes.length).toBe(1); + expect(e.firstChild.tagName).toBe('BR'); + }) + ); + }); +}); diff --git a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js index 143035fcbaf56..296bb572b576b 100644 --- a/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/shared/__tests__/ReactServerRendering-test.js @@ -23,132 +23,20 @@ var ReactTestUtils; var ID_ATTRIBUTE_NAME; var ROOT_ATTRIBUTE_NAME; -// performs fn asynchronously and expects count errors logged to console.error. -// will fail the test if the count of errors logged is not equal to count. -function expectErrors(fn, count) { - if (console.error.calls && console.error.calls.reset) { - console.error.calls.reset(); - } else { - spyOn(console, 'error'); - } - - return fn().then((result) => { - if (console.error.calls.count() !== count) { - console.log(`We expected ${count} warning(s), but saw ${console.error.calls.count()} warning(s).`); - if (console.error.calls.count() > 0) { - console.log(`We saw these warnings:`); - for (var i = 0; i < console.error.calls.count(); i++) { - console.log(console.error.calls.argsFor(i)[0]); - } - } - } - expectDev(console.error.calls.count()).toBe(count); - return result; - }); -} - -// renders the reactElement into domElement, and expects a certain number of errors. -// returns a Promise that resolves when the render is complete. -function renderIntoDom(reactElement, domElement, errorCount = 0) { - return expectErrors( - () => new Promise((resolve) => ReactDOM.render(reactElement, domElement, () => resolve(domElement.firstChild))), - errorCount - ); -} - -// Renders text using SSR and then stuffs it into a DOM node; returns the DOM -// element that corresponds with the reactElement. -// Does not render on client or perform client-side revival. -function serverRender(reactElement, errorCount = 0) { - return expectErrors( - () => Promise.resolve(ReactDOMServer.renderToString(reactElement)), - errorCount) - .then((markup) => { - var domElement = document.createElement('div'); - domElement.innerHTML = markup; - return domElement.firstChild; - }); -} - -const clientCleanRender = (element, errorCount = 0) => { - const div = document.createElement('div'); - return renderIntoDom(element, div, errorCount); -}; - -const clientRenderOnServerString = (element, errorCount = 0) => { - return serverRender(element, errorCount).then((markup) => { - resetModules(); - var domElement = document.createElement('div'); - domElement.innerHTML = markup; - return renderIntoDom(element, domElement, errorCount); - }); -}; - -const clientRenderOnBadMarkup = (element, errorCount = 0) => { - var domElement = document.createElement('div'); - domElement.innerHTML = '
'; - return renderIntoDom(element, domElement, errorCount + 1); -}; - -// runs a DOM rendering test as four different tests, with four different rendering -// scenarios: -// -- render to string on server -// -- render on client without any server markup "clean client render" -// -- render on client on top of good server-generated string markup -// -- render on client on top of bad server-generated markup -// -// testFn is a test that has one arg, which is a render function. the render -// function takes in a ReactElement and an optional expected error count and -// returns a promise of a DOM Element. -// -// You should only perform tests that examine the DOM of the results of -// render; you should not depend on the interactivity of the returned DOM element, -// as that will not work in the server string scenario. -function itRenders(desc, testFn) { - it(`${desc} with server string render`, - () => testFn(serverRender)); - itClientRenders(desc, testFn); -} - -// run testFn in three different rendering scenarios: -// -- render on client without any server markup "clean client render" -// -- render on client on top of good server-generated string markup -// -- render on client on top of bad server-generated markup -// -// testFn is a test that has one arg, which is a render function. the render -// function takes in a ReactElement and an optional expected error count and -// returns a promise of a DOM Element. -// -// Since all of the renders in this function are on the client, you can test interactivity, -// unlike with itRenders. -function itClientRenders(desc, testFn) { - it(`${desc} with clean client render`, - () => testFn(clientCleanRender)); - it(`${desc} with client render on top of good server markup`, - () => testFn(clientRenderOnServerString)); - it(`${desc} with client render on top of bad server markup`, - () => testFn(clientRenderOnBadMarkup)); -} - -function resetModules() { - jest.resetModuleRegistry(); - React = require('React'); - ReactDOM = require('ReactDOM'); - ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); - ReactMarkupChecksum = require('ReactMarkupChecksum'); - ReactTestUtils = require('ReactTestUtils'); - ReactReconcileTransaction = require('ReactReconcileTransaction'); - - ExecutionEnvironment = require('ExecutionEnvironment'); - ExecutionEnvironment.canUseDOM = false; - ReactDOMServer = require('ReactDOMServer'); - - -} - describe('ReactDOMServer', () => { beforeEach(() => { - resetModules(); + jest.resetModules(); + React = require('React'); + ReactDOM = require('ReactDOM'); + ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); + ReactMarkupChecksum = require('ReactMarkupChecksum'); + ReactTestUtils = require('ReactTestUtils'); + ReactReconcileTransaction = require('ReactReconcileTransaction'); + + ExecutionEnvironment = require('ExecutionEnvironment'); + ExecutionEnvironment.canUseDOM = false; + ReactDOMServer = require('ReactDOMServer'); + var DOMProperty = require('DOMProperty'); ID_ATTRIBUTE_NAME = DOMProperty.ID_ATTRIBUTE_NAME; ROOT_ATTRIBUTE_NAME = DOMProperty.ROOT_ATTRIBUTE_NAME; @@ -661,27 +549,4 @@ describe('ReactDOMServer', () => { ); }).toThrowError(/Cannot assign to read only property.*/); }); - - describe('basic rendering', function() { - itRenders('should render a blank div', render => - render(
).then(e => expect(e.tagName.toLowerCase()).toBe('div'))); - - itRenders('should render a div with inline styles', render => - render(
).then(e => { - expect(e.style.color).toBe('red'); - expect(e.style.width).toBe('30px'); - }) - ); - - itRenders('should render a self-closing tag', render => - render(
).then(e => expect(e.tagName.toLowerCase()).toBe('br'))); - - itRenders('should render a self-closing tag as a child', render => - render(

).then(e => { - expect(e.childNodes.length).toBe(1); - expect(e.firstChild.tagName.toLowerCase()).toBe('br'); - }) - ); - }); - }); From 1d680beee8d8c20eb336bbae9ce67460ccb9cf30 Mon Sep 17 00:00:00 2001 From: Sasha Aickin Date: Wed, 1 Mar 2017 12:37:57 -0800 Subject: [PATCH 7/9] Converted the unit tests to use async-await style. --- .babelrc | 7 +- package.json | 1 + .../ReactDOMServerIntegration-test.js | 104 +++++++++--------- 3 files changed, 60 insertions(+), 52 deletions(-) diff --git a/.babelrc b/.babelrc index cda595ba65281..b51c917883e68 100644 --- a/.babelrc +++ b/.babelrc @@ -24,5 +24,10 @@ "transform-es3-property-literals", "./scripts/babel/transform-object-assign-require", "transform-react-jsx-source" - ] + ], + "env": { + "test": { + "plugins": ["transform-async-to-generator"] + } + } } diff --git a/package.json b/package.json index b549bbb3617a6..2bde4e440618a 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,7 @@ "babel-jest": "^19.0.0", "babel-plugin-check-es2015-constants": "^6.5.0", "babel-plugin-syntax-trailing-function-commas": "^6.5.0", + "babel-plugin-transform-async-to-generator": "^6.22.0", "babel-plugin-transform-class-properties": "^6.11.5", "babel-plugin-transform-es2015-arrow-functions": "^6.5.2", "babel-plugin-transform-es2015-block-scoped-functions": "^6.5.0", diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index daf84a2f219a4..5839b823741f3 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -19,41 +19,44 @@ let ReactDOMServer; // Helper functions for rendering tests // ==================================== +// promisified version of ReactDOM.render() +function asyncReactDOMRender(reactElement, domElement) { + return new Promise(resolve => + ReactDOM.render(reactElement, domElement, resolve)); +} // performs fn asynchronously and expects count errors logged to console.error. // will fail the test if the count of errors logged is not equal to count. -function expectErrors(fn, count) { +async function expectErrors(fn, count) { if (console.error.calls && console.error.calls.reset) { console.error.calls.reset(); } else { spyOn(console, 'error'); } - return fn().then((result) => { - if (console.error.calls.count() !== count) { - console.log(`We expected ${count} warning(s), but saw ${console.error.calls.count()} warning(s).`); - if (console.error.calls.count() > 0) { - console.log(`We saw these warnings:`); - for (var i = 0; i < console.error.calls.count(); i++) { - console.log(console.error.calls.argsFor(i)[0]); - } + const result = await fn(); + if (console.error.calls.count() !== count && console.error.calls.count() !== 0) { + console.log(`We expected ${count} warning(s), but saw ${console.error.calls.count()} warning(s).`); + if (console.error.calls.count() > 0) { + console.log(`We saw these warnings:`); + for (var i = 0; i < console.error.calls.count(); i++) { + console.log(console.error.calls.argsFor(i)[0]); } } - expectDev(console.error.calls.count()).toBe(count); - return result; - }); + } + expectDev(console.error.calls.count()).toBe(count); + return result; } // renders the reactElement into domElement, and expects a certain number of errors. // returns a Promise that resolves when the render is complete. function renderIntoDom(reactElement, domElement, errorCount = 0) { return expectErrors( - () => new Promise((resolve) => { + async () => { ExecutionEnvironment.canUseDOM = true; - ReactDOM.render(reactElement, domElement, () => { - ExecutionEnvironment.canUseDOM = false; - resolve(domElement.firstChild); - }); - }), + await asyncReactDOMRender(reactElement, domElement); + ExecutionEnvironment.canUseDOM = false; + return domElement.firstChild; + }, errorCount ); } @@ -61,15 +64,13 @@ function renderIntoDom(reactElement, domElement, errorCount = 0) { // Renders text using SSR and then stuffs it into a DOM node; returns the DOM // element that corresponds with the reactElement. // Does not render on client or perform client-side revival. -function serverRender(reactElement, errorCount = 0) { - return expectErrors( +async function serverRender(reactElement, errorCount = 0) { + const markup = await expectErrors( () => Promise.resolve(ReactDOMServer.renderToString(reactElement)), - errorCount) - .then((markup) => { - var domElement = document.createElement('div'); - domElement.innerHTML = markup; - return domElement.firstChild; - }); + errorCount); + var domElement = document.createElement('div'); + domElement.innerHTML = markup; + return domElement.firstChild; } const clientCleanRender = (element, errorCount = 0) => { @@ -77,12 +78,11 @@ const clientCleanRender = (element, errorCount = 0) => { return renderIntoDom(element, div, errorCount); }; -const clientRenderOnServerString = (element, errorCount = 0) => { - return serverRender(element, errorCount).then((markup) => { - var domElement = document.createElement('div'); - domElement.innerHTML = markup; - return renderIntoDom(element, domElement, errorCount); - }); +const clientRenderOnServerString = async (element, errorCount = 0) => { + const markup = await serverRender(element, errorCount); + var domElement = document.createElement('div'); + domElement.innerHTML = markup; + return renderIntoDom(element, domElement, errorCount); }; const clientRenderOnBadMarkup = (element, errorCount = 0) => { @@ -143,24 +143,26 @@ describe('ReactDOMServerIntegration', () => { }); describe('basic rendering', function() { - itRenders('a blank div', render => - render(
).then(e => expect(e.tagName).toBe('DIV'))); - - itRenders('a div with inline styles', render => - render(
).then(e => { - expect(e.style.color).toBe('red'); - expect(e.style.width).toBe('30px'); - }) - ); - - itRenders('a self-closing tag', render => - render(
).then(e => expect(e.tagName).toBe('BR'))); - - itRenders('a self-closing tag as a child', render => - render(

).then(e => { - expect(e.childNodes.length).toBe(1); - expect(e.firstChild.tagName).toBe('BR'); - }) - ); + itRenders('a blank div', async render => { + const e = await render(
); + expect(e.tagName).toBe('DIV'); + }); + + itRenders('a div with inline styles', async render => { + const e = await render(
); + expect(e.style.color).toBe('red'); + expect(e.style.width).toBe('30px'); + }); + + itRenders('a self-closing tag', async render => { + const e = await render(
); + expect(e.tagName).toBe('BR'); + }); + + itRenders('a self-closing tag as a child', async render => { + const e = await render(

); + expect(e.childNodes.length).toBe(1); + expect(e.firstChild.tagName).toBe('BR'); + }); }); }); From d9ac8bba695d346847e2f3db416bcd4b82981d4c Mon Sep 17 00:00:00 2001 From: Sasha Aickin Date: Wed, 1 Mar 2017 14:16:20 -0800 Subject: [PATCH 8/9] Moved async-await babel transform for tests from .babelrc to preprocessor.js. --- .babelrc | 7 +------ scripts/jest/preprocessor.js | 9 ++++++++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/.babelrc b/.babelrc index b51c917883e68..cda595ba65281 100644 --- a/.babelrc +++ b/.babelrc @@ -24,10 +24,5 @@ "transform-es3-property-literals", "./scripts/babel/transform-object-assign-require", "transform-react-jsx-source" - ], - "env": { - "test": { - "plugins": ["transform-async-to-generator"] - } - } + ] } diff --git a/scripts/jest/preprocessor.js b/scripts/jest/preprocessor.js index 20f372dc57946..10b93f890116a 100644 --- a/scripts/jest/preprocessor.js +++ b/scripts/jest/preprocessor.js @@ -22,6 +22,7 @@ var pathToBabel = path.join(require.resolve('babel-core'), '..', 'package.json') var pathToModuleMap = require.resolve('fbjs/module-map'); var pathToBabelPluginDevWithCode = require.resolve('../error-codes/dev-expression-with-codes'); var pathToBabelPluginModules = require.resolve('fbjs-scripts/babel-6/rewrite-modules'); +var pathToBabelPluginAsyncToGenerator = require.resolve('babel-plugin-transform-async-to-generator'); var pathToBabelrc = path.join(__dirname, '..', '..', '.babelrc'); var pathToErrorCodes = require.resolve('../error-codes/codes.json'); @@ -54,11 +55,17 @@ module.exports = { !filePath.match(/\/node_modules\//) && !filePath.match(/\/third_party\//) ) { + // for test files, we also apply the async-await transform, but we want to + // make sure we don't accidentally apply that transform to product code. + var isTestFile = !!filePath.match(/__tests__/); return babel.transform( src, Object.assign( {filename: path.relative(process.cwd(), filePath)}, - babelOptions + babelOptions, + isTestFile ? { + plugins: [pathToBabelPluginAsyncToGenerator].concat(babelOptions.plugins), + } : {} ) ).code; } From ec6254995a8355d1b622049cbc7699f98b1c3e18 Mon Sep 17 00:00:00 2001 From: Sasha Aickin Date: Wed, 1 Mar 2017 15:08:00 -0800 Subject: [PATCH 9/9] Response to code review in PR #9089. Thanks, @spicyj! --- scripts/jest/preprocessor.js | 2 +- .../dom/shared/__tests__/ReactDOMServerIntegration-test.js | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/jest/preprocessor.js b/scripts/jest/preprocessor.js index 10b93f890116a..4a48cfe9e5554 100644 --- a/scripts/jest/preprocessor.js +++ b/scripts/jest/preprocessor.js @@ -57,7 +57,7 @@ module.exports = { ) { // for test files, we also apply the async-await transform, but we want to // make sure we don't accidentally apply that transform to product code. - var isTestFile = !!filePath.match(/__tests__/); + var isTestFile = !!filePath.match(/\/__tests__\//); return babel.transform( src, Object.assign( diff --git a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js index 5839b823741f3..afe70f87cce37 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js @@ -67,7 +67,8 @@ function renderIntoDom(reactElement, domElement, errorCount = 0) { async function serverRender(reactElement, errorCount = 0) { const markup = await expectErrors( () => Promise.resolve(ReactDOMServer.renderToString(reactElement)), - errorCount); + errorCount + ); var domElement = document.createElement('div'); domElement.innerHTML = markup; return domElement.firstChild;