Merge pull request #344 from Workiva/3.0.0-wip-dart1-rebased

RM-58638 Release over_react 3.0.0-alpha.0+dart1 (Includes CPLAT-7548)

Author: rmconsole2-wf

lib/src/component/dom_components.dart

@@ -50,7 +50,7 @@ class DomProps extends component_base.UiProps
   DomProps(this.componentFactory, [Map props]) : this.props = props ?? ({});
 
   @override
-  final ReactDomComponentFactoryProxy componentFactory;
+  ReactComponentFactoryProxy componentFactory;
 
   @override
   final Map props;
@@ -76,7 +76,7 @@ class SvgProps extends component_base.UiProps
   SvgProps(this.componentFactory, [Map props]) : this.props = props ?? ({});
 
   @override
-  final ReactDomComponentFactoryProxy componentFactory;
+  ReactComponentFactoryProxy componentFactory;
 
   @override
   final Map props;

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

pubspec.yaml

@@ -1,5 +1,5 @@
 name: over_react
-version: 2.5.0+dart1
+version: 3.0.0-alpha.0+dart1
 description: A library for building statically-typed React UI components using Dart.
 homepage: https://github.com/Workiva/over_react/
 authors:
@@ -16,11 +16,11 @@ dependencies:
   logging: ">=0.11.3+2 <1.0.0"
   meta: ^1.1.6
   path: ^1.5.1
-  react: ^4.9.0
+  react: ^5.0.0-alpha
   source_span: ^1.4.1
   transformer_utils: ^0.1.5
   w_common: ^1.13.0
-  w_flux: ^2.9.5
+  w_flux: ^2.10.4
   platform_detect: ^1.3.4
   quiver: ">=0.25.0 <=0.28.0" # 0.28.0+ is Dart 2 only
 dev_dependencies:
@@ -31,7 +31,7 @@ dev_dependencies:
   dart_dev: ^1.9.6
   dependency_validator: ^1.2.2
   mockito: ^2.2.2
-  over_react_test: '>=1.6.0 <3.0.0'
+  over_react_test: ^2.5.2
   source_gen: ^0.7.4+3
   test: ^0.12.34
 

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,70 @@ 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];
+        expect(errArg.toString(), contains('FlawedComponentException: I was thrown from inside 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,59 @@
+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 FlawedComponentException();
+    }
+  }
+
+  @override
+  render() {
+    return (Dom.button()
+      ..addTestId('flawedButton')
+      ..onClick = (_) {
+        setState(newState()..hasError = true);
+      }
+    )('oh hai');
+  }
+}
+
+class FlawedComponentException implements Exception {
+  @override
+  String toString() => 'FlawedComponentException: I was thrown from inside FlawedComponent.componentWillUpdate!';
+}

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

test/over_react/dom/render_test.dart

@@ -92,11 +92,6 @@ main() {
     });
 
     group('throws', () {
-      test('when `element` is `null`', () {
-        expect(() => react_dom.render(null, mountNode), throwsA(anything));
-        expect(mountNode.children, isEmpty);
-      });
-
       test('when `mountNode` is `null`', () {
         expect(() => react_dom.render(Dom.div()('oh hai'), null), throwsA(anything));
       });

test/over_react/util/react_wrappers_test.dart

@@ -984,8 +984,8 @@ main() {
         });
 
         test('a String', () {
-          expect(() => getProps('string'), throwsArgumentError);
-        }, testOn: 'js');
+          expect(() => getProps('string'), throwsA(anything));
+        });
 
         test('null', () {
           expect(() => getProps(null), throwsArgumentError);
@@ -1019,6 +1019,7 @@ main() {
             ]));
           });
 
+          // TODO: 3.0.0 this is failing on Dart 2 dart2js tests only.
           test('a JS composite component', () {
             var calls = [];
 
@@ -1040,6 +1041,7 @@ main() {
             ]));
           });
 
+          // TODO: 3.0.0 this is failing on Dart 2 dart2js tests only.
           test('a DOM component', () {
             var calls = [];
 

web/index.dart

@@ -1,28 +1,42 @@
 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';
 import './demos/constants.dart';
+import 'src/demos/faulty-component.dart';
 
 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'));
 }