Skip to content

Commit 382fd04

Browse files
author
Brian Vaughn
committed
Add failing test for sCU current props
this.props is not reset when a render is bailed out on. This can cause potential problems for subsequent calls to shouldComponentUpdate if certain types of comparisons are made between this.props and nextProps.
1 parent 6e47f43 commit 382fd04

File tree

2 files changed

+83
-0
lines changed

2 files changed

+83
-0
lines changed

scripts/fiber/tests-failing.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ src/renderers/shared/__tests__/ReactPerf-test.js
5757
* should not count time in a portal towards lifecycle method
5858
* should work when measurement starts during reconciliation
5959

60+
src/renderers/shared/fiber/__tests__/ReactIncremental-test.js
61+
* should restore previous props when work is bailed out before commit
62+
6063
src/renderers/shared/hooks/__tests__/ReactComponentTreeHook-test.js
6164
* can be retrieved by ID
6265

src/renderers/shared/fiber/__tests__/ReactIncremental-test.js

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2021,4 +2021,84 @@ describe('ReactIncremental', () => {
20212021
});
20222022
ReactNoop.flush();
20232023
});
2024+
2025+
it('should restore previous props when work is bailed out before commit', () => {
2026+
let cduNextProps = null;
2027+
let cduPrevProps = null;
2028+
let scuNextProps = null;
2029+
let scuPrevProps = null;
2030+
2031+
function SecondChild(props) {
2032+
return <span>{props.children}</span>;
2033+
}
2034+
2035+
class FirstChild extends React.Component {
2036+
componentDidUpdate(prevProps, prevState) {
2037+
cduNextProps = this.props;
2038+
cduPrevProps = prevProps;
2039+
}
2040+
shouldComponentUpdate(nextProps, nextState) {
2041+
scuNextProps = nextProps;
2042+
scuPrevProps = this.props;
2043+
return this.props.children !== nextProps.children;
2044+
}
2045+
render() {
2046+
return <span>{this.props.children}</span>;
2047+
}
2048+
}
2049+
2050+
class Middle extends React.Component {
2051+
render() {
2052+
return (
2053+
<div>
2054+
<FirstChild>{this.props.flag}</FirstChild>
2055+
<SecondChild>{this.props.flag}</SecondChild>
2056+
</div>
2057+
);
2058+
}
2059+
}
2060+
2061+
function Root(props) {
2062+
return (
2063+
<div hidden={true}>
2064+
<Middle {...props} />
2065+
</div>
2066+
);
2067+
}
2068+
2069+
// Initial render of the entire tree.
2070+
// Renders: Root, Middle, FirstChild, SecondChild
2071+
ReactNoop.render(<Root flag={0} />);
2072+
ReactNoop.flush();
2073+
2074+
// Schedule low priority work to update children.
2075+
// Renders: Root
2076+
ReactNoop.render(<Root flag={1} />);
2077+
ReactNoop.flushDeferredPri(20);
2078+
2079+
// Children are now pending rendering.
2080+
// Give them enough time to partially render.
2081+
// Renders: Middle, FirstChild
2082+
ReactNoop.flushDeferredPri(30 + 5);
2083+
2084+
// At this point sCU should have been called with previous:0, next:1
2085+
// Since the render is not completed cDU should not be called yet.
2086+
expect(scuPrevProps.children).toEqual(0);
2087+
expect(scuNextProps.children).toEqual(1);
2088+
expect(cduPrevProps).toEqual(null);
2089+
expect(cduNextProps).toEqual(null);
2090+
2091+
// Then interrupt the partial render with higher priority work.
2092+
// The in-progress child content will bailout.
2093+
// Renders: Root, Middle, FirstChild, SecondChild
2094+
ReactNoop.render(<Root flag={2} />);
2095+
ReactNoop.flush();
2096+
2097+
// At this point, since the low priority work was bailed out on,
2098+
// sCU and cDU should have been called with previous:0, next:2
2099+
expect(scuPrevProps.children).toEqual(0);
2100+
expect(scuNextProps.children).toEqual(2);
2101+
expect(cduPrevProps.children).toEqual(0);
2102+
expect(cduNextProps.children).toEqual(2);
2103+
});
20242104
});

0 commit comments

Comments
 (0)