From 32a5cb0b1e8052fb9548f1e071030259b822260f Mon Sep 17 00:00:00 2001
From: Aaron Lademann
Date: Thu, 2 May 2019 09:14:43 -0700
Subject: [PATCH 1/5] Add temporary JS error boundary
---
lib/src/component/error_boundary.dart | 76 +++++++++++++++++--
.../component/error_boundary_test.dart | 53 +++++++++++++
.../component/fixtures/flawed_component.dart | 54 +++++++++++++
.../fixtures/dummy_composite_component.dart | 2 +-
web/demos/demos.dart | 5 ++
web/demos/faulty-component.dart | 50 ++++++++++++
web/index.dart | 25 ++++--
web/index.html | 12 +++
8 files changed, 264 insertions(+), 13 deletions(-)
create mode 100644 test/over_react/component/fixtures/flawed_component.dart
create mode 100644 web/demos/faulty-component.dart
diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart
index 0255d603e..d9762f7a9 100644
--- a/lib/src/component/error_boundary.dart
+++ b/lib/src/component/error_boundary.dart
@@ -1,14 +1,71 @@
+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);
+ }),
+ }));
+
+ // 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.
+ 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?
+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.
@@ -93,12 +150,19 @@ class ErrorBoundaryComponent 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
diff --git a/test/over_react/component/error_boundary_test.dart b/test/over_react/component/error_boundary_test.dart
index 52fd97014..fedcd6a9c 100644
--- a/test/over_react/component/error_boundary_test.dart
+++ b/test/over_react/component/error_boundary_test.dart
@@ -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 jacket;
@@ -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 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']);
+ });
+
+ 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));
diff --git a/test/over_react/component/fixtures/flawed_component.dart b/test/over_react/component/fixtures/flawed_component.dart
new file mode 100644
index 000000000..1bf708675
--- /dev/null
+++ b/test/over_react/component/fixtures/flawed_component.dart
@@ -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 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 {
+ @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');
+ }
+}
diff --git a/test/over_react/dom/fixtures/dummy_composite_component.dart b/test/over_react/dom/fixtures/dummy_composite_component.dart
index 89044a45b..ccb1af41a 100644
--- a/test/over_react/dom/fixtures/dummy_composite_component.dart
+++ b/test/over_react/dom/fixtures/dummy_composite_component.dart
@@ -55,7 +55,7 @@ class TestCompositeComponentComponent extends UiComponent 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 {
+ @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'),
+ );
+ }
+}
diff --git a/web/index.dart b/web/index.dart
index 8cede48ab..ee04c23f0 100644
--- a/web/index.dart
+++ b/web/index.dart
@@ -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';
@@ -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) {
+ print('Consumer props.onComponentDidCatch($error, $info)');
+ }
+ )(Faulty()()),
+ querySelector('$demoMountNodeSelectorPrefix--faulty-component'),
+ );
+
+ react_dom.render(
+ Faulty()(), querySelector('$demoMountNodeSelectorPrefix--faulty-component-without-error-boundary'));
}
diff --git a/web/index.html b/web/index.html
index f09448696..ccf03ce71 100644
--- a/web/index.html
+++ b/web/index.html
@@ -55,6 +55,18 @@ ToggleButton (Radio)
around with the component rendered below.
+
+
Component That Throws A Caught Error
+
Modify the source of /web/demos/faulty-component.dart
to play
+ around with the component rendered below.
+
+
+
+
Component That Throws an Uncaught Error
+
Modify the source of /web/demos/faulty-component.dart
to play
+ around with the component rendered below.
+
+
From 3eceba34f009abfe77aff3234ad463b6d2b322ed Mon Sep 17 00:00:00 2001
From: Aaron Lademann
Date: Fri, 3 May 2019 15:29:06 -0700
Subject: [PATCH 2/5] Fix strange analyzer errors that only show in CI
+ Do not show in IDE analyzer view
---
lib/src/component/error_boundary.dart | 1 +
lib/src/component_declaration/component_type_checking.dart | 1 +
2 files changed, 2 insertions(+)
diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart
index d9762f7a9..e88ec21e5 100644
--- a/lib/src/component/error_boundary.dart
+++ b/lib/src/component/error_boundary.dart
@@ -50,6 +50,7 @@ final ReactElement Function([Map props, List children]) _jsErrorBoundaryComponen
// 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);
diff --git a/lib/src/component_declaration/component_type_checking.dart b/lib/src/component_declaration/component_type_checking.dart
index 44ad6cb2c..c962d6288 100644
--- a/lib/src/component_declaration/component_type_checking.dart
+++ b/lib/src/component_declaration/component_type_checking.dart
@@ -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));
}
From f80eb61d627fb69ee57bbf634aa5763c2489919c Mon Sep 17 00:00:00 2001
From: Aaron Lademann
Date: Mon, 6 May 2019 15:49:42 -0700
Subject: [PATCH 3/5] Dartify the error thrown from componentDidCatch
---
lib/src/component/error_boundary.dart | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart
index e88ec21e5..3ec70d028 100644
--- a/lib/src/component/error_boundary.dart
+++ b/lib/src/component/error_boundary.dart
@@ -6,7 +6,7 @@ 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;
+import 'package:react/react_client/react_interop.dart' show React, ReactClassConfig, throwErrorFromJS;
// ignore: uri_has_not_been_generated
part 'error_boundary.over_react.g.dart';
@@ -41,8 +41,15 @@ final ReactElement Function([Map props, List children]) _jsErrorBoundaryComponen
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);
+ // Due to the error object being passed in from ReactJS it is a javascript object that does not get dartified.
+ // To fix this we throw the error again from Dart to the JS side and catch it Dart side which re-dartifies it.
+ try {
+ throwErrorFromJS(error);
+ } catch (e, stack) {
+ // The Dart stack track gets lost so we manually add it to the info object for reference.
+ info['dartStackTrace'] = stack;
+ js_util.getProperty(js_util.getProperty(jsThis, 'props'), 'onComponentDidCatch')(error, info);
+ }
}),
}));
From 90b1a398c2b1ee3c36d3fe6bec69bbb11af6e19e Mon Sep 17 00:00:00 2001
From: Aaron Lademann
Date: Mon, 6 May 2019 16:06:09 -0700
Subject: [PATCH 4/5] Address CR feedback
---
lib/src/component/error_boundary.dart | 9 +++------
.../over_react/component/error_boundary_test.dart | 15 ++++++++++++---
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart
index 3ec70d028..b4f7d0d3e 100644
--- a/lib/src/component/error_boundary.dart
+++ b/lib/src/component/error_boundary.dart
@@ -41,14 +41,13 @@ final ReactElement Function([Map props, List children]) _jsErrorBoundaryComponen
return js_util.getProperty(jsProps, 'children');
}),
'componentDidCatch': allowInteropCaptureThis((jsThis, error, info) {
+ final jsProps = js_util.getProperty(jsThis, 'props');
// Due to the error object being passed in from ReactJS it is a javascript object that does not get dartified.
// To fix this we throw the error again from Dart to the JS side and catch it Dart side which re-dartifies it.
try {
throwErrorFromJS(error);
- } catch (e, stack) {
- // The Dart stack track gets lost so we manually add it to the info object for reference.
- info['dartStackTrace'] = stack;
- js_util.getProperty(js_util.getProperty(jsThis, 'props'), 'onComponentDidCatch')(error, info);
+ } catch (error, stack) {
+ js_util.getProperty(jsProps, 'onComponentDidCatch')(error, info);
}
}),
}));
@@ -68,11 +67,9 @@ final ReactElement Function([Map props, List children]) _jsErrorBoundaryComponen
})();
// TODO: Need to type the second argument once react-dart implements bindings for the ReactJS "componentStack".
-// TODO: Will it be possible to make the first argument typed as a real `Error` instance again in Dart 2?
typedef _ComponentDidCatchCallback(/*Error*/dynamic error, /*ComponentStack*/dynamic componentStack);
// TODO: Need to type the second argument once react-dart implements bindings for the ReactJS "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
diff --git a/test/over_react/component/error_boundary_test.dart b/test/over_react/component/error_boundary_test.dart
index fedcd6a9c..f5bf22b60 100644
--- a/test/over_react/component/error_boundary_test.dart
+++ b/test/over_react/component/error_boundary_test.dart
@@ -16,6 +16,7 @@
library error_boundary_test;
import 'dart:html';
+import 'dart:js';
import 'package:over_react/over_react.dart';
import 'package:over_react_test/over_react_test.dart';
import 'package:test/test.dart';
@@ -38,7 +39,7 @@ void main() {
// TODO: Add tests that exercise the actual ReactJS 16 error lifecycle methods once implemented.
group('catches component errors', () {
- List calls;
+ List