Merge pull request #291 from Workiva/3.0.0/CPLAT-5599_js-error-boundary

CPLAT-5599 Add temporary JS error boundary

lib/src/component/error_boundary.dart

@@ -1,14 +1,80 @@
+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, throwErrorFromJS;
 
 // 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');
+      // 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 (error, stack) {
+        final callback = js_util.getProperty(jsProps, 'onComponentDidCatch');
+
+        if (callback != null) {
+          callback(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.
+  // 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);
+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);
+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 +159,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

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));
 }
 

test/over_react/component/error_boundary_test.dart

@@ -15,10 +15,14 @@
 @Timeout(const Duration(seconds: 2))
 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';
 
+import './fixtures/flawed_component.dart';
+
 void main() {
   group('ErrorBoundary', () {
     TestJacket<ErrorBoundaryComponent> jacket;
@@ -34,6 +38,72 @@ void main() {
     });
 
     // TODO: Add tests that exercise the actual ReactJS 16 error lifecycle methods once implemented.
+    group('catches component errors', () {
+      List<Map<String, 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 = (err, info) {
+              calls.add({'onComponentDidCatch': [err, info]});
+            }
+          )(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.single.keys, ['onComponentDidCatch']);
+        final errArg = calls.single['onComponentDidCatch'][0];
+        // TODO: Figure out why a `const isInstanceOf<Error>()` doesn't work.
+        expect(errArg.toString(), contains('Instance of \'Error\''));
+        expect(errArg.toString(), contains('FlawedComponent.componentWillUpdate'));
+
+        final infoArg = calls.single['onComponentDidCatch'][1];
+        expect(infoArg, isNotNull);
+      });
+
+      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('does not throw a null exception when `props.onComponentDidCatch` is not set', () {
+        jacket = mount(ErrorBoundary()((Flawed()..addTestId('flawed'))()), mountNode: mountNode);
+        // The click causes the componentDidCatch lifecycle method to execute
+        // and we want to ensure that no Dart null error is thrown as a result of no consumer prop callback being set.
+        expect(() => jacket.getNode().click(), returnsNormally);
+      });
+    });
 
     test('initializes with the expected default prop values', () {
       jacket = mount(ErrorBoundary()(dummyChild));

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<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');
+  }
+}

test/over_react/dom/fixtures/dummy_composite_component.dart

@@ -55,7 +55,7 @@ class TestCompositeComponentComponent extends UiComponent<TestCompositeComponent
 
   @override
   render() {
-    return Dom.div()('oh hai');
+    return Dom.div()('oh hai', props.children);
   }
 }
 

web/demos/demos.dart

@@ -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';
@@ -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';

web/demos/faulty-component.dart

@@ -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'),
+    );
+  }
+}

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'));
 }