Skip to content

Commit

Permalink
Add failing test for sCU current props
Browse files Browse the repository at this point in the history
This.props is not reset when a render is bailed out on. This can cause potential problems for subsequent calls to shouldComponentUpdate.
  • Loading branch information
Brian Vaughn committed Dec 27, 2016
1 parent 6e47f43 commit 9ee0624
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 0 deletions.
3 changes: 3 additions & 0 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ src/renderers/shared/__tests__/ReactPerf-test.js
* should not count time in a portal towards lifecycle method
* should work when measurement starts during reconciliation

src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
* should restore previous props when work is bailed out before commit

src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js
* can be retrieved by ID

Expand Down
80 changes: 80 additions & 0 deletions src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2021,4 +2021,84 @@ describe('ReactIncremental', () => {
});
ReactNoop.flush();
});

it('should restore previous props when work is bailed out before commit', () => {
let cduNextProps = null
let cduPrevProps = null
let scuNextProps = null
let scuPrevProps = null

function SecondChild (props) {
return <span>{props.children}</span>;
}

class FirstChild extends React.Component {
componentDidUpdate(prevProps, prevState) {
cduNextProps = this.props
cduPrevProps = prevProps
}
shouldComponentUpdate(nextProps, nextState) {
scuNextProps = nextProps
scuPrevProps = this.props
return this.props.children !== nextProps.children;
}
render() {
return <span>{this.props.children}</span>;
}
}

class Middle extends React.Component {
render() {
return (
<div>
<FirstChild>{this.props.flag}</FirstChild>
<SecondChild>{this.props.flag}</SecondChild>
</div>
);
}
}

function Root(props) {
return (
<div hidden={true}>
<Middle {...props} />
</div>
);
}

// Initial render of the entire tree.
// Renders: Root, Middle, FirstChild, SecondChild
ReactNoop.render(<Root flag={0} />);
ReactNoop.flush();

// Schedule low priority work to update children.
// Renders: Root
ReactNoop.render(<Root flag={1} />);
ReactNoop.flushDeferredPri(20);

// Children are now pending rendering.
// Give them enough time to partially render.
// Renders: Middle, FirstChild
ReactNoop.flushDeferredPri(30 + 5);

// At this point sCU should have been called with previous:0, next:1
// Since the render is not completed cDU should not be called yet.
expect(scuPrevProps.children).toEqual(0)
expect(scuNextProps.children).toEqual(1)
expect(cduPrevProps).toEqual(null)
expect(cduNextProps).toEqual(null)

// Then interrupt the partial render with higher priority work.
// The in-progress child content will bailout.
// Renders: Root, Middle, FirstChild, SecondChild
ReactNoop.render(<Root flag={2} />);
ReactNoop.flush();

// At this point, since the low priority work was bailed out on,
// sCU and cDU should have been called with previous:0, next:2
expect(scuPrevProps.children).toEqual(0)
expect(scuNextProps.children).toEqual(2)
expect(cduPrevProps.children).toEqual(0)
expect(cduNextProps.children).toEqual(2)
});
});

0 comments on commit 9ee0624

Please sign in to comment.