Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change the argument passed to CallbackQueue.getPooled #10101

Merged
merged 10 commits into from
Jul 12, 2017
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 @@ -1972,6 +1973,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 @@ -2001,6 +2003,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 @@ -844,4 +844,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