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.

Sign up for GitHub

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

ErrorBoundaries: Initial pass at the easy case of updates #6020

Closed
wants to merge 1 commit into from
Closed

ErrorBoundaries: Initial pass at the easy case of updates #6020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
351 changes: 350 additions & 1 deletion src/core/__tests__/ReactErrorBoundaries-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('ReactErrorBoundaries', function() {
expect(EventPluginHub.putListener).not.toBeCalled();
});

it('will catch exceptions in componentWillUnmount', function() {
it('will catch exceptions in componentWillUnmount initial render', function() {
class ErrorBoundary extends React.Component {
constructor() {
super();
Expand Down Expand Up @@ -236,4 +236,353 @@ describe('ReactErrorBoundaries', function() {
'Box componentWillUnmount',
]);
});

it('catches errors on update', function() {
var log = [];

class Box extends React.Component {
constructor(props) {
super(props);
this.state = {errorMessage: null};
}
render() {
if (this.state.errorMessage != null) {
log.push('Box renderError');
return <div>Error: {this.state.errorMessage}</div>;
}
log.push('Box render');
var ref = function(x) {
log.push('Inquisitive ref ' + x);
};
return (
<div>
<Inquisitive ref={ref} />
{this.props.angry ? <Angry /> : <div />}
</div>
);
}
unstable_handleError(e) {
log.push('error handled');
this.setState({errorMessage: e.message});
}
componentDidMount() {
log.push('Box componentDidMount');
}
componentWillUnmount() {
log.push('Box componentWillUnmount');
}
}

class Inquisitive extends React.Component {
render() {
log.push('Inquisitive render');
return <div>What is love?</div>;
}
componentDidMount() {
log.push('Inquisitive componentDidMount');
}
componentWillUnmount() {
log.push('Inquisitive componentWillUnmount');
}
}

class Angry extends React.Component {
render() {
log.push('Angry render');
throw new Error('Please, do not render me.');
}
componentDidMount() {
log.push('Angry componentDidMount');
}
componentWillUnmount() {
log.push('Angry componentWillUnmount');
}
}

var container = document.createElement('div');
ReactDOM.render(<Box angry={false} />, container);
ReactDOM.render(<Box angry={true} />, container);
expect(container.textContent).toBe('Error: Please, do not render me.');
expect(log).toEqual([
'Box render',
'Inquisitive render',
'Inquisitive componentDidMount',
'Inquisitive ref [object Object]',
'Box componentDidMount',
'Box render',
'Inquisitive ref null',
'Inquisitive render',
'Angry render',
'error handled',
'Inquisitive ref null',
'Inquisitive componentWillUnmount',
'Box renderError',
]);
});

it('catches componentWillUnmount errors on update', function() {
var log = [];

class ErrorBoundary extends React.Component {
constructor(props) {
super(props);
this.state = {errorMessage: null};
}
render() {
if (this.state.errorMessage != null) {
log.push('Box renderError');
return <div>Error: I am now a sad component :(</div>;
}
log.push('Box render');

return (
<div>
<BrokenUnmount />
<BrokenUnmount />
{this.props.angry ? null : <BrokenUnmount />}
</div>
);
}
unstable_handleError(e) {
log.push('error handled');
this.setState({errorMessage: e.message});
}
componentDidMount() {
log.push('Box componentDidMount');
}
componentWillUnmount() {
log.push('Box componentWillUnmount');
}
}

class BrokenUnmount extends React.Component {
render() {
return <div>text</div>;
}
componentWillUnmount() {
log.push('BrokenUnmount is attempting to unmount');
throw new Error('Always broken.');
}
}

var container = document.createElement('div');
ReactDOM.render(<ErrorBoundary angry={false} />, container);
ReactDOM.render(<ErrorBoundary angry={true} />, container);
expect(container.textContent).toBe('Error: I am now a sad component :(');
expect(log).toEqual([
'Box render',
'Box componentDidMount',
'Box render',
'BrokenUnmount is attempting to unmount',
'error handled',
'BrokenUnmount is attempting to unmount',
'BrokenUnmount is attempting to unmount',
'BrokenUnmount is attempting to unmount',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little unfortunate that we re-call componentWillUnmount on the component that already failed to unmount.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was fixed in #6613.

'Box renderError',
]);
ReactDOM.unmountComponentAtNode(container);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add log.length = 0; before this line so that the next expect is clearer?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

expect(log).toEqual([
'Box render',
'Box componentDidMount',
'Box render',
'BrokenUnmount is attempting to unmount',
'error handled',
'BrokenUnmount is attempting to unmount',
'BrokenUnmount is attempting to unmount',
'BrokenUnmount is attempting to unmount',
'Box renderError',
'Box componentWillUnmount',
]);
});

it('catches componentWillUnmount errors nested children', function() {
class ErrorBoundary extends React.Component {
constructor(props) {
super(props);
this.state = {errorMessage: null};
}
render() {
if (this.state.errorMessage != null) {
return <div>Error: I am now a sad component :(</div>;
}

return (
<div>
<InnocentParent />
{this.props.angry ? null : <InnocentParent />}
</div>
);
}
unstable_handleError(e) {
this.setState({errorMessage: e.message});
}
}

class InnocentParent extends React.Component {
render() {
return <BrokenUnmount />;
}
}

class BrokenUnmount extends React.Component {
render() {
return <div>text</div>;
}
componentWillUnmount() {
throw new Error('Always broken.');
}
}

var container = document.createElement('div');
ReactDOM.render(<ErrorBoundary angry={false} />, container);
ReactDOM.render(<ErrorBoundary angry={true} />, container);
expect(container.textContent).toBe('Error: I am now a sad component :(');
ReactDOM.unmountComponentAtNode(container);
});

it('doesn\'t get into inconsistent state during removals', function() {
class ErrorBoundary extends React.Component {
constructor(props) {
super(props);
this.state = {errorMessage: null};
}
render() {
if (this.state.errorMessage != null) {
return <div>Error: I am now a sad component :(</div>;
}

return (
<div>{this.props.children}</div>
);
}
unstable_handleError(e) {
this.setState({errorMessage: e.message});
}
}

class InnocentComponent extends React.Component {
render() {
return <div>text</div>;
}
}

class GuiltyComponent extends React.Component {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call this BrokenUnmount here or GuiltyComponent everywhere for consistency?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

render() {
return <div>text</div>;
}
componentWillUnmount() {
throw new Error('Always broken.');
}
}

var container = document.createElement('div');
ReactDOM.render(<ErrorBoundary><InnocentComponent /><GuiltyComponent /><InnocentComponent /></ErrorBoundary>, container);
ReactDOM.render(<ErrorBoundary />, container);
expect(container.textContent).toBe('Error: I am now a sad component :(');
ReactDOM.unmountComponentAtNode(container);
});

it('doesn\'t get into inconsistent state during additions', function() {
class ErrorBoundary extends React.Component {
constructor(props) {
super(props);
this.state = {errorMessage: null};
}
render() {
if (this.state.errorMessage != null) {
return <div>Error: I am now a sad component :(</div>;
}

return (
<div>{this.props.children}</div>
);
}
unstable_handleError(e) {
this.setState({errorMessage: e.message});
}
}

class InnocentComponent extends React.Component {
render() {
return <div>text</div>;
}
}

class GuiltyComponent extends React.Component {
render() {
throw new Error('Always broken.');
}
}

var container = document.createElement('div');
ReactDOM.render(<ErrorBoundary />, container);
ReactDOM.render(<ErrorBoundary><InnocentComponent /><GuiltyComponent /><InnocentComponent /></ErrorBoundary>, container);
expect(container.textContent).toBe('Error: I am now a sad component :(');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we test that refs/componentDidMount are not called on any of the mounting children but that they are called for the displayed error message?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

ReactDOM.unmountComponentAtNode(container);
});

it('doesn\'t get into inconsistent state during reorders', function() {

function generateElements() {
var elements = [];
for (var i = 0; i < 100; i++) {
elements.push(<InnocentComponent key={i} />);
}
elements.push(<GuiltyComponent key={100} />);

var currentIndex = elements.length;
while (0 !== currentIndex) {
var randomIndex = Math.floor(Math.random() * currentIndex);
currentIndex -= 1;
var temporaryValue = elements[currentIndex];
elements[currentIndex] = elements[randomIndex];
elements[randomIndex] = temporaryValue;
}
return elements;
}

class ErrorBoundary extends React.Component {
constructor(props) {
super(props);
this.state = {errorMessage: null};
}
render() {
if (this.state.errorMessage != null) {
return <div>Error: I am now a sad component :(</div>;
}

return (
<div>{this.props.children}</div>
);
}
unstable_handleError(e) {
this.setState({errorMessage: e.message});
}
}

class InnocentComponent extends React.Component {
render() {
return <div>text</div>;
}
}

class GuiltyComponent extends React.Component {
render() {
if (fail) {
throw new Error('Always broken.');
}
return <div>text</div>;
}
}

var fail = false;

var container = document.createElement('div');
ReactDOM.render(<ErrorBoundary>{generateElements()}</ErrorBoundary>, container);
fail = true;
ReactDOM.render(<ErrorBoundary>{generateElements()}</ErrorBoundary>, container);

expect(container.textContent).toBe('Error: I am now a sad component :(');
ReactDOM.unmountComponentAtNode(container);
});
});
Loading