mirror of
https://github.com/facebook/react.git
synced 2025-11-01 09:12:30 +00:00
Trigger a proper no-op warning for async state changes on server (#7127)
This commit fixes #5473: ReactDOMServer.renderToString: presence of onClick
handler causes errors on async update
This commit performs the following changes:
- Adds a getUpdateQueue method to ReactServerRenderingTransaction,
ReactReconcileTransaction, ReactNativeReconcileTransaction and
ReactTestReconcileTransaction
- Make the ReactCompositeComponent call this getUpdateQueue instead of using
ReactUpdateQueue that was unwanted at certain moments on server
- On ReactServerRenderingTransaction, dispatch ReactUpdateQueue's methods
while rendering and warning methods afterwards. This is done through the new
ReactServerUpdateQueue class
- Added a series of tests that mimics the case presented in #5473 with setState,
forceUpdate and replaceState
- Add flow typechecking on concerned files
(cherry picked from commit dbdddf1c82)
This commit is contained in:
committed by
Paul O’Shannessy
parent
67cbe6d471
commit
173d065a9e
@@ -13,8 +13,9 @@
|
||||
|
||||
var warning = require('warning');
|
||||
|
||||
function warnTDZ(publicInstance, callerName) {
|
||||
function warnNoop(publicInstance, callerName) {
|
||||
if (__DEV__) {
|
||||
var constructor = publicInstance.constructor;
|
||||
warning(
|
||||
false,
|
||||
'%s(...): Can only update a mounted or mounting component. ' +
|
||||
@@ -22,7 +23,7 @@ function warnTDZ(publicInstance, callerName) {
|
||||
'This is a no-op. Please check the code for the %s component.',
|
||||
callerName,
|
||||
callerName,
|
||||
publicInstance.constructor && publicInstance.constructor.displayName || ''
|
||||
constructor && (constructor.displayName || constructor.name) || 'ReactClass'
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -67,7 +68,7 @@ var ReactNoopUpdateQueue = {
|
||||
* @internal
|
||||
*/
|
||||
enqueueForceUpdate: function(publicInstance) {
|
||||
warnTDZ(publicInstance, 'forceUpdate');
|
||||
warnNoop(publicInstance, 'forceUpdate');
|
||||
},
|
||||
|
||||
/**
|
||||
@@ -82,7 +83,7 @@ var ReactNoopUpdateQueue = {
|
||||
* @internal
|
||||
*/
|
||||
enqueueReplaceState: function(publicInstance, completeState) {
|
||||
warnTDZ(publicInstance, 'replaceState');
|
||||
warnNoop(publicInstance, 'replaceState');
|
||||
},
|
||||
|
||||
/**
|
||||
@@ -96,7 +97,7 @@ var ReactNoopUpdateQueue = {
|
||||
* @internal
|
||||
*/
|
||||
enqueueSetState: function(publicInstance, partialState) {
|
||||
warnTDZ(publicInstance, 'setState');
|
||||
warnNoop(publicInstance, 'setState');
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ var PooledClass = require('PooledClass');
|
||||
var ReactBrowserEventEmitter = require('ReactBrowserEventEmitter');
|
||||
var ReactInputSelection = require('ReactInputSelection');
|
||||
var Transaction = require('Transaction');
|
||||
var ReactUpdateQueue = require('ReactUpdateQueue');
|
||||
|
||||
|
||||
/**
|
||||
@@ -104,7 +105,7 @@ var TRANSACTION_WRAPPERS = [
|
||||
*
|
||||
* @class ReactReconcileTransaction
|
||||
*/
|
||||
function ReactReconcileTransaction(useCreateElement) {
|
||||
function ReactReconcileTransaction(useCreateElement: boolean) {
|
||||
this.reinitializeTransaction();
|
||||
// Only server-side rendering really needs this option (see
|
||||
// `ReactServerRendering`), but server-side uses
|
||||
@@ -135,6 +136,13 @@ var Mixin = {
|
||||
return this.reactMountReady;
|
||||
},
|
||||
|
||||
/**
|
||||
* @return {object} The queue to collect React async events.
|
||||
*/
|
||||
getUpdateQueue: function() {
|
||||
return ReactUpdateQueue;
|
||||
},
|
||||
|
||||
/**
|
||||
* Save current transaction state -- if the return value from this method is
|
||||
* passed to `rollback`, the transaction will be reset to that state.
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
|
||||
var PooledClass = require('PooledClass');
|
||||
var Transaction = require('Transaction');
|
||||
var ReactServerUpdateQueue = require('ReactServerUpdateQueue');
|
||||
|
||||
|
||||
/**
|
||||
@@ -34,6 +35,7 @@ function ReactServerRenderingTransaction(renderToStaticMarkup) {
|
||||
this.reinitializeTransaction();
|
||||
this.renderToStaticMarkup = renderToStaticMarkup;
|
||||
this.useCreateElement = false;
|
||||
this.updateQueue = new ReactServerUpdateQueue(this);
|
||||
}
|
||||
|
||||
var Mixin = {
|
||||
@@ -54,6 +56,13 @@ var Mixin = {
|
||||
return noopCallbackQueue;
|
||||
},
|
||||
|
||||
/**
|
||||
* @return {object} The queue to collect React async events.
|
||||
*/
|
||||
getUpdateQueue: function() {
|
||||
return this.updateQueue;
|
||||
},
|
||||
|
||||
/**
|
||||
* `PooledClass` looks for this, and will invoke this before allowing this
|
||||
* instance to be reused.
|
||||
|
||||
@@ -0,0 +1,132 @@
|
||||
/**
|
||||
* Copyright 2015-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.
|
||||
*
|
||||
* @providesModule ReactServerUpdateQueue
|
||||
* @flow
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
var ReactUpdateQueue = require('ReactUpdateQueue');
|
||||
var Transaction = require('Transaction');
|
||||
var warning = require('warning');
|
||||
|
||||
function warnNoop(publicInstance: ReactComponent<any, any, any>, callerName: string) {
|
||||
if (__DEV__) {
|
||||
var constructor = publicInstance.constructor;
|
||||
warning(
|
||||
false,
|
||||
'%s(...): Can only update a mounting component. ' +
|
||||
'This usually means you called %s() outside componentWillMount() on the server. ' +
|
||||
'This is a no-op. Please check the code for the %s component.',
|
||||
callerName,
|
||||
callerName,
|
||||
constructor && (constructor.displayName || constructor.name) || 'ReactClass'
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* This is the update queue used for server rendering.
|
||||
* It delegates to ReactUpdateQueue while server rendering is in progress and
|
||||
* switches to ReactNoopUpdateQueue after the transaction has completed.
|
||||
* @class ReactServerUpdateQueue
|
||||
* @param {Transaction} transaction
|
||||
*/
|
||||
class ReactServerUpdateQueue {
|
||||
/* :: transaction: Transaction; */
|
||||
|
||||
constructor(transaction: Transaction) {
|
||||
this.transaction = transaction;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether or not this composite component is mounted.
|
||||
* @param {ReactClass} publicInstance The instance we want to test.
|
||||
* @return {boolean} True if mounted, false otherwise.
|
||||
* @protected
|
||||
* @final
|
||||
*/
|
||||
isMounted(publicInstance: ReactComponent<any, any, any>): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Enqueue a callback that will be executed after all the pending updates
|
||||
* have processed.
|
||||
*
|
||||
* @param {ReactClass} publicInstance The instance to use as `this` context.
|
||||
* @param {?function} callback Called after state is updated.
|
||||
* @internal
|
||||
*/
|
||||
enqueueCallback(publicInstance: ReactComponent<any, any, any>, callback?: Function, callerName?: string) {
|
||||
if (this.transaction.isInTransaction()) {
|
||||
ReactUpdateQueue.enqueueCallback(publicInstance, callback, callerName);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Forces an update. This should only be invoked when it is known with
|
||||
* certainty that we are **not** in a DOM transaction.
|
||||
*
|
||||
* You may want to call this when you know that some deeper aspect of the
|
||||
* component's state has changed but `setState` was not called.
|
||||
*
|
||||
* This will not invoke `shouldComponentUpdate`, but it will invoke
|
||||
* `componentWillUpdate` and `componentDidUpdate`.
|
||||
*
|
||||
* @param {ReactClass} publicInstance The instance that should rerender.
|
||||
* @internal
|
||||
*/
|
||||
enqueueForceUpdate(publicInstance: ReactComponent<any, any, any>) {
|
||||
if (this.transaction.isInTransaction()) {
|
||||
ReactUpdateQueue.enqueueForceUpdate(publicInstance);
|
||||
} else {
|
||||
warnNoop(publicInstance, 'forceUpdate');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Replaces all of the state. Always use this or `setState` to mutate state.
|
||||
* You should treat `this.state` as immutable.
|
||||
*
|
||||
* There is no guarantee that `this.state` will be immediately updated, so
|
||||
* accessing `this.state` after calling this method may return the old value.
|
||||
*
|
||||
* @param {ReactClass} publicInstance The instance that should rerender.
|
||||
* @param {object|function} completeState Next state.
|
||||
* @internal
|
||||
*/
|
||||
enqueueReplaceState(publicInstance: ReactComponent<any, any, any>, completeState: Object|Function) {
|
||||
if (this.transaction.isInTransaction()) {
|
||||
ReactUpdateQueue.enqueueReplaceState(publicInstance, completeState);
|
||||
} else {
|
||||
warnNoop(publicInstance, 'replaceState');
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets a subset of the state. This only exists because _pendingState is
|
||||
* internal. This provides a merging strategy that is not available to deep
|
||||
* properties which is confusing. TODO: Expose pendingState or don't use it
|
||||
* during the merge.
|
||||
*
|
||||
* @param {ReactClass} publicInstance The instance that should rerender.
|
||||
* @param {object|function} partialState Next partial state to be merged with state.
|
||||
* @internal
|
||||
*/
|
||||
enqueueSetState(publicInstance: ReactComponent<any, any, any>, partialState: Object|Function) {
|
||||
if (this.transaction.isInTransaction()) {
|
||||
ReactUpdateQueue.enqueueSetState(publicInstance, partialState);
|
||||
} else {
|
||||
warnNoop(publicInstance, 'setState');
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = ReactServerUpdateQueue;
|
||||
@@ -400,4 +400,82 @@ describe('ReactServerRendering', function() {
|
||||
expect(markup.indexOf('hello, world') >= 0).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it('warns with a no-op when an async setState is triggered', function() {
|
||||
class Foo extends React.Component {
|
||||
componentWillMount() {
|
||||
this.setState({text: 'hello'});
|
||||
setTimeout(() => {
|
||||
this.setState({text: 'error'});
|
||||
});
|
||||
}
|
||||
render() {
|
||||
return <div onClick={() => {}}>{this.state.text}</div>;
|
||||
}
|
||||
}
|
||||
|
||||
spyOn(console, 'error');
|
||||
ReactServerRendering.renderToString(<Foo />);
|
||||
jest.runOnlyPendingTimers();
|
||||
expect(console.error.calls.count()).toBe(1);
|
||||
expect(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. Please check the code for the Foo component.'
|
||||
);
|
||||
var markup = ReactServerRendering.renderToStaticMarkup(<Foo />);
|
||||
expect(markup).toBe('<div>hello</div>');
|
||||
});
|
||||
|
||||
it('warns with a no-op when an async replaceState is triggered', function() {
|
||||
var Bar = React.createClass({
|
||||
componentWillMount: function() {
|
||||
this.replaceState({text: 'hello'});
|
||||
setTimeout(() => {
|
||||
this.replaceState({text: 'error'});
|
||||
});
|
||||
},
|
||||
render: function() {
|
||||
return <div onClick={() => {}}>{this.state.text}</div>;
|
||||
},
|
||||
});
|
||||
|
||||
spyOn(console, 'error');
|
||||
ReactServerRendering.renderToString(<Bar />);
|
||||
jest.runOnlyPendingTimers();
|
||||
expect(console.error.calls.count()).toBe(1);
|
||||
expect(console.error.calls.mostRecent().args[0]).toBe(
|
||||
'Warning: replaceState(...): Can only update a mounting component. ' +
|
||||
'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(<Bar />);
|
||||
expect(markup).toBe('<div>hello</div>');
|
||||
});
|
||||
|
||||
it('warns with a no-op when an async forceUpdate is triggered', function() {
|
||||
var Baz = React.createClass({
|
||||
componentWillMount: function() {
|
||||
this.forceUpdate();
|
||||
setTimeout(() => {
|
||||
this.forceUpdate();
|
||||
});
|
||||
},
|
||||
render: function() {
|
||||
return <div onClick={() => {}}></div>;
|
||||
},
|
||||
});
|
||||
|
||||
spyOn(console, 'error');
|
||||
ReactServerRendering.renderToString(<Baz />);
|
||||
jest.runOnlyPendingTimers();
|
||||
expect(console.error.calls.count()).toBe(1);
|
||||
expect(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. Please check the code for the Baz component.'
|
||||
);
|
||||
var markup = ReactServerRendering.renderToStaticMarkup(<Baz />);
|
||||
expect(markup).toBe('<div></div>');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
var CallbackQueue = require('CallbackQueue');
|
||||
var PooledClass = require('PooledClass');
|
||||
var Transaction = require('Transaction');
|
||||
var ReactUpdateQueue = require('ReactUpdateQueue');
|
||||
|
||||
/**
|
||||
* Provides a `CallbackQueue` queue for collecting `onDOMReady` callbacks during
|
||||
@@ -81,6 +82,13 @@ var Mixin = {
|
||||
return this.reactMountReady;
|
||||
},
|
||||
|
||||
/**
|
||||
* @return {object} The queue to collect React async events.
|
||||
*/
|
||||
getUpdateQueue: function() {
|
||||
return ReactUpdateQueue;
|
||||
},
|
||||
|
||||
/**
|
||||
* `PooledClass` looks for this, and will invoke this before allowing this
|
||||
* instance to be reused.
|
||||
|
||||
@@ -20,7 +20,6 @@ var ReactInstrumentation = require('ReactInstrumentation');
|
||||
var ReactNodeTypes = require('ReactNodeTypes');
|
||||
var ReactPropTypeLocations = require('ReactPropTypeLocations');
|
||||
var ReactReconciler = require('ReactReconciler');
|
||||
var ReactUpdateQueue = require('ReactUpdateQueue');
|
||||
|
||||
var checkReactTypeSpec = require('checkReactTypeSpec');
|
||||
|
||||
@@ -197,8 +196,10 @@ var ReactCompositeComponentMixin = {
|
||||
|
||||
var Component = this._currentElement.type;
|
||||
|
||||
var updateQueue = transaction.getUpdateQueue();
|
||||
|
||||
// Initialize the public class
|
||||
var inst = this._constructComponent(publicProps, publicContext);
|
||||
var inst = this._constructComponent(publicProps, publicContext, updateQueue);
|
||||
var renderedElement;
|
||||
|
||||
// Support functional components
|
||||
@@ -245,7 +246,7 @@ var ReactCompositeComponentMixin = {
|
||||
inst.props = publicProps;
|
||||
inst.context = publicContext;
|
||||
inst.refs = emptyObject;
|
||||
inst.updater = ReactUpdateQueue;
|
||||
inst.updater = updateQueue;
|
||||
|
||||
this._instance = inst;
|
||||
|
||||
@@ -345,20 +346,20 @@ var ReactCompositeComponentMixin = {
|
||||
return markup;
|
||||
},
|
||||
|
||||
_constructComponent: function(publicProps, publicContext) {
|
||||
_constructComponent: function(publicProps, publicContext, updateQueue) {
|
||||
if (__DEV__) {
|
||||
ReactCurrentOwner.current = this;
|
||||
try {
|
||||
return this._constructComponentWithoutOwner(publicProps, publicContext);
|
||||
return this._constructComponentWithoutOwner(publicProps, publicContext, updateQueue);
|
||||
} finally {
|
||||
ReactCurrentOwner.current = null;
|
||||
}
|
||||
} else {
|
||||
return this._constructComponentWithoutOwner(publicProps, publicContext);
|
||||
return this._constructComponentWithoutOwner(publicProps, publicContext, updateQueue);
|
||||
}
|
||||
},
|
||||
|
||||
_constructComponentWithoutOwner: function(publicProps, publicContext) {
|
||||
_constructComponentWithoutOwner: function(publicProps, publicContext, updateQueue) {
|
||||
var Component = this._currentElement.type;
|
||||
var instanceOrElement;
|
||||
if (shouldConstruct(Component)) {
|
||||
@@ -370,7 +371,7 @@ var ReactCompositeComponentMixin = {
|
||||
);
|
||||
}
|
||||
}
|
||||
instanceOrElement = new Component(publicProps, publicContext, ReactUpdateQueue);
|
||||
instanceOrElement = new Component(publicProps, publicContext, updateQueue);
|
||||
if (__DEV__) {
|
||||
if (this._debugID !== 0) {
|
||||
ReactInstrumentation.debugTool.onEndLifeCycleTimer(
|
||||
@@ -390,7 +391,7 @@ var ReactCompositeComponentMixin = {
|
||||
);
|
||||
}
|
||||
}
|
||||
instanceOrElement = Component(publicProps, publicContext, ReactUpdateQueue);
|
||||
instanceOrElement = Component(publicProps, publicContext, updateQueue);
|
||||
if (__DEV__) {
|
||||
if (this._debugID !== 0) {
|
||||
ReactInstrumentation.debugTool.onEndLifeCycleTimer(
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
var CallbackQueue = require('CallbackQueue');
|
||||
var PooledClass = require('PooledClass');
|
||||
var Transaction = require('Transaction');
|
||||
var ReactUpdateQueue = require('ReactUpdateQueue');
|
||||
|
||||
/**
|
||||
* Provides a `CallbackQueue` queue for collecting `onDOMReady` callbacks during
|
||||
@@ -81,6 +82,13 @@ var Mixin = {
|
||||
return this.reactMountReady;
|
||||
},
|
||||
|
||||
/**
|
||||
* @return {object} The queue to collect React async events.
|
||||
*/
|
||||
getUpdateQueue: function() {
|
||||
return ReactUpdateQueue;
|
||||
},
|
||||
|
||||
/**
|
||||
* `PooledClass` looks for this, and will invoke this before allowing this
|
||||
* instance to be reused.
|
||||
|
||||
Reference in New Issue
Block a user