diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5b2021a00888b..95392e2f5f17c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -57,6 +57,7 @@ Facebook has a [bounty program](https://www.facebook.com/whitehat/) for the safe * `"use strict";` * 80 character line length * "Attractive" +* Do not use the optional parameters of `setTimeout` and `setInterval` ## License diff --git a/src/core/__tests__/ReactRenderDocument-test.js b/src/core/__tests__/ReactRenderDocument-test.js index 04e3d30525fba..489312ee46683 100644 --- a/src/core/__tests__/ReactRenderDocument-test.js +++ b/src/core/__tests__/ReactRenderDocument-test.js @@ -64,14 +64,13 @@ describe('rendering React components at document', function() { } }); - React.renderComponentToString(, function(markup) { - testDocument = getTestDocument(markup); - var component = React.renderComponent(, testDocument); - expect(testDocument.body.innerHTML).toBe('Hello world'); + var markup = React.renderComponentToString(); + testDocument = getTestDocument(markup); + var component = React.renderComponent(, testDocument); + expect(testDocument.body.innerHTML).toBe('Hello world'); - var componentID = ReactMount.getReactRootID(testDocument); - expect(componentID).toBe(component._rootNodeID); - }); + var componentID = ReactMount.getReactRootID(testDocument); + expect(componentID).toBe(component._rootNodeID); }); it('should not be able to unmount component from document node', function() { @@ -92,17 +91,16 @@ describe('rendering React components at document', function() { } }); - React.renderComponentToString(, function(markup) { - testDocument = getTestDocument(markup); - React.renderComponent(, testDocument); - expect(testDocument.body.innerHTML).toBe('Hello world'); + var markup = React.renderComponentToString(); + testDocument = getTestDocument(markup); + React.renderComponent(, testDocument); + expect(testDocument.body.innerHTML).toBe('Hello world'); - expect(function() { - React.unmountComponentAtNode(testDocument); - }).toThrow(UNMOUNT_INVARIANT_MESSAGE); + expect(function() { + React.unmountComponentAtNode(testDocument); + }).toThrow(UNMOUNT_INVARIANT_MESSAGE); - expect(testDocument.body.innerHTML).toBe('Hello world'); - }); + expect(testDocument.body.innerHTML).toBe('Hello world'); }); it('should not be able to switch root constructors', function() { @@ -138,20 +136,19 @@ describe('rendering React components at document', function() { } }); - React.renderComponentToString(, function(markup) { - testDocument = getTestDocument(markup); + var markup = React.renderComponentToString(); + testDocument = getTestDocument(markup); - React.renderComponent(, testDocument); + React.renderComponent(, testDocument); - expect(testDocument.body.innerHTML).toBe('Hello world'); + expect(testDocument.body.innerHTML).toBe('Hello world'); - // Reactive update - expect(function() { - React.renderComponent(, testDocument); - }).toThrow(UNMOUNT_INVARIANT_MESSAGE); + // Reactive update + expect(function() { + React.renderComponent(, testDocument); + }).toThrow(UNMOUNT_INVARIANT_MESSAGE); - expect(testDocument.body.innerHTML).toBe('Hello world'); - }); + expect(testDocument.body.innerHTML).toBe('Hello world'); }); it('should be able to mount into document', function() { @@ -172,16 +169,14 @@ describe('rendering React components at document', function() { } }); - React.renderComponentToString( - , - function(markup) { - testDocument = getTestDocument(markup); + var markup = React.renderComponentToString( + + ); + testDocument = getTestDocument(markup); - React.renderComponent(, testDocument); + React.renderComponent(, testDocument); - expect(testDocument.body.innerHTML).toBe('Hello world'); - } - ); + expect(testDocument.body.innerHTML).toBe('Hello world'); }); it('should give helpful errors on state desync', function() { @@ -202,26 +197,24 @@ describe('rendering React components at document', function() { } }); - React.renderComponentToString( - , - function(markup) { - testDocument = getTestDocument(markup); - - expect(function() { - // Notice the text is different! - React.renderComponent(, testDocument); - }).toThrow( - 'Invariant Violation: ' + - 'You\'re trying to render a component to the document using ' + - 'server rendering but the checksum was invalid. This usually ' + - 'means you rendered a different component type or props on ' + - 'the client from the one on the server, or your render() methods ' + - 'are impure. React cannot handle this case due to cross-browser ' + - 'quirks by rendering at the document root. You should look for ' + - 'environment dependent code in your components and ensure ' + - 'the props are the same client and server side.' - ); - } + var markup = React.renderComponentToString( + + ); + testDocument = getTestDocument(markup); + + expect(function() { + // Notice the text is different! + React.renderComponent(, testDocument); + }).toThrow( + 'Invariant Violation: ' + + 'You\'re trying to render a component to the document using ' + + 'server rendering but the checksum was invalid. This usually ' + + 'means you rendered a different component type or props on ' + + 'the client from the one on the server, or your render() methods ' + + 'are impure. React cannot handle this case due to cross-browser ' + + 'quirks by rendering at the document root. You should look for ' + + 'environment dependent code in your components and ensure ' + + 'the props are the same client and server side.' ); }); diff --git a/src/core/__tests__/ReactTextComponent-test.js b/src/core/__tests__/ReactTextComponent-test.js index 911adf4f49f64..451205564116c 100644 --- a/src/core/__tests__/ReactTextComponent-test.js +++ b/src/core/__tests__/ReactTextComponent-test.js @@ -30,9 +30,8 @@ describe('ReactTextComponent', function() { var ThisThingShouldBeEscaped = '">>> LULZ <<<"'; var ThisThingWasBeEscaped = '">>> LULZ <<<"'; var thing = React.DOM.div(null, React.DOM.span({key:ThisThingShouldBeEscaped}, ["LULZ"])); - React.renderComponentToString(thing, function(html){ - expect(html).not.toContain(ThisThingShouldBeEscaped); - expect(html).toContain(ThisThingWasBeEscaped); - }); + var html = React.renderComponentToString(thing); + expect(html).not.toContain(ThisThingShouldBeEscaped); + expect(html).toContain(ThisThingWasBeEscaped); }) }); diff --git a/src/environment/ReactServerRendering.js b/src/environment/ReactServerRendering.js index e5046edbeb783..9c67e880ea754 100644 --- a/src/environment/ReactServerRendering.js +++ b/src/environment/ReactServerRendering.js @@ -27,30 +27,27 @@ var invariant = require('invariant'); /** * @param {ReactComponent} component - * @param {function} callback + * @return {String} the markup */ -function renderComponentToString(component, callback) { - // We use a callback API to keep the API async in case in the future we ever - // need it, but in reality this is a synchronous operation. - +function renderComponentToString(component) { invariant( ReactComponent.isValidComponent(component), 'renderComponentToString(): You must pass a valid ReactComponent.' ); invariant( - typeof callback === 'function', - 'renderComponentToString(): You must pass a function as a callback.' + !(arguments.length === 2 && typeof arguments[1] === 'function'), + 'renderComponentToString(): This function became synchronous and now ' + + 'returns the generated markup. Please remove the second parameter.' ); var id = ReactInstanceHandles.createReactRootID(); var transaction = ReactReconcileTransaction.getPooled(); transaction.reinitializeTransaction(); try { - transaction.perform(function() { + return transaction.perform(function() { var markup = component.mountComponent(id, transaction, 0); - markup = ReactMarkupChecksum.addChecksumToMarkup(markup); - callback(markup); + return ReactMarkupChecksum.addChecksumToMarkup(markup); }, null); } finally { ReactReconcileTransaction.release(transaction); diff --git a/src/environment/__tests__/ReactServerRendering-test.js b/src/environment/__tests__/ReactServerRendering-test.js index 8ed7b67daed3b..e4253ff41d2be 100644 --- a/src/environment/__tests__/ReactServerRendering-test.js +++ b/src/environment/__tests__/ReactServerRendering-test.js @@ -56,12 +56,8 @@ describe('ReactServerRendering', function() { }); it('should generate simple markup', function() { - var response; - ReactServerRendering.renderComponentToString( - hello world, - function(response_) { - response = response_; - } + var response = ReactServerRendering.renderComponentToString( + hello world ); expect(response).toMatch( 'hello world, - function() { - expect(EventPluginHub.__getListenerBank()).toEqual({}); - } + var response = ReactServerRendering.renderComponentToString( + hello world ); + expect(EventPluginHub.__getListenerBank()).toEqual({}); }); it('should render composite components', function() { @@ -92,12 +86,8 @@ describe('ReactServerRendering', function() { return My name is {this.props.name}; } }); - var response; - ReactServerRendering.renderComponentToString( - , - function(response_) { - response = response_; - } + var response = ReactServerRendering.renderComponentToString( + ); expect(response).toMatch( '
, - function (_response) { - response = _response; - } + var response = ReactServerRendering.renderComponentToString( + ); expect(response).toMatch( @@ -212,14 +197,10 @@ describe('ReactServerRendering', function() { expect(element.innerHTML).toEqual(''); ExecutionEnvironment.canUseDOM = false; - ReactServerRendering.renderComponentToString( - , - function(markup) { - lastMarkup = markup; - } + lastMarkup = ReactServerRendering.renderComponentToString( + ); ExecutionEnvironment.canUseDOM = true; - element.innerHTML = lastMarkup + ' __sentinel__'; React.renderComponent(, element); @@ -250,23 +231,25 @@ describe('ReactServerRendering', function() { expect( ReactServerRendering.renderComponentToString.bind( ReactServerRendering, - 'not a component', - function() {} + 'not a component' ) ).toThrow( 'Invariant Violation: renderComponentToString(): You must pass ' + 'a valid ReactComponent.' ); + }); + it('should provide guidance for breaking API changes', function() { expect( ReactServerRendering.renderComponentToString.bind( ReactServerRendering, - React.DOM.div(), - 'not a function' +
, + function(){} ) ).toThrow( - 'Invariant Violation: renderComponentToString(): You must pass ' + - 'a function as a callback.' + 'Invariant Violation: renderComponentToString(): This function became ' + + 'synchronous and now returns the generated markup. Please remove the ' + + 'second parameter.' ); }); });