From 50d905b0838857e76f7eb2f0875047c264f4c24e Mon Sep 17 00:00:00 2001 From: Garrett McCullough Date: Wed, 12 Jul 2017 10:49:13 -0700 Subject: [PATCH] change the argument passed to CallbackQueue.getPooled (#10101) * change the argument passed to CallbackQueue.getPooled * remove undefined from function call * add test for ReactNativeReconcileTransaction * update log of tests * change test to one that operates on setState * added new tests and fixed another instance of the bug * run prettier * update names of tests and minor clean up * remove arg from CallbackQueue and update tests --- scripts/fiber/tests-passing.txt | 3 +++ .../__tests__/ReactCompositeComponent-test.js | 15 +++++++++++++++ .../dom/stack/client/ReactReconcileTransaction.js | 2 +- .../dom/test/__tests__/ReactTestUtils-test.js | 15 +++++++++++++++ .../native/ReactNativeReconcileTransaction.js | 2 +- .../native/__tests__/ReactNativeMount-test.js | 15 +++++++++++++++ .../shared/stack/reconciler/CallbackQueue.js | 11 ++++------- 7 files changed, 54 insertions(+), 9 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index f642283bdbb9e..0a68591cb7bfb 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -397,6 +397,7 @@ src/renderers/__tests__/ReactCompositeComponent-test.js * prepares new child before unmounting old * respects a shallow shouldComponentUpdate implementation * does not do a deep comparison for a shallow shouldComponentUpdate implementation +* should call setState callback with no arguments src/renderers/__tests__/ReactCompositeComponentDOMMinimalism-test.js * should not render extra nodes for non-interpolated text @@ -1975,6 +1976,7 @@ src/renderers/dom/test/__tests__/ReactTestUtils-test.js * should enable rendering of cloned element * should set the type of the event * should work with renderIntoDocument +* should call setState callback with no arguments src/renderers/native/__tests__/ReactNativeAttributePayload-test.js * should work with simple example @@ -2004,6 +2006,7 @@ src/renderers/native/__tests__/ReactNativeMount-test.js * should be able to create and update a native component * returns the correct instance and calls it in the callback * renders and reorders children +* calls setState with no arguments src/renderers/native/__tests__/createReactNativeComponentClass-test.js * should register viewConfigs diff --git a/src/renderers/__tests__/ReactCompositeComponent-test.js b/src/renderers/__tests__/ReactCompositeComponent-test.js index df10b88867523..2f69aa9ebcd4b 100644 --- a/src/renderers/__tests__/ReactCompositeComponent-test.js +++ b/src/renderers/__tests__/ReactCompositeComponent-test.js @@ -1472,4 +1472,19 @@ describe('ReactCompositeComponent', () => { instance.setState(getInitialState()); expect(renderCalls).toBe(3); }); + + it('should call setState callback with no arguments', () => { + let mockArgs; + class Component extends React.Component { + componentDidMount() { + this.setState({}, (...args) => (mockArgs = args)); + } + render() { + return false; + } + } + + ReactTestUtils.renderIntoDocument(); + expect(mockArgs.length).toEqual(0); + }); }); diff --git a/src/renderers/dom/stack/client/ReactReconcileTransaction.js b/src/renderers/dom/stack/client/ReactReconcileTransaction.js index 732709e2112e5..7047e47fa694f 100644 --- a/src/renderers/dom/stack/client/ReactReconcileTransaction.js +++ b/src/renderers/dom/stack/client/ReactReconcileTransaction.js @@ -120,7 +120,7 @@ function ReactReconcileTransaction(useCreateElement) { // accessible and defaults to false when `ReactDOMComponent` and // `ReactDOMTextComponent` checks it in `mountComponent`.` this.renderToStaticMarkup = false; - this.reactMountReady = CallbackQueue.getPooled(null); + this.reactMountReady = CallbackQueue.getPooled(); this.useCreateElement = useCreateElement; } diff --git a/src/renderers/dom/test/__tests__/ReactTestUtils-test.js b/src/renderers/dom/test/__tests__/ReactTestUtils-test.js index b86f3fa0a2782..e320887c32378 100644 --- a/src/renderers/dom/test/__tests__/ReactTestUtils-test.js +++ b/src/renderers/dom/test/__tests__/ReactTestUtils-test.js @@ -946,4 +946,19 @@ describe('ReactTestUtils', () => { ); }); }); + + it('should call setState callback with no arguments', () => { + let mockArgs; + class Component extends React.Component { + componentDidMount() { + this.setState({}, (...args) => (mockArgs = args)); + } + render() { + return false; + } + } + + ReactTestUtils.renderIntoDocument(); + expect(mockArgs.length).toEqual(0); + }); }); diff --git a/src/renderers/native/ReactNativeReconcileTransaction.js b/src/renderers/native/ReactNativeReconcileTransaction.js index 9ee4fc9815ac0..f97b04fe004f0 100644 --- a/src/renderers/native/ReactNativeReconcileTransaction.js +++ b/src/renderers/native/ReactNativeReconcileTransaction.js @@ -67,7 +67,7 @@ if (__DEV__) { */ function ReactNativeReconcileTransaction() { this.reinitializeTransaction(); - this.reactMountReady = CallbackQueue.getPooled(null); + this.reactMountReady = CallbackQueue.getPooled(); } var Mixin = { diff --git a/src/renderers/native/__tests__/ReactNativeMount-test.js b/src/renderers/native/__tests__/ReactNativeMount-test.js index 9b4556dbaaf87..d65a633c7f4c4 100644 --- a/src/renderers/native/__tests__/ReactNativeMount-test.js +++ b/src/renderers/native/__tests__/ReactNativeMount-test.js @@ -108,4 +108,19 @@ describe('ReactNative', () => { ReactNative.render(, 11); expect(UIManager.__dumpHierarchyForJestTestsOnly()).toMatchSnapshot(); }); + + it('calls setState with no arguments', () => { + var mockArgs; + class Component extends React.Component { + componentDidMount() { + this.setState({}, (...args) => (mockArgs = args)); + } + render() { + return false; + } + } + + ReactNative.render(, 11); + expect(mockArgs.length).toEqual(0); + }); }); diff --git a/src/renderers/shared/stack/reconciler/CallbackQueue.js b/src/renderers/shared/stack/reconciler/CallbackQueue.js index 0362598ac1e6a..ea10bb4be4057 100644 --- a/src/renderers/shared/stack/reconciler/CallbackQueue.js +++ b/src/renderers/shared/stack/reconciler/CallbackQueue.js @@ -28,15 +28,13 @@ var validateCallback = require('validateCallback'); * @implements PooledClass * @internal */ -class CallbackQueue { - _callbacks: ?Array<(arg: Targ) => void>; +class CallbackQueue { + _callbacks: ?Array<() => void>; _contexts: ?Array; - _arg: Targ; - constructor(arg) { + constructor() { this._callbacks = null; this._contexts = null; - this._arg = arg; } /** @@ -62,7 +60,6 @@ class CallbackQueue { notifyAll() { var callbacks = this._callbacks; var contexts = this._contexts; - var arg = this._arg; if (callbacks && contexts) { invariant( callbacks.length === contexts.length, @@ -72,7 +69,7 @@ class CallbackQueue { this._contexts = null; for (var i = 0; i < callbacks.length; i++) { validateCallback(callbacks[i]); - callbacks[i].call(contexts[i], arg); + callbacks[i].call(contexts[i]); } callbacks.length = 0; contexts.length = 0;