Skip to content

Commit

Permalink
change the argument passed to CallbackQueue.getPooled (#10101)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
gwmccull authored and gaearon committed Jul 12, 2017
1 parent 5d5589b commit 50d905b
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 9 deletions.
3 changes: 3 additions & 0 deletions scripts/fiber/tests-passing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions src/renderers/__tests__/ReactCompositeComponent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(<Component />);
expect(mockArgs.length).toEqual(0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
15 changes: 15 additions & 0 deletions src/renderers/dom/test/__tests__/ReactTestUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(<Component />);
expect(mockArgs.length).toEqual(0);
});
});
2 changes: 1 addition & 1 deletion src/renderers/native/ReactNativeReconcileTransaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ if (__DEV__) {
*/
function ReactNativeReconcileTransaction() {
this.reinitializeTransaction();
this.reactMountReady = CallbackQueue.getPooled(null);
this.reactMountReady = CallbackQueue.getPooled();
}

var Mixin = {
Expand Down
15 changes: 15 additions & 0 deletions src/renderers/native/__tests__/ReactNativeMount-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,4 +108,19 @@ describe('ReactNative', () => {
ReactNative.render(<Component chars={after} />, 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(<Component />, 11);
expect(mockArgs.length).toEqual(0);
});
});
11 changes: 4 additions & 7 deletions src/renderers/shared/stack/reconciler/CallbackQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,13 @@ var validateCallback = require('validateCallback');
* @implements PooledClass
* @internal
*/
class CallbackQueue<T, Targ> {
_callbacks: ?Array<(arg: Targ) => void>;
class CallbackQueue<T> {
_callbacks: ?Array<() => void>;
_contexts: ?Array<T>;
_arg: Targ;

constructor(arg) {
constructor() {
this._callbacks = null;
this._contexts = null;
this._arg = arg;
}

/**
Expand All @@ -62,7 +60,6 @@ class CallbackQueue<T, Targ> {
notifyAll() {
var callbacks = this._callbacks;
var contexts = this._contexts;
var arg = this._arg;
if (callbacks && contexts) {
invariant(
callbacks.length === contexts.length,
Expand All @@ -72,7 +69,7 @@ class CallbackQueue<T, Targ> {
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;
Expand Down

0 comments on commit 50d905b

Please sign in to comment.