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

CPLAT-5599 Add temporary JS error boundary #291

Merged
merged 5 commits into from
May 7, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
77 changes: 71 additions & 6 deletions lib/src/component/error_boundary.dart
Original file line number Diff line number Diff line change
@@ -1,14 +1,72 @@
import 'dart:html';
import 'dart:js_util' as js_util;

import 'package:js/js.dart';
import 'package:meta/meta.dart';
import 'package:over_react/over_react.dart';
import 'package:react/react_client.dart';
import 'package:react/react_client/js_interop_helpers.dart';
import 'package:react/react_client/react_interop.dart' show React, ReactClassConfig;

// ignore: uri_has_not_been_generated
part 'error_boundary.over_react.g.dart';

/// A __temporary, private JS component for use only by [ErrorBoundary]__ that utilizes its own lightweight
/// JS interop to make use of the ReactJS 16 `componentDidCatch` lifecycle method to prevent consumer
/// react component trees from unmounting as a result of child component errors being "uncaught".
///
/// > __Why this is here__
/// >
/// > In order to release react-dart 5.0.0 _(which upgrades to ReactJS 16)_
/// without depending on Dart 2 / `Component2` (coming in react-dart 5.1.0) / `UiComponent2` (coming in over_react 3.1.0) -
/// and all the new lifecycle methods that those expose, we need to ensure that - at a minimum - the `componentDidCatch`
/// lifecycle method is handled by components wrapped in our [ErrorBoundary] component so that the behavior of
/// an application when a component within a tree throws - is the same as it was when using ReactJS 15.
/// >
/// > Otherwise, the update to react-dart 5.0.0 / over_react 3.0.0 will result in consumer apps rendering completely
/// "blank" screens when their trees unmount as a result of a child component throwing an error.
/// This would be unexpected, unwanted - and since we will not add a Dart js-interop layer around `componentDidCatch`
/// until react-dart 5.1.0 / over_react 3.1.0 - unmanageable for consumers.
///
/// __This will be removed in over_react 3.1.0__ once [ErrorBoundaryComponent] is extending from `UiStatefulComponent2`
/// which will ensure that the [ErrorBoundaryComponent.componentDidCatch] lifecycle method has real js-interop bindings
/// via react-dart 5.1.0's `Component2` base class.
///
/// TODO: Remove in 3.1.0
final ReactElement Function([Map props, List children]) _jsErrorBoundaryComponentFactory = (() {
var componentClass = React.createClass(jsifyAndAllowInterop({
'displayName': 'JsErrorBoundary',
'render': allowInteropCaptureThis((jsThis) {
final jsProps = js_util.getProperty(jsThis, 'props');
return js_util.getProperty(jsProps, 'children');
}),
'componentDidCatch': allowInteropCaptureThis((jsThis, error, info) {
final jsProps = js_util.getProperty(jsThis, 'props');
js_util.getProperty(jsProps, 'onComponentDidCatch')(error, info);
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
}),
}));

// Despite what the ReactJS docs say about only needing _either_ componentDidCatch or getDerivedStateFromError
// in order to define an "error boundary" component, that is not actually the case.
//
// The tree will never get re-rendered after an error is caught unless both are defined.
// ignore: argument_type_not_assignable
js_util.setProperty(componentClass, 'getDerivedStateFromError', allowInterop((_) => js_util.newObject()));

var reactFactory = React.createFactory(componentClass);

return ([Map props = const {}, List children = const []]) {
return reactFactory(jsifyAndAllowInterop(props), listifyChildren(children));
};
})();

// TODO: Need to type the second argument once react-dart implements bindings for the ReactJS "componentStack".
typedef _ComponentDidCatchCallback(Error error, /*ComponentStack*/dynamic componentStack);
// TODO: Will it be possible to make the first argument typed as a real `Error` instance again in Dart 2?
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
typedef _ComponentDidCatchCallback(/*Error*/dynamic error, /*ComponentStack*/dynamic componentStack);

// TODO: Need to type the second argument once react-dart implements bindings for the ReactJS "componentStack".
typedef ReactElement _FallbackUiRenderer(Error error, /*ComponentStack*/dynamic componentStack);
// TODO: Will it be possible to make the first argument typed as a real `Error` instance again in Dart 2?
typedef ReactElement _FallbackUiRenderer(/*Error*/dynamic error, /*ComponentStack*/dynamic componentStack);

/// A higher-order component that will catch ReactJS errors anywhere within the child component tree and
/// display a fallback UI instead of the component tree that crashed.
Expand Down Expand Up @@ -93,12 +151,19 @@ class ErrorBoundaryComponent<T extends ErrorBoundaryProps, S extends ErrorBounda
}

@override
render() => state.hasError
? props.fallbackUIRenderer(_error, _componentStack)
: props.children.single;
render() {
// TODO: 3.1.0 - Remove the `_jsErrorBoundaryComponentFactory`, and restore just the children of it once this component is extending from `UiStatefulComponent2`.
return _jsErrorBoundaryComponentFactory({
'onComponentDidCatch': props.onComponentDidCatch
},
state.hasError
? [props.fallbackUIRenderer(_error, _componentStack)]
: props.children
);
}

ReactElement _renderDefaultFallbackUI(_, __) =>
throw new UnimplementedError('Fallback UI will not be supported until support for ReactJS 16 is released in version 3.0.0');
throw new UnimplementedError('Fallback UI will not be supported until support for ReactJS 16 lifecycle methods is released in version 3.1.0');

@mustCallSuper
@override
Expand Down
1 change: 1 addition & 0 deletions lib/src/component_declaration/component_type_checking.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ void setComponentTypeMeta(ReactDartComponentFactoryProxy factory, {
bool isWrapper,
ReactDartComponentFactoryProxy parentType
}) {
// ignore: argument_type_not_assignable
setProperty(factory.type, _componentTypeMetaKey, new ComponentTypeMeta(isWrapper, parentType));
}

Expand Down
53 changes: 53 additions & 0 deletions test/over_react/component/error_boundary_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
@Timeout(const Duration(seconds: 2))
library error_boundary_test;

import 'dart:html';
import 'package:over_react/over_react.dart';
import 'package:over_react_test/over_react_test.dart';
import 'package:test/test.dart';

import './fixtures/flawed_component.dart';

void main() {
group('ErrorBoundary', () {
TestJacket<ErrorBoundaryComponent> jacket;
Expand All @@ -34,6 +37,56 @@ void main() {
});

// TODO: Add tests that exercise the actual ReactJS 16 error lifecycle methods once implemented.
group('catches component errors', () {
List<String> calls;
DivElement mountNode;

void verifyReact16ErrorHandlingWithoutErrorBoundary() {
mountNode = new DivElement();
document.body.append(mountNode);
var jacketOfFlawedComponentWithNoErrorBoundary = mount(Flawed()(), mountNode: mountNode);
expect(mountNode.children, isNotEmpty, reason: 'test setup sanity check');
jacketOfFlawedComponentWithNoErrorBoundary.getNode().click();
expect(mountNode.children, isEmpty,
reason: 'rendered trees not wrapped in an ErrorBoundary '
'should get unmounted when an error is thrown within child component lifecycle methods');

mountNode.remove();
mountNode = new DivElement();
document.body.append(mountNode);
}

setUp(() {
// Verify the behavior of a component that throws when it is not wrapped in an error boundary first
verifyReact16ErrorHandlingWithoutErrorBoundary();

calls = [];
jacket = mount(
(ErrorBoundary()
..onComponentDidCatch = (_, __) { calls.add('onComponentDidCatch'); }
)(Flawed()()),
mountNode: mountNode,
);
expect(mountNode.children, isNotEmpty, reason: 'test setup sanity check');
// Cause an error to be thrown within a ReactJS lifecycle method
jacket.getNode().click();
});

tearDown(() {
mountNode.remove();
mountNode = null;
});

test('and calls `props.onComponentDidCatch`', () {
expect(calls, ['onComponentDidCatch']);
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
});

test('and re-renders the tree as a result', () {
expect(mountNode.children, isNotEmpty,
reason: 'rendered trees wrapped in an ErrorBoundary '
'should NOT get unmounted when an error is thrown within child component lifecycle methods');
});
});

test('initializes with the expected default prop values', () {
jacket = mount(ErrorBoundary()(dummyChild));
Expand Down
54 changes: 54 additions & 0 deletions test/over_react/component/fixtures/flawed_component.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import 'package:over_react/over_react.dart';

// ignore: uri_has_not_been_generated
part 'flawed_component.over_react.g.dart';

@Factory()
// ignore: undefined_identifier
UiFactory<FlawedProps> Flawed = _$Flawed;

@Props()
class _$FlawedProps extends UiProps {}

// AF-3369 This will be removed once the transition to Dart 2 is complete.
// ignore: mixin_of_non_class, undefined_class
class FlawedProps extends _$FlawedProps with _$FlawedPropsAccessorsMixin {
// ignore: undefined_identifier, undefined_class, const_initialized_with_non_constant_value
static const PropsMeta meta = _$metaForFlawedProps;
}

@State()
class _$FlawedState extends UiState {
bool hasError;
}

// AF-3369 This will be removed once the transition to Dart 2 is complete.
// ignore: mixin_of_non_class, undefined_class
class FlawedState extends _$FlawedState with _$FlawedStateAccessorsMixin {
// ignore: undefined_identifier, undefined_class, const_initialized_with_non_constant_value
static const StateMeta meta = _$metaForFlawedState;
}

@Component()
class FlawedComponent extends UiStatefulComponent<FlawedProps, FlawedState> {
@override
Map getInitialState() => (newState()..hasError = false);

@override
void componentWillUpdate(_, Map nextState) {
final tNextState = typedStateFactory(nextState);
if (tNextState.hasError && !state.hasError) {
throw new Error();
}
}

@override
render() {
return (Dom.button()
..addTestId('flawedButton')
..onClick = (_) {
setState(newState()..hasError = true);
}
)('oh hai');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class TestCompositeComponentComponent extends UiComponent<TestCompositeComponent

@override
render() {
return Dom.div()('oh hai');
return Dom.div()('oh hai', props.children);
}
}

Expand Down
5 changes: 5 additions & 0 deletions web/demos/demos.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import 'package:react/react_client.dart';
import 'package:over_react/over_react.dart';
import '../src/demo_components.dart';

// ignore: uri_has_not_been_generated
part 'demos.over_react.g.dart';

// Parts
part 'button/button-examples.dart';
part 'button/button-types.dart';
Expand All @@ -14,6 +17,8 @@ part 'button/button-block.dart';
part 'button/button-active.dart';
part 'button/button-disabled.dart';

part 'faulty-component.dart';

part 'list-group/list-group-basic.dart';
part 'list-group/list-group-tags.dart';
part 'list-group/list-group-anchors-and-buttons.dart';
Expand Down
50 changes: 50 additions & 0 deletions web/demos/faulty-component.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
part of over_react.web.demos;

@Factory()
// ignore: undefined_identifier
UiFactory<FaultyProps> Faulty = _$Faulty;

@Props()
class _$FaultyProps extends UiProps {}

// AF-3369 This will be removed once the transition to Dart 2 is complete.
// ignore: mixin_of_non_class, undefined_class
class FaultyProps extends _$FaultyProps with _$FaultyPropsAccessorsMixin {
// ignore: undefined_identifier, undefined_class, const_initialized_with_non_constant_value
static const PropsMeta meta = _$metaForFaultyProps;
}

@State()
class _$FaultyState extends UiState {
bool hasErrored;
}

// AF-3369 This will be removed once the transition to Dart 2 is complete.
// ignore: mixin_of_non_class, undefined_class
class FaultyState extends _$FaultyState with _$FaultyStateAccessorsMixin {
// ignore: undefined_identifier, undefined_class, const_initialized_with_non_constant_value
static const StateMeta meta = _$metaForFaultyState;
}

@Component()
class FaultyComponent extends UiStatefulComponent<FaultyProps, FaultyState> {
@override
Map getInitialState() => (newState()..hasErrored = false);

@override
void componentWillUpdate(_, Map nextState) {
final tNextState = typedStateFactory(nextState);
if (tNextState.hasErrored && !state.hasErrored) {
throw new Error();
}
}

@override
render() {
return (Dom.div()..className = 'btn-toolbar')(
(Button()..onClick = (_) {
setState(newState()..hasErrored = true);
})('Click me to throw an error'),
);
}
}
25 changes: 19 additions & 6 deletions web/index.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'dart:html';

import 'package:over_react/over_react.dart';
import 'package:over_react/react_dom.dart' as react_dom;
import 'package:react/react_client.dart' show setClientConfiguration;
import './demos/demos.dart';
Expand All @@ -9,20 +10,32 @@ void main() {
setClientConfiguration();

react_dom.render(
buttonExamplesDemo(), querySelector('$demoMountNodeSelectorPrefix--button'));
ErrorBoundary()(buttonExamplesDemo()), querySelector('$demoMountNodeSelectorPrefix--button'));

react_dom.render(
listGroupBasicDemo(), querySelector('$demoMountNodeSelectorPrefix--list-group'));
ErrorBoundary()(listGroupBasicDemo()), querySelector('$demoMountNodeSelectorPrefix--list-group'));

react_dom.render(
progressBasicDemo(), querySelector('$demoMountNodeSelectorPrefix--progress'));
ErrorBoundary()(progressBasicDemo()), querySelector('$demoMountNodeSelectorPrefix--progress'));

react_dom.render(
tagBasicDemo(), querySelector('$demoMountNodeSelectorPrefix--tag'));
ErrorBoundary()(tagBasicDemo()), querySelector('$demoMountNodeSelectorPrefix--tag'));

react_dom.render(
checkboxToggleButtonDemo(), querySelector('$demoMountNodeSelectorPrefix--checkbox-toggle'));
ErrorBoundary()(checkboxToggleButtonDemo()), querySelector('$demoMountNodeSelectorPrefix--checkbox-toggle'));

react_dom.render(
radioToggleButtonDemo(), querySelector('$demoMountNodeSelectorPrefix--radio-toggle'));
ErrorBoundary()(radioToggleButtonDemo()), querySelector('$demoMountNodeSelectorPrefix--radio-toggle'));

react_dom.render(
(ErrorBoundary()
..onComponentDidCatch = (error, info) {
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
print('Consumer props.onComponentDidCatch($error, $info)');
}
)(Faulty()()),
querySelector('$demoMountNodeSelectorPrefix--faulty-component'),
);

react_dom.render(
Faulty()(), querySelector('$demoMountNodeSelectorPrefix--faulty-component-without-error-boundary'));
}
12 changes: 12 additions & 0 deletions web/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,18 @@ <h2>ToggleButton (Radio)</h2>
around with the component rendered below.</p>
<div class="component-demo__mount--radio-toggle"></div>
</div>
<div class="col-xs-12 col-lg-6 p-3">
<h2>Component That Throws A Caught Error</h2>
<p>Modify the source of <code>/web/demos/faulty-component.dart</code> to play
around with the component rendered below.</p>
<div class="component-demo__mount--faulty-component"></div>
</div>
<div class="col-xs-12 col-lg-6 p-3">
<h2>Component That Throws an Uncaught Error</h2>
<p>Modify the source of <code>/web/demos/faulty-component.dart</code> to play
around with the component rendered below.</p>
<div class="component-demo__mount--faulty-component-without-error-boundary"></div>
</div>
</div>
</div>

Expand Down