From 39f1f112663159fecb19f3d2cf8862e63a09697d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 21 Nov 2016 14:54:24 -0800 Subject: [PATCH 1/2] Use the public ReactDOMServer in tests We need this because it runs the injection. --- .../__tests__/ReactServerRendering-test.js | 56 +++++++++---------- .../__tests__/ReactCompositeComponent-test.js | 6 +- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js b/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js index 7d4819f02f0e..bb0399719c32 100644 --- a/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js @@ -14,15 +14,15 @@ var ExecutionEnvironment; var React; var ReactDOM; +var ReactDOMServer; var ReactMarkupChecksum; var ReactReconcileTransaction; var ReactTestUtils; -var ReactServerRendering; var ID_ATTRIBUTE_NAME; var ROOT_ATTRIBUTE_NAME; -describe('ReactServerRendering', () => { +describe('ReactDOMServer', () => { beforeEach(() => { jest.resetModuleRegistry(); React = require('React'); @@ -33,7 +33,7 @@ describe('ReactServerRendering', () => { ExecutionEnvironment = require('ExecutionEnvironment'); ExecutionEnvironment.canUseDOM = false; - ReactServerRendering = require('ReactServerRendering'); + ReactDOMServer = require('ReactDOMServer'); var DOMProperty = require('DOMProperty'); ID_ATTRIBUTE_NAME = DOMProperty.ID_ATTRIBUTE_NAME; @@ -42,7 +42,7 @@ describe('ReactServerRendering', () => { describe('renderToString', () => { it('should generate simple markup', () => { - var response = ReactServerRendering.renderToString( + var response = ReactDOMServer.renderToString( hello world ); expect(response).toMatch(new RegExp( @@ -53,7 +53,7 @@ describe('ReactServerRendering', () => { }); it('should generate simple markup for self-closing tags', () => { - var response = ReactServerRendering.renderToString( + var response = ReactDOMServer.renderToString( ); expect(response).toMatch(new RegExp( @@ -64,7 +64,7 @@ describe('ReactServerRendering', () => { }); it('should generate simple markup for attribute with `>` symbol', () => { - var response = ReactServerRendering.renderToString( + var response = ReactDOMServer.renderToString( ); expect(response).toMatch(new RegExp( @@ -81,7 +81,7 @@ describe('ReactServerRendering', () => { } } - var response = ReactServerRendering.renderToString(); + var response = ReactDOMServer.renderToString(); expect(response).toBe(''); }); @@ -100,7 +100,7 @@ describe('ReactServerRendering', () => { } } - var response = ReactServerRendering.renderToString( + var response = ReactDOMServer.renderToString( ); expect(response).toMatch(new RegExp( @@ -160,7 +160,7 @@ describe('ReactServerRendering', () => { } } - var response = ReactServerRendering.renderToString( + var response = ReactDOMServer.renderToString( ); @@ -233,7 +233,7 @@ describe('ReactServerRendering', () => { expect(element.innerHTML).toEqual(''); ExecutionEnvironment.canUseDOM = false; - lastMarkup = ReactServerRendering.renderToString( + lastMarkup = ReactDOMServer.renderToString( ); ExecutionEnvironment.canUseDOM = true; @@ -269,8 +269,8 @@ describe('ReactServerRendering', () => { it('should throw with silly args', () => { expect( - ReactServerRendering.renderToString.bind( - ReactServerRendering, + ReactDOMServer.renderToString.bind( + ReactDOMServer, 'not a component' ) ).toThrowError( @@ -293,7 +293,7 @@ describe('ReactServerRendering', () => { } } - var response = ReactServerRendering.renderToStaticMarkup( + var response = ReactDOMServer.renderToStaticMarkup( ); @@ -307,7 +307,7 @@ describe('ReactServerRendering', () => { } } - var response = ReactServerRendering.renderToStaticMarkup( + var response = ReactDOMServer.renderToStaticMarkup( ); @@ -359,7 +359,7 @@ describe('ReactServerRendering', () => { } } - var response = ReactServerRendering.renderToStaticMarkup( + var response = ReactDOMServer.renderToStaticMarkup( ); @@ -378,8 +378,8 @@ describe('ReactServerRendering', () => { it('should throw with silly args', () => { expect( - ReactServerRendering.renderToStaticMarkup.bind( - ReactServerRendering, + ReactDOMServer.renderToStaticMarkup.bind( + ReactDOMServer, 'not a component' ) ).toThrowError( @@ -402,7 +402,7 @@ describe('ReactServerRendering', () => { // We shouldn't ever be calling this on the server throw new Error('Browser reconcile transaction should not be used'); }; - var markup = ReactServerRendering.renderToString( + var markup = ReactDOMServer.renderToString( ); expect(markup.indexOf('hello, world') >= 0).toBe(true); @@ -411,7 +411,7 @@ describe('ReactServerRendering', () => { it('renders components with different batching strategies', () => { class StaticComponent extends React.Component { render() { - const staticContent = ReactServerRendering.renderToStaticMarkup( + const staticContent = ReactDOMServer.renderToStaticMarkup(
@@ -431,8 +431,8 @@ describe('ReactServerRendering', () => { } expect( - ReactServerRendering.renderToString.bind( - ReactServerRendering, + ReactDOMServer.renderToString.bind( + ReactDOMServer,
@@ -456,7 +456,7 @@ describe('ReactServerRendering', () => { } spyOn(console, 'error'); - ReactServerRendering.renderToString(); + ReactDOMServer.renderToString(); jest.runOnlyPendingTimers(); expectDev(console.error.calls.count()).toBe(1); expectDev(console.error.calls.mostRecent().args[0]).toBe( @@ -464,7 +464,7 @@ describe('ReactServerRendering', () => { ' This usually means you called setState() outside componentWillMount() on the server.' + ' This is a no-op. Please check the code for the Foo component.' ); - var markup = ReactServerRendering.renderToStaticMarkup(); + var markup = ReactDOMServer.renderToStaticMarkup(); expect(markup).toBe('
hello
'); }); @@ -482,7 +482,7 @@ describe('ReactServerRendering', () => { }); spyOn(console, 'error'); - ReactServerRendering.renderToString(); + ReactDOMServer.renderToString(); jest.runOnlyPendingTimers(); expectDev(console.error.calls.count()).toBe(1); expectDev(console.error.calls.mostRecent().args[0]).toBe( @@ -490,7 +490,7 @@ describe('ReactServerRendering', () => { 'This usually means you called replaceState() outside componentWillMount() on the server. ' + 'This is a no-op. Please check the code for the Bar component.' ); - var markup = ReactServerRendering.renderToStaticMarkup(); + var markup = ReactDOMServer.renderToStaticMarkup(); expect(markup).toBe('
hello
'); }); @@ -509,7 +509,7 @@ describe('ReactServerRendering', () => { } spyOn(console, 'error'); - ReactServerRendering.renderToString(); + ReactDOMServer.renderToString(); jest.runOnlyPendingTimers(); expectDev(console.error.calls.count()).toBe(1); expectDev(console.error.calls.mostRecent().args[0]).toBe( @@ -517,7 +517,7 @@ describe('ReactServerRendering', () => { 'This usually means you called forceUpdate() outside componentWillMount() on the server. ' + 'This is a no-op. Please check the code for the Baz component.' ); - var markup = ReactServerRendering.renderToStaticMarkup(); + var markup = ReactDOMServer.renderToStaticMarkup(); expect(markup).toBe('
'); }); @@ -528,7 +528,7 @@ describe('ReactServerRendering', () => { return
{props.children}
; } expect(() => { - ReactServerRendering.renderToStaticMarkup( + ReactDOMServer.renderToStaticMarkup( diff --git a/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js b/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js index b8bb48039fd2..4fcc514b3b1f 100644 --- a/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js +++ b/src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js @@ -16,9 +16,9 @@ var MorphingComponent; var React; var ReactDOM; var ReactDOMFeatureFlags; +var ReactDOMServer; var ReactCurrentOwner; var ReactPropTypes; -var ReactServerRendering; var ReactTestUtils; describe('ReactCompositeComponent', () => { @@ -28,10 +28,10 @@ describe('ReactCompositeComponent', () => { React = require('React'); ReactDOM = require('ReactDOM'); ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); + ReactDOMServer = require('ReactDOMServer'); ReactCurrentOwner = require('ReactCurrentOwner'); ReactPropTypes = require('ReactPropTypes'); ReactTestUtils = require('ReactTestUtils'); - ReactServerRendering = require('ReactServerRendering'); MorphingComponent = class extends React.Component { state = {activated: false}; @@ -108,7 +108,7 @@ describe('ReactCompositeComponent', () => { } } - var markup = ReactServerRendering.renderToString(); + var markup = ReactDOMServer.renderToString(); var container = document.createElement('div'); container.innerHTML = markup; From 72f13887f85f6718e08409d9e1da6a738319a3f1 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 21 Nov 2016 15:17:10 -0800 Subject: [PATCH 2/2] Let Fiber skip react data attributes and comments in SSR tests This markup is testing implementation details rather than behavior. --- scripts/fiber/tests-failing.txt | 19 ------------------- scripts/fiber/tests-passing-except-dev.txt | 3 +++ scripts/fiber/tests-passing.txt | 16 ++++++++++++++++ .../__tests__/ReactServerRendering-test.js | 14 +++++++++++++- 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/scripts/fiber/tests-failing.txt b/scripts/fiber/tests-failing.txt index 23726bdeccb9..eaddb72087a0 100644 --- a/scripts/fiber/tests-failing.txt +++ b/scripts/fiber/tests-failing.txt @@ -108,24 +108,6 @@ src/renderers/dom/stack/client/__tests__/ReactRenderDocument-test.js * should throw on full document render w/ no markup * supports findDOMNode on full-page components -src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js -* should generate simple markup -* should generate simple markup for self-closing tags -* should generate simple markup for attribute with `>` symbol -* should generate comment markup for component returns null -* should render composite components -* should only execute certain lifecycle methods -* should have the correct mounting behavior -* should not put checksum and React ID on components -* should not put checksum and React ID on text components -* should only execute certain lifecycle methods -* allows setState in componentWillMount without using DOM -* renders components with different batching strategies -* warns with a no-op when an async setState is triggered -* 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 - src/renderers/shared/__tests__/ReactPerf-test.js * should count no-op update as waste * should count no-op update in child as waste @@ -150,7 +132,6 @@ src/renderers/shared/stack/reconciler/__tests__/ReactComponentLifeCycle-test.js * should carry through each of the phases of setup src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js -* should not thrash a server rendered layout with client side one * should warn about `forceUpdate` on unmounted components * should warn about `setState` on unmounted components * should warn about `setState` in render diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt index f6b4477bb082..247e5287cb58 100644 --- a/scripts/fiber/tests-passing-except-dev.txt +++ b/scripts/fiber/tests-passing-except-dev.txt @@ -65,6 +65,9 @@ src/renderers/dom/stack/client/__tests__/ReactMountDestruction-test.js * should warn when unmounting a non-container root node * should warn when unmounting a non-container, non-root node +src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js +* should have the correct mounting behavior + src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js * uses displayName or Unknown for classic components * uses displayName, name, or ReactComponent for modern components diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 9a54b3c411b8..3c3452f31073 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -919,8 +919,23 @@ src/renderers/dom/stack/client/__tests__/findDOMNode-test.js * findDOMNode should not throw an error when called within a component that is not mounted src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js +* should generate simple markup +* should generate simple markup for self-closing tags +* should generate simple markup for attribute with `>` symbol +* should generate comment markup for component returns null +* should render composite components +* should only execute certain lifecycle methods * should throw with silly args +* should not put checksum and React ID on components +* should not put checksum and React ID on text components +* should only execute certain lifecycle methods * should throw with silly args +* allows setState in componentWillMount without using DOM +* renders components with different batching strategies +* warns with a no-op when an async setState is triggered +* 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 src/renderers/native/__tests__/ReactNativeAttributePayload-test.js * should work with simple example @@ -1231,6 +1246,7 @@ src/renderers/shared/stack/reconciler/__tests__/ReactComponentLifeCycle-test.js src/renderers/shared/stack/reconciler/__tests__/ReactCompositeComponent-test.js * should support module pattern components * should support rendering to different child types over time +* should not thrash a server rendered layout with client side one * should react to state changes from callbacks * should rewire refs when rendering to different child types * should not cache old DOM nodes when switching constructors diff --git a/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js b/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js index bb0399719c32..3416fb3fbcb5 100644 --- a/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js +++ b/src/renderers/dom/stack/server/__tests__/ReactServerRendering-test.js @@ -14,6 +14,7 @@ var ExecutionEnvironment; var React; var ReactDOM; +var ReactDOMFeatureFlags; var ReactDOMServer; var ReactMarkupChecksum; var ReactReconcileTransaction; @@ -27,6 +28,7 @@ describe('ReactDOMServer', () => { jest.resetModuleRegistry(); React = require('React'); ReactDOM = require('ReactDOM'); + ReactDOMFeatureFlags = require('ReactDOMFeatureFlags'); ReactMarkupChecksum = require('ReactMarkupChecksum'); ReactTestUtils = require('ReactTestUtils'); ReactReconcileTransaction = require('ReactReconcileTransaction'); @@ -241,7 +243,17 @@ describe('ReactDOMServer', () => { var instance = ReactDOM.render(, element); expect(mountCount).toEqual(3); - expect(element.innerHTML).toBe(lastMarkup); + + var expectedMarkup = lastMarkup; + if (ReactDOMFeatureFlags.useFiber) { + var reactMetaData = /\s+data-react[a-z-]+=\"[^\"]*\"/g; + var reactComments = //g; + expectedMarkup = + expectedMarkup + .replace(reactMetaData, '') + .replace(reactComments, ''); + } + expect(element.innerHTML).toBe(expectedMarkup); // Ensure the events system works after mount into server markup expect(numClicks).toEqual(0);