Skip to content

Commit

Permalink
Fixes an issue where setState callback was not called
Browse files Browse the repository at this point in the history
  • Loading branch information
Sampo Kivistö authored and Sampo Kivistö committed Nov 7, 2018
1 parent 85a8b11 commit 21b11ee
Show file tree
Hide file tree
Showing 13 changed files with 188 additions and 67 deletions.
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@
"test:package": "node fixtures/packaging/build-all.js"
},
"devDependencies": {
"@babel/core": "^7.1.2",
"@babel/core": "^7.1.5",
"@babel/plugin-proposal-class-properties": "7.1.0",
"@babel/preset-env": "7.1.0",
"@babel/preset-env": "7.1.5",
"@babel/preset-typescript": "^7.1.0",
"@types/history": "^4.7.2",
"@types/jest": "^23.3.9",
Expand All @@ -107,9 +107,9 @@
"npm-run-all": "^4.1.3",
"perf-monitor": "^0.4.1",
"pre-commit": "^1.2.2",
"prettier": "^1.14.3",
"prettier": "^1.15.1",
"rimraf": "^2.6.2",
"rollup": "^0.66.6",
"rollup": "^0.67.0",
"rollup-plugin-alias": "^1.4.0",
"rollup-plugin-babel": "^4.0.3",
"rollup-plugin-buble": "^0.19.4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ describe('FormElements', () => {
let callCounter = 0;
let args = [];

const spy = function (arg) {
const spy = function(arg) {
callCounter++;
args.push(arg);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ describe('BUG: instance - null', () => {
}

/*
* FireFox v51.0 fixes issue with href
* iOS still broken (21.1.2017)
*/
* FireFox v51.0 fixes issue with href
* iOS still broken (21.1.2017)
*/

return (
<svg focusable="false" className={'svgicon'}>
Expand Down
18 changes: 10 additions & 8 deletions packages/inferno-create-element/__tests__/svg.spec.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { render, Component } from 'inferno';
import { innerHTML } from 'inferno-utils';
import { createElement } from 'inferno-create-element';
import { rerender } from "../../inferno-compat/src";
import { rerender } from '../../inferno-compat/src';

describe('createTree - SVG (JSX)', () => {
let container;
Expand Down Expand Up @@ -210,7 +210,6 @@ describe('createTree - SVG (JSX)', () => {
expect(innerHTML(container.innerHTML)).toBe(innerHTML('<svg id="test"></svg>'));
});


describe('SVG elements', () => {
it('Should keep SVG children flagged when parent is SVG', () => {
class Rect extends Component {
Expand All @@ -224,15 +223,18 @@ describe('createTree - SVG (JSX)', () => {
}

render() {
return (
createElement('rect', {
className: this.state.className
})
)
return createElement('rect', {
className: this.state.className
});
}
}

render(<svg><Rect/></svg>, container);
render(
<svg>
<Rect />
</svg>,
container
);

expect(container.firstChild.firstChild.getAttribute('class')).toBe('foo');

Expand Down
15 changes: 9 additions & 6 deletions packages/inferno-hydrate/__tests__/hydrate.spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -1231,15 +1231,18 @@ describe('rendering routine', () => {
}

render() {
return (
createElement('rect', {
className: this.state.className
})
)
return createElement('rect', {
className: this.state.className
});
}
}

hydrate(<svg><Rect/></svg>, container);
hydrate(
<svg>
<Rect />
</svg>,
container
);

expect(container.firstChild.firstChild.getAttribute('class')).toBe('foo');

Expand Down
16 changes: 8 additions & 8 deletions packages/inferno-mobx/src/observer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ function patch(target, funcName, runMixinFirst) {
const f = !base
? mixinFunc
: runMixinFirst === true
? function() {
mixinFunc.apply(this, arguments);
base.apply(this, arguments);
}
: function() {
base.apply(this, arguments);
mixinFunc.apply(this, arguments);
};
? function() {
mixinFunc.apply(this, arguments);
base.apply(this, arguments);
}
: function() {
base.apply(this, arguments);
mixinFunc.apply(this, arguments);
};

// MWE: ideally we freeze here to protect against accidental overwrites in component instances, see #195
// ...but that breaks react-hot-loader, see #231...
Expand Down
8 changes: 3 additions & 5 deletions packages/inferno-server/src/renderToString.queuestream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,10 @@ export class RenderQueueStream extends Readable {
if (!hasNewAPI && isFunction(instance.componentWillMount)) {
instance.$BR = true;
instance.componentWillMount();
if (instance.$PSS) {
const pending = instance.$PS;

if (pending) {
const state = instance.state;
const pending = instance.$PS;

if (state === null) {
instance.state = pending;
Expand All @@ -105,7 +106,6 @@ export class RenderQueueStream extends Readable {
state[key] = pending[key];
}
}
instance.$PSS = false;
instance.$PS = null;
}
instance.$BR = false;
Expand All @@ -118,7 +118,6 @@ export class RenderQueueStream extends Readable {
const promisePosition = this.promises.push([]) - 1;
this.addToQueue(
initialProps.then(dataForContext => {
instance.$PSS = false;
if (typeof dataForContext === 'object') {
instance.props = combineFrom(instance.props, dataForContext);
}
Expand Down Expand Up @@ -149,7 +148,6 @@ export class RenderQueueStream extends Readable {
instance.state = createDerivedState(instance, props, instance.state);
}
const renderOutput = instance.render(instance.props, instance.state, instance.context);
instance.$PSS = false;

if (isInvalid(renderOutput)) {
this.addToQueue('<!--!-->', position);
Expand Down
6 changes: 2 additions & 4 deletions packages/inferno-server/src/renderToString.stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ export class RenderStream extends Readable {
instance.$BR = true;

return Promise.resolve(!hasNewAPI && instance.componentWillMount && instance.componentWillMount()).then(() => {
if (instance.$PSS) {
const pending = instance.$PS;
if (pending) {
const state = instance.state;
const pending = instance.$PS;

if (state === null) {
instance.state = pending;
Expand All @@ -94,7 +94,6 @@ export class RenderStream extends Readable {
state[key] = pending[key];
}
}
instance.$PSS = false;
instance.$PS = null;
}

Expand All @@ -103,7 +102,6 @@ export class RenderStream extends Readable {
instance.state = createDerivedState(instance, props, instance.state);
}
const renderOutput = instance.render(instance.props, instance.state, instance.context);
instance.$PSS = false;

if (isInvalid(renderOutput)) {
return this.push('<!--!-->');
Expand Down
5 changes: 3 additions & 2 deletions packages/inferno-server/src/renderToString.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ function renderVNodeToString(vNode, parent, context): string {
instance.$BR = true;
instance.componentWillMount();
instance.$BR = false;
if (instance.$PSS) {
const pending = instance.$PS;

if (pending) {
const state = instance.state;
const pending = instance.$PS;

if (state === null) {
instance.state = pending;
Expand Down
128 changes: 128 additions & 0 deletions packages/inferno/__tests__/setState.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ describe('setState', () => {

beforeEach(function() {
container = document.createElement('div');
document.body.appendChild(container);
});

afterEach(function() {
rerender(); // Flush pending stuff, if any
render(null, container);
document.body.removeChild(container);
container.innerHTML = '';
});

Expand Down Expand Up @@ -914,4 +916,130 @@ describe('setState', () => {

expect(container.textContent).toBe('ACTIVE : ACTIVE');
});

it('Should call all setState callbacks', () => {
let counter = 0;
let setStateCounter = 0;

class Child extends Component<any, any> {
constructor(props) {
super(props);
this.state = {test: false};
this.callCallback = this.callCallback.bind(this);
}

public callCallback() {
setStateCounter++;
this.setState({test: true}, () => {
counter++;
});
}

public render() {
if (setStateCounter === 1) {
this.callCallback();
this.props.callback(true);
}

return (
<div>Child</div>
);
}
}

class MidChild extends Component<any, any> {
constructor(props) {
super(props);
this.state = {test: false};
this.callCallback = this.callCallback.bind(this);
}

public callCallback() {
setStateCounter++;
this.setState({test: true}, () => {
counter++;
this.props.callback(true);
});
}

public render() {
return [
<button onClick={this.callCallback}>Click</button>,
<Child callback={this.props.callback}/>
];
}
}

class Parent extends Component<any, any> {
constructor(props) {
super(props);
this.state = {test: false, foobar: false};
this.doSomething = this.doSomething.bind(this);
this.callCallback = this.callCallback.bind(this);
}

public callCallback(testValue) {
setStateCounter++;
this.setState({test: testValue}, () => {
counter++;
this.props.callback(true);
});
}

public doSomething() {
setStateCounter++;
this.setState({foobar: true}, () => {
counter++;
});
}

public render() {
return (
<div>
<MidChild callback={this.callCallback} foobar={this.state.foobar}/>
</div>
);
}
}

class Wrapper extends Component<any, any> {
constructor(props) {
super(props);
this.state = {didCounter: 0};
this.doSomething = this.doSomething.bind(this);
}

public doSomething() {
setStateCounter++;

this.setState({didCounter: ++this.state.didCounter}, () => {
counter++;
});
}

public render() {
return (
<div>
<Parent callback={this.doSomething}/>
<span>{this.state.didCounter}</span>
</div>
);
}
}

render(<Wrapper />, container);

expect(counter).toBe(0);
expect(setStateCounter).toBe(0);

const btn = container.querySelector('button');

btn.click();

rerender();
expect(setStateCounter).toBe(6);
expect(counter).toBe(6);

expect(container.innerHTML).toEqual('<div><div><button>Click</button><div>Child</div></div><span>2</span></div>');
})
});
3 changes: 1 addition & 2 deletions packages/inferno/src/DOM/patching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -393,9 +393,8 @@ function patchClassComponent(lastVNode, nextVNode, parentDOM, context, isSVG: bo
}
instance.$BR = false;
}
if (instance.$PSS) {
if (!isNull(instance.$PS)) {
nextState = combineFrom(nextState, instance.$PS) as any;
instance.$PSS = false;
instance.$PS = null;
}
}
Expand Down
6 changes: 3 additions & 3 deletions packages/inferno/src/DOM/utils/componentutil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,10 @@ export function createClassComponentInstance(vNode: VNode, Component, props, con
instance.$BR = true;
instance.componentWillMount();

if (instance.$PSS) {
const pending = instance.$PS as any;

if (!isNull(pending)) {
const state = instance.state;
const pending = instance.$PS as any;

if (isNull(state)) {
instance.state = pending;
Expand All @@ -81,7 +82,6 @@ export function createClassComponentInstance(vNode: VNode, Component, props, con
state[key] = pending[key];
}
}
instance.$PSS = false;
instance.$PS = null;
}

Expand Down
Loading

0 comments on commit 21b11ee

Please sign in to comment.