From a559b44f4a28ca80dc54dd3d15a205d862746172 Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Wed, 4 Dec 2019 12:48:03 -0700 Subject: [PATCH 01/13] inital pass --- lib/src/component/error_boundary.dart | 35 ++- .../component/error_boundary_recoverable.dart | 25 ++ ...ror_boundary_recoverable.over_react.g.dart | 260 ++++++++++++++++++ .../component/error_boundary_test.dart | 5 + 4 files changed, 324 insertions(+), 1 deletion(-) create mode 100644 lib/src/component/error_boundary_recoverable.dart create mode 100644 lib/src/component/error_boundary_recoverable.over_react.g.dart diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart index 3db97d23c..fb8feb3e9 100644 --- a/lib/src/component/error_boundary.dart +++ b/lib/src/component/error_boundary.dart @@ -1,4 +1,6 @@ +import 'package:meta/meta.dart'; import 'package:over_react/over_react.dart'; +import 'package:over_react/src/component/error_boundary_recoverable.dart'; part 'error_boundary.over_react.g.dart'; @@ -22,4 +24,35 @@ class _$ErrorBoundaryState extends UiState with ErrorBoundaryStateMixin {} @Component2(isWrapper: true, isErrorBoundary: true) class ErrorBoundaryComponent - extends UiStatefulComponent2 with ErrorBoundaryMixin {} + extends UiStatefulComponent2 with ErrorBoundaryMixin { + + @override + Map getDerivedStateFromError(error) { + return (newState() + ..hasError = true + ..showFallbackUIOnError = true + ); + } + + @override + void componentDidCatch(error, ReactErrorInfo info) { + if (props.onComponentDidCatch != null) { + props.onComponentDidCatch(error, info); + } + + if (props.onComponentIsUnrecoverable != null) { + props.onComponentIsUnrecoverable(error, info); + } + } + + @override + void componentDidUpdate(Map prevProps, Map prevState, [dynamic snapshot]) {} + + @override + render() { + if (state.hasError) { + return super.render(); + } + return (RecoverableErrorBoundary()..addProps(props))(props.children); + } +} diff --git a/lib/src/component/error_boundary_recoverable.dart b/lib/src/component/error_boundary_recoverable.dart new file mode 100644 index 000000000..474b1ed3d --- /dev/null +++ b/lib/src/component/error_boundary_recoverable.dart @@ -0,0 +1,25 @@ +import 'package:over_react/over_react.dart'; + +part 'error_boundary_recoverable.over_react.g.dart'; + +/// 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. +/// +/// Optionally, use the [ErrorBoundaryProps.onComponentDidCatch] +/// to send error / stack trace information to a logging endpoint for your application. +/// +/// To make your own custom error boundaries, you can utilize the [ErrorBoundaryPropsMixin], +/// [ErrorBoundaryStateMixin] and [ErrorBoundaryMixin]s on any component that is annotated +/// using `@Component2(isErrorBoundary: true)`. See the [ErrorBoundaryMixin] for an example implementation. +@Factory() +UiFactory RecoverableErrorBoundary = _$RecoverableErrorBoundary; + +@Props() +class _$RecoverableErrorBoundaryProps extends UiProps with ErrorBoundaryPropsMixin {} + +@State() +class _$RecoverableErrorBoundaryState extends UiState with ErrorBoundaryStateMixin {} + +@Component2(isWrapper: true, isErrorBoundary: true) +class RecoverableErrorBoundaryComponent + extends UiStatefulComponent2 with ErrorBoundaryMixin {} diff --git a/lib/src/component/error_boundary_recoverable.over_react.g.dart b/lib/src/component/error_boundary_recoverable.over_react.g.dart new file mode 100644 index 000000000..960b566e4 --- /dev/null +++ b/lib/src/component/error_boundary_recoverable.over_react.g.dart @@ -0,0 +1,260 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +// ignore_for_file: deprecated_member_use_from_same_package, unnecessary_null_in_if_null_operators, prefer_null_aware_operators +part of 'error_boundary_recoverable.dart'; + +// ************************************************************************** +// OverReactBuilder (package:over_react/src/builder.dart) +// ************************************************************************** + +// React component factory implementation. +// +// Registers component implementation and links type meta to builder factory. +final $RecoverableErrorBoundaryComponentFactory = registerComponent2( + () => _$RecoverableErrorBoundaryComponent(), + builderFactory: RecoverableErrorBoundary, + componentClass: RecoverableErrorBoundaryComponent, + isWrapper: true, + parentType: null, + displayName: 'RecoverableErrorBoundary', + skipMethods: const [], +); + +abstract class _$RecoverableErrorBoundaryPropsAccessorsMixin + implements _$RecoverableErrorBoundaryProps { + @override + Map get props; + + /* GENERATED CONSTANTS */ + + static const List $props = []; + static const List $propKeys = []; +} + +const PropsMeta _$metaForRecoverableErrorBoundaryProps = PropsMeta( + fields: _$RecoverableErrorBoundaryPropsAccessorsMixin.$props, + keys: _$RecoverableErrorBoundaryPropsAccessorsMixin.$propKeys, +); + +class RecoverableErrorBoundaryProps extends _$RecoverableErrorBoundaryProps + with _$RecoverableErrorBoundaryPropsAccessorsMixin { + static const PropsMeta meta = _$metaForRecoverableErrorBoundaryProps; +} + +_$$RecoverableErrorBoundaryProps _$RecoverableErrorBoundary( + [Map backingProps]) => + backingProps == null + ? _$$RecoverableErrorBoundaryProps$JsMap(JsBackedMap()) + : _$$RecoverableErrorBoundaryProps(backingProps); + +// Concrete props implementation. +// +// Implements constructor and backing map, and links up to generated component factory. +abstract class _$$RecoverableErrorBoundaryProps + extends _$RecoverableErrorBoundaryProps + with _$RecoverableErrorBoundaryPropsAccessorsMixin + implements RecoverableErrorBoundaryProps { + _$$RecoverableErrorBoundaryProps._(); + + factory _$$RecoverableErrorBoundaryProps(Map backingMap) { + if (backingMap == null || backingMap is JsBackedMap) { + return _$$RecoverableErrorBoundaryProps$JsMap(backingMap); + } else { + return _$$RecoverableErrorBoundaryProps$PlainMap(backingMap); + } + } + + /// Let `UiProps` internals know that this class has been generated. + @override + bool get $isClassGenerated => true; + + /// The `ReactComponentFactory` associated with the component built by this class. + @override + ReactComponentFactoryProxy get componentFactory => + super.componentFactory ?? $RecoverableErrorBoundaryComponentFactory; + + /// The default namespace for the prop getters/setters generated for this class. + @override + String get propKeyNamespace => 'RecoverableErrorBoundaryProps.'; +} + +// Concrete props implementation that can be backed by any [Map]. +class _$$RecoverableErrorBoundaryProps$PlainMap + extends _$$RecoverableErrorBoundaryProps { + // This initializer of `_props` to an empty map, as well as the reassignment + // of `_props` in the constructor body is necessary to work around a DDC bug: https://github.com/dart-lang/sdk/issues/36217 + _$$RecoverableErrorBoundaryProps$PlainMap(Map backingMap) + : this._props = {}, + super._() { + this._props = backingMap ?? {}; + } + + /// The backing props map proxied by this class. + @override + Map get props => _props; + Map _props; +} + +// Concrete props implementation that can only be backed by [JsMap], +// allowing dart2js to compile more optimal code for key-value pair reads/writes. +class _$$RecoverableErrorBoundaryProps$JsMap + extends _$$RecoverableErrorBoundaryProps { + // This initializer of `_props` to an empty map, as well as the reassignment + // of `_props` in the constructor body is necessary to work around a DDC bug: https://github.com/dart-lang/sdk/issues/36217 + _$$RecoverableErrorBoundaryProps$JsMap(JsBackedMap backingMap) + : this._props = JsBackedMap(), + super._() { + this._props = backingMap ?? JsBackedMap(); + } + + /// The backing props map proxied by this class. + @override + JsBackedMap get props => _props; + JsBackedMap _props; +} + +abstract class _$RecoverableErrorBoundaryStateAccessorsMixin + implements _$RecoverableErrorBoundaryState { + @override + Map get state; + + /* GENERATED CONSTANTS */ + + static const List $state = []; + static const List $stateKeys = []; +} + +const StateMeta _$metaForRecoverableErrorBoundaryState = StateMeta( + fields: _$RecoverableErrorBoundaryStateAccessorsMixin.$state, + keys: _$RecoverableErrorBoundaryStateAccessorsMixin.$stateKeys, +); + +class RecoverableErrorBoundaryState extends _$RecoverableErrorBoundaryState + with _$RecoverableErrorBoundaryStateAccessorsMixin { + static const StateMeta meta = _$metaForRecoverableErrorBoundaryState; +} + +// Concrete state implementation. +// +// Implements constructor and backing map. +abstract class _$$RecoverableErrorBoundaryState + extends _$RecoverableErrorBoundaryState + with _$RecoverableErrorBoundaryStateAccessorsMixin + implements RecoverableErrorBoundaryState { + _$$RecoverableErrorBoundaryState._(); + + factory _$$RecoverableErrorBoundaryState(Map backingMap) { + if (backingMap == null || backingMap is JsBackedMap) { + return _$$RecoverableErrorBoundaryState$JsMap(backingMap); + } else { + return _$$RecoverableErrorBoundaryState$PlainMap(backingMap); + } + } + + /// Let `UiState` internals know that this class has been generated. + @override + bool get $isClassGenerated => true; +} + +// Concrete state implementation that can be backed by any [Map]. +class _$$RecoverableErrorBoundaryState$PlainMap + extends _$$RecoverableErrorBoundaryState { + // This initializer of `_state` to an empty map, as well as the reassignment + // of `_state` in the constructor body is necessary to work around a DDC bug: https://github.com/dart-lang/sdk/issues/36217 + _$$RecoverableErrorBoundaryState$PlainMap(Map backingMap) + : this._state = {}, + super._() { + this._state = backingMap ?? {}; + } + + /// The backing state map proxied by this class. + @override + Map get state => _state; + Map _state; +} + +// Concrete state implementation that can only be backed by [JsMap], +// allowing dart2js to compile more optimal code for key-value pair reads/writes. +class _$$RecoverableErrorBoundaryState$JsMap + extends _$$RecoverableErrorBoundaryState { + // This initializer of `_state` to an empty map, as well as the reassignment + // of `_state` in the constructor body is necessary to work around a DDC bug: https://github.com/dart-lang/sdk/issues/36217 + _$$RecoverableErrorBoundaryState$JsMap(JsBackedMap backingMap) + : this._state = JsBackedMap(), + super._() { + this._state = backingMap ?? JsBackedMap(); + } + + /// The backing state map proxied by this class. + @override + JsBackedMap get state => _state; + JsBackedMap _state; +} + +// Concrete component implementation mixin. +// +// Implements typed props/state factories, defaults `consumedPropKeys` to the keys +// generated for the associated props class. +class _$RecoverableErrorBoundaryComponent + extends RecoverableErrorBoundaryComponent { + _$$RecoverableErrorBoundaryProps$JsMap _cachedTypedProps; + + @override + _$$RecoverableErrorBoundaryProps$JsMap get props => _cachedTypedProps; + + @override + set props(Map value) { + assert( + getBackingMap(value) is JsBackedMap, + 'Component2.props should never be set directly in ' + 'production. If this is required for testing, the ' + 'component should be rendered within the test. If ' + 'that does not have the necessary result, the last ' + 'resort is to use typedPropsFactoryJs.'); + super.props = value; + _cachedTypedProps = typedPropsFactoryJs(getBackingMap(value)); + } + + @override + _$$RecoverableErrorBoundaryProps$JsMap typedPropsFactoryJs( + JsBackedMap backingMap) => + _$$RecoverableErrorBoundaryProps$JsMap(backingMap); + + @override + _$$RecoverableErrorBoundaryProps typedPropsFactory(Map backingMap) => + _$$RecoverableErrorBoundaryProps(backingMap); + + _$$RecoverableErrorBoundaryState$JsMap _cachedTypedState; + @override + _$$RecoverableErrorBoundaryState$JsMap get state => _cachedTypedState; + + @override + set state(Map value) { + assert( + value is JsBackedMap, + 'Component2.state should only be set via ' + 'initialState or setState.'); + super.state = value; + _cachedTypedState = typedStateFactoryJs(value); + } + + @override + _$$RecoverableErrorBoundaryState$JsMap typedStateFactoryJs( + JsBackedMap backingMap) => + _$$RecoverableErrorBoundaryState$JsMap(backingMap); + + @override + _$$RecoverableErrorBoundaryState typedStateFactory(Map backingMap) => + _$$RecoverableErrorBoundaryState(backingMap); + + /// Let `UiComponent` internals know that this class has been generated. + @override + bool get $isClassGenerated => true; + + /// The default consumed props, taken from _$RecoverableErrorBoundaryProps. + /// Used in `ConsumedProps` if [consumedProps] is not overridden. + @override + final List $defaultConsumedProps = const [ + _$metaForRecoverableErrorBoundaryProps + ]; +} diff --git a/test/over_react/component/error_boundary_test.dart b/test/over_react/component/error_boundary_test.dart index 66dbd5187..ef5bb9e2c 100644 --- a/test/over_react/component/error_boundary_test.dart +++ b/test/over_react/component/error_boundary_test.dart @@ -17,11 +17,16 @@ library error_boundary_test; import 'package:over_react/over_react.dart'; +import 'package:over_react/src/component/error_boundary_recoverable.dart'; import 'package:test/test.dart'; import 'shared_error_boundary_tests.dart'; void main() { + group('RecoverableErrorBoundary', () { + sharedErrorBoundaryTests(() => RecoverableErrorBoundary()); + }); + group('ErrorBoundary', () { sharedErrorBoundaryTests(() => ErrorBoundary()); }); From b80c85fdbf2ca07c5f2e71347ee05203b45cbda0 Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Wed, 11 Dec 2019 08:42:25 -0700 Subject: [PATCH 02/13] fixing up errorboundary tests --- lib/src/component/error_boundary.dart | 109 +++++++++++++++++- .../component/error_boundary_test.dart | 8 +- .../shared_error_boundary_tests.dart | 83 ++++++++----- test/over_react_component_test.dart | 22 ++-- 4 files changed, 172 insertions(+), 50 deletions(-) diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart index fb8feb3e9..ad0c230ed 100644 --- a/lib/src/component/error_boundary.dart +++ b/lib/src/component/error_boundary.dart @@ -1,4 +1,4 @@ -import 'package:meta/meta.dart'; +import 'package:logging/logging.dart'; import 'package:over_react/over_react.dart'; import 'package:over_react/src/component/error_boundary_recoverable.dart'; @@ -24,10 +24,60 @@ class _$ErrorBoundaryState extends UiState with ErrorBoundaryStateMixin {} @Component2(isWrapper: true, isErrorBoundary: true) class ErrorBoundaryComponent - extends UiStatefulComponent2 with ErrorBoundaryMixin { + extends UiStatefulComponent2 { + + // ---------------------------------------------- \/ ---------------------------------------------- + // How This ErrorBoundary Works: + // + // Background Info: + // React gives each Error Boundary one chance to handle or recover when an error is throws, if + // the Error Boundary's children throw again on its next render after React calls + // `getDerivedStateFromError` and `componentDidCatch`, React will ascend the tree to the next + // Error Boundary above it and rerender offering it a chance to handle the error. If none of + // the parent Error Boundaries have a successful render cycle, React unmounts the entire tree. + // __Note:__ When an Error Boundary remounts its children React clears all child components + // previous state, including child ErrorBoundaries meaning they lose all previous knowledge + // of any errors thrown. + // + // Solution: + // To prevent unmounting the entire tree when React cannot find an Error Boundary that is able + // to handle the error we wrap an Error Boundary with another Error Boundary (this one!). The + // child Error Boundary will handle errors that are "recoverable", so if an error gets to this + // Error Boundary we know it is "unrecoverable" and can present a fallback. + // + // ----------------------------------------------------------------------------------------------- + // Implementation: + // + // [1] Renders a child Error Boundary that is able to handle Errors thrown outside of the initial + // render cycle, allowing it a chance to "recover". + // + // [2] If we catch an error in this Error Boundary that indicates that the child Error Boundary was + // unable to handle or recover from the error, so we know that it was "unrecoverable" and we + // haven't had a successful render. + // + // [2.1] If we have `props.fallbackUIRenderer` we present that. + // + // [2.2] Present nothing instead of the broken children. + // ---------------------------------------------- /\ ---------------------------------------------- + + ReactErrorInfo _errorInfo; + String _error; + + @override + Map get defaultProps => (newProps() + ..identicalErrorFrequencyTolerance = Duration(seconds: 5) + ..loggerName = defaultErrorBoundaryLoggerName + ..shouldLogErrors = true + ); + + @override + get initialState => newState() + ..hasError = false + ..showFallbackUIOnError = true; @override Map getDerivedStateFromError(error) { + _error = error; return (newState() ..hasError = true ..showFallbackUIOnError = true @@ -36,23 +86,70 @@ class ErrorBoundaryComponent RecoverableErrorBoundary()); - }); + // group('RecoverableErrorBoundary', () { + // sharedErrorBoundaryTests(() => RecoverableErrorBoundary()); + // }); group('ErrorBoundary', () { - sharedErrorBoundaryTests(() => ErrorBoundary()); + sharedErrorBoundaryTests(() => ErrorBoundary(), isWrapper: true); }); } diff --git a/test/over_react/component/shared_error_boundary_tests.dart b/test/over_react/component/shared_error_boundary_tests.dart index 60088cd41..107d8a4b3 100644 --- a/test/over_react/component/shared_error_boundary_tests.dart +++ b/test/over_react/component/shared_error_boundary_tests.dart @@ -1,5 +1,6 @@ import 'dart:html'; import 'package:logging/logging.dart'; +import 'package:meta/meta.dart'; import 'package:over_react/over_react.dart'; import 'package:over_react_test/over_react_test.dart'; import 'package:test/test.dart'; @@ -9,8 +10,10 @@ import './fixtures/flawed_component_on_mount.dart'; import './fixtures/flawed_component_that_renders_a_string.dart'; import './fixtures/flawed_component_that_renders_nothing.dart'; -void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { - TestJacket jacket; +/// `isWrapper` identifies an ErrorBoundary that wraps another Error Boundary in order to handle +/// render cycle "unrecoverable" errors. +void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder, { bool isWrapper = false }) { + TestJacket> jacket; ReactElement dummyChild; setUp(() { @@ -26,7 +29,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { List> calls; DivElement mountNode; - void verifyReact16ErrorHandlingWithoutErrorBoundary() { + void verifyReact16ErrorHandlingWithoutbuilder() { mountNode = DivElement(); document.body.append(mountNode); var jacketOfFlawedComponentWithNoErrorBoundary = mount(Flawed()(), mountNode: mountNode); @@ -47,7 +50,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { setUp(() { // Verify the behavior of a component that throws when it is not wrapped in an error boundary first - verifyReact16ErrorHandlingWithoutErrorBoundary(); + verifyReact16ErrorHandlingWithoutbuilder(); calls = []; jacket = mount( @@ -60,6 +63,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { ); expect(mountNode.children, isNotEmpty, reason: 'test setup sanity check'); // Cause an error to be thrown within a ReactJS lifecycle method + print(jacket.getNode().innerHtml); queryByTestId(jacket.getInstance(), 'flawedComponent_flawedButton').click(); }); @@ -77,9 +81,11 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { expect(infoArg, isA()); }); - test('and sets `state.hasError` to true as a result', () { - expect(jacket.getDartInstance().state.hasError, isTrue); - }); + if (!isWrapper) { + test('and sets `state.hasError` to true as a result', () { + expect(jacket.getDartInstance().state.hasError, isTrue); + }); + } test('and re-renders the tree as a result', () { expect(mountNode.children, isNotEmpty, @@ -97,7 +103,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { test('initializes with the expected default prop values', () { jacket = mount(builder()(dummyChild)); - + print(jacket.getProps()); expect(ErrorBoundaryPropsMapView(jacket.getProps()).identicalErrorFrequencyTolerance.inSeconds, 5); }); @@ -115,16 +121,31 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { expect(jacket.getNode().text, 'hi there'); }); - test('its child when `state.error` is true and `state.showFallbackUIOnError` is false', () { - jacket = mount(builder()(dummyChild)); - final component = jacket.getDartInstance(); - component.setState(component.newState() - ..hasError = true - ..showFallbackUIOnError = false - ); + if (isWrapper) { + test('no child when `state.error` is true and ignores when `state.showFallbackUIOnError` is false', () { + jacket = mount(builder()(dummyChild)); + final component = jacket.getDartInstance(); + component.setState(component.newState() + ..hasError = true + ..showFallbackUIOnError = false + ); - expect(jacket.getNode().text, 'hi there'); - }); + expect(getByTestId(jacket.getInstance(), 'dummyChild'), isNull, + reason: 'The child component tree should have been removed from the dom'); + expect(jacket.getNode(), hasAttr(defaultTestIdKey, 'ErrorBoundary.unrecoverableErrorInnerHtmlContainerNode')); + }); + } else { + test('its child when `state.error` is true and `state.showFallbackUIOnError` is false', () { + jacket = mount(builder()(dummyChild)); + final component = jacket.getDartInstance(); + component.setState(component.newState() + ..hasError = true + ..showFallbackUIOnError = false + ); + + expect(jacket.getNode().text, 'hi there'); + }); + } group('fallback UI when `state.error` is true', () { test('and `state.showFallbackUIOnError` is true ("unrecoverable" error state)', () { @@ -170,13 +191,15 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { expect(getByTestId(jacket.getInstance(), 'fallbackNode'), isNotNull); }); - test('when reset() is called', () { - jacket.getDartInstance().reset(); - expect(jacket.getDartInstance().state.hasError, isFalse); - expect(jacket.getDartInstance().state.showFallbackUIOnError, isTrue); - expect(getByTestId(jacket.getInstance(), 'dummyChild'), isNotNull); - expect(getByTestId(jacket.getInstance(), 'fallbackNode'), isNull); - }); + if (!isWrapper) { + test('when reset() is called', () { + (jacket.getDartInstance() as dynamic).reset(); + expect(jacket.getDartInstance().state.hasError, isFalse); + expect(jacket.getDartInstance().state.showFallbackUIOnError, isTrue); + expect(getByTestId(jacket.getInstance(), 'dummyChild'), isNotNull); + expect(getByTestId(jacket.getInstance(), 'fallbackNode'), isNull); + }); + } group('when a new child is passed in', () { test('', () { @@ -198,7 +221,9 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { ..fallbackUIRenderer = (_, __) => (Dom.div()..addTestId('fallbackNode'))('Something went wrong') ) )((FlawedOnMount()..addTestId('flawed'))())); - expect(jacket.getDartInstance().state.hasError, isTrue); + if (!isWrapper) { + expect(jacket.getDartInstance().state.hasError, isTrue); + } expect(jacket.getDartInstance().state.showFallbackUIOnError, isTrue); expect(getByTestId(jacket.getInstance(), 'flawed'), isNull); expect(getByTestId(jacket.getInstance(), 'fallbackNode'), isNotNull); @@ -227,7 +252,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { expect(calls, isNotNull, reason: 'test setup sanity check'); expect(calls, isNotEmpty, reason: 'test setup sanity check'); expect(calls[0].keys.single, 'onComponentDidCatch', reason: 'test setup sanity check'); - expect(jacket.getDartInstance().state.hasError, isTrue, reason: 'test setup sanity check'); + //expect(jacket.getDartInstance().state.hasError, isTrue, reason: 'test setup sanity check'); final componentDidCatchCallbackArguments = calls[0]['onComponentDidCatch']; if (componentDidCatchCallbackArguments != null) { @@ -828,7 +853,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { void sharedSetup({String loggerName, bool shouldLogErrors = true, Logger customLogger}) { calls = []; jacket = mount( - (ErrorBoundary() + (builder() ..loggerName = loggerName ..shouldLogErrors = shouldLogErrors ..logger = customLogger @@ -921,7 +946,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { setUp(() { Logger('myCustomErrorLoggerName').clearListeners(); jacket.rerender( - (ErrorBoundary() + (builder() ..loggerName = 'myCustomErrorLoggerName' ..identicalErrorFrequencyTolerance = const Duration(milliseconds: identicalErrorFrequencyToleranceInMs) ..onComponentDidCatch = (err, info) { @@ -946,7 +971,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder) { setUp(() { Logger(defaultErrorBoundaryLoggerName).clearListeners(); jacket.rerender( - (ErrorBoundary() + (builder() ..loggerName = null ..identicalErrorFrequencyTolerance = const Duration(milliseconds: identicalErrorFrequencyToleranceInMs) ..onComponentDidCatch = (err, info) { diff --git a/test/over_react_component_test.dart b/test/over_react_component_test.dart index e99193743..60e48841d 100644 --- a/test/over_react_component_test.dart +++ b/test/over_react_component_test.dart @@ -41,16 +41,16 @@ void main() { enableTestMode(); - abstract_transition_test.main(); - abstract_transition2_test.main(); - error_boundary_mixin_test.main(); + // abstract_transition_test.main(); + // abstract_transition2_test.main(); + //error_boundary_mixin_test.main(); error_boundary_test.main(); - forward_ref_test.main(); - dom_components_test.main(); - prop_mixins_test.main(); - prop_typedefs_test.main(); - resize_sensor_test.main(); - fragment_component_test.main(); - context_test.main(); - typed_factory_test.main(); + // forward_ref_test.main(); + // dom_components_test.main(); + // prop_mixins_test.main(); + // prop_typedefs_test.main(); + // resize_sensor_test.main(); + // fragment_component_test.main(); + // context_test.main(); + // typed_factory_test.main(); } From 5609a77ce60e3c3fc2ccf8e5d34337fde91c100d Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Thu, 12 Dec 2019 12:37:45 -0700 Subject: [PATCH 03/13] Fix all the tests!!!! --- lib/src/component/error_boundary.dart | 24 ++- lib/src/component/error_boundary_mixins.dart | 3 + .../shared_error_boundary_tests.dart | 97 +++++++++--- web/component2/index.dart | 12 ++ web/component2/index.html | 7 + web/component2/src/demos.dart | 1 + web/src/demos/faulty-on-mount-component.dart | 26 ++++ ...aulty-on-mount-component.over_react.g.dart | 147 ++++++++++++++++++ 8 files changed, 282 insertions(+), 35 deletions(-) create mode 100644 web/src/demos/faulty-on-mount-component.dart create mode 100644 web/src/demos/faulty-on-mount-component.over_react.g.dart diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart index ad0c230ed..fa954bf8d 100644 --- a/lib/src/component/error_boundary.dart +++ b/lib/src/component/error_boundary.dart @@ -61,7 +61,7 @@ class ErrorBoundaryComponent (newProps() @@ -71,18 +71,16 @@ class ErrorBoundaryComponent newState() + get initialState => (newState() ..hasError = false - ..showFallbackUIOnError = true; + ..showFallbackUIOnError = true + ); @override - Map getDerivedStateFromError(error) { - _error = error; - return (newState() - ..hasError = true - ..showFallbackUIOnError = true - ); - } + Map getDerivedStateFromError(error) => (newState() + ..hasError = true + ..showFallbackUIOnError = true + ); @override void componentDidCatch(error, ReactErrorInfo info) { @@ -104,7 +102,7 @@ class ErrorBoundaryComponent on UiStatefulComponent2 { @override diff --git a/test/over_react/component/shared_error_boundary_tests.dart b/test/over_react/component/shared_error_boundary_tests.dart index 107d8a4b3..543747f2e 100644 --- a/test/over_react/component/shared_error_boundary_tests.dart +++ b/test/over_react/component/shared_error_boundary_tests.dart @@ -1,6 +1,5 @@ import 'dart:html'; import 'package:logging/logging.dart'; -import 'package:meta/meta.dart'; import 'package:over_react/over_react.dart'; import 'package:over_react_test/over_react_test.dart'; import 'package:test/test.dart'; @@ -63,7 +62,6 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil ); expect(mountNode.children, isNotEmpty, reason: 'test setup sanity check'); // Cause an error to be thrown within a ReactJS lifecycle method - print(jacket.getNode().innerHtml); queryByTestId(jacket.getInstance(), 'flawedComponent_flawedButton').click(); }); @@ -103,7 +101,6 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil test('initializes with the expected default prop values', () { jacket = mount(builder()(dummyChild)); - print(jacket.getProps()); expect(ErrorBoundaryPropsMapView(jacket.getProps()).identicalErrorFrequencyTolerance.inSeconds, 5); }); @@ -251,8 +248,12 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil void _setCallbackVarValues() { expect(calls, isNotNull, reason: 'test setup sanity check'); expect(calls, isNotEmpty, reason: 'test setup sanity check'); - expect(calls[0].keys.single, 'onComponentDidCatch', reason: 'test setup sanity check'); - //expect(jacket.getDartInstance().state.hasError, isTrue, reason: 'test setup sanity check'); + + if (isWrapper && calls.length > 3) { + // Only the last 2 calls to `onComponentDidCatch` and `onComponentIsUnrecoverable` + // belong to the wrapping error boundary. + calls = calls.sublist(calls.length - 2); + } final componentDidCatchCallbackArguments = calls[0]['onComponentDidCatch']; if (componentDidCatchCallbackArguments != null) { @@ -260,8 +261,9 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil errorInfoSentToComponentDidCatchCallback = componentDidCatchCallbackArguments[1]; } + // At this point the component is "Unrecoverable" if (calls.length > 1) { - expect(calls[1].keys.single, 'onComponentIsUnrecoverable', reason: 'test setup sanity check'); + expect(calls[calls.length - 1].keys.single, 'onComponentIsUnrecoverable', reason: 'test setup sanity check'); final componentIsUnrecoverableCallbackArguments = calls[1]['onComponentIsUnrecoverable']; if (componentIsUnrecoverableCallbackArguments != null) { errorSentToComponentIsUnrecoverableCallback = componentIsUnrecoverableCallbackArguments[0]; @@ -329,6 +331,41 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil _setCallbackVarValues(); } + if (isWrapper) { + group('and the errors are during the render cycle', () { + group('and they occurred on every render', () { + setUp(() async { + sharedSetup(errorBoundaryChildren: FlawedOnMount()()); + }); + + test('the components wrapped by the ErrorBoundary get unmounted', () { + expect(getByTestId(jacket.getInstance(), 'flawedComponent'), isNull, + reason: 'The flawed component should have been unmounted'); + expect(jacket.getDartInstance().state.showFallbackUIOnError, isTrue, + reason: 'Fallback UI should be rendered instead of the flawed component tree'); + expect(jacket.getNode(), + hasAttr(defaultTestIdKey, 'ErrorBoundary.unrecoverableErrorInnerHtmlContainerNode')); + calls.clear(); + }); + + test('the correct callbacks are called with the correct arguments', () { + expect(calls.any((call) => call.keys.single == 'onComponentIsUnrecoverable'), isTrue, + reason: 'onComponentIsUnrecoverable should have been called'); + + _setCallbackVarValues(); + + expect(errorSentToComponentDidCatchCallback, isA()); + expect(errorSentToComponentIsUnrecoverableCallback, isA()); + expect(errorSentToComponentDidCatchCallback, errorSentToComponentIsUnrecoverableCallback); + + expect(errorInfoSentToComponentDidCatchCallback, isA()); + expect(errorInfoSentToComponentIsUnrecoverableCallback, isA()); + expect(errorInfoSentToComponentDidCatchCallback, errorInfoSentToComponentIsUnrecoverableCallback); + }); + }); + }); + } + group('and the errors are exactly the same', () { group('and they occurred more frequently than the value of props.identicalErrorFrequencyTolerance', () { String errorBoundaryInnerHtmlBeforeUnrecoverableError; @@ -475,8 +512,10 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil test('the components wrapped by the ErrorBoundary get remounted', () { expect(getByTestId(jacket.getInstance(), 'flawedComponent'), isNotNull, reason: 'The flawed component should have been remounted'); - expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + if (!isWrapper) { + expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, + reason: 'Fallback UI should be not be rendered'); + } }); test('the correct callbacks are called with the correct arguments', () { @@ -583,8 +622,10 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil test('the components wrapped by the ErrorBoundary get remounted', () { expect(getByTestId(jacket.getInstance(), 'flawedComponent'), isNotNull, reason: 'The flawed component should have been remounted'); - expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + if (!isWrapper) { + expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, + reason: 'Fallback UI should be not be rendered'); + } }); test('the correct callbacks are called with the correct arguments', () { @@ -651,8 +692,10 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil test('the components wrapped by the ErrorBoundary get remounted', () { expect(getByTestId(jacket.getInstance(), 'flawedComponent'), isNotNull, reason: 'The flawed component should have been remounted'); - expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + if (!isWrapper) { + expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, + reason: 'Fallback UI should be not be rendered'); + } }); test('the correct callbacks are called with the correct arguments', () { @@ -684,8 +727,10 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil test('the components wrapped by the ErrorBoundary get remounted', () { expect(getByTestId(jacket.getInstance(), 'flawedComponent'), isNotNull, reason: 'The flawed component should have been remounted'); - expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + if (!isWrapper) { + expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, + reason: 'Fallback UI should be not be rendered'); + } }); test('the correct callbacks are called with the correct arguments', () { @@ -723,8 +768,10 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil test('the components wrapped by the ErrorBoundary get remounted', () { expect(getByTestId(jacket.getInstance(), 'nestedFlawedComponent'), isNotNull, reason: 'The flawed component should have been remounted'); - expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + if (!isWrapper) { + expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, + reason: 'Fallback UI should be not be rendered'); + } }); test('the correct callbacks are called with the correct arguments', () { @@ -756,8 +803,10 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil test('the components wrapped by the ErrorBoundary get remounted', () { expect(getByTestId(jacket.getInstance(), 'nestedFlawedComponent'), isNotNull, reason: 'The flawed component should have been remounted'); - expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + if (!isWrapper) { + expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, + reason: 'Fallback UI should be not be rendered'); + } }); test('the correct callbacks are called with the correct arguments', () { @@ -791,8 +840,10 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil test('the components wrapped by the ErrorBoundary get remounted', () { expect(getByTestId(jacket.getInstance(), 'nestedFlawedComponent'), isNotNull, reason: 'The flawed component should have been remounted'); - expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + if (!isWrapper) { + expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, + reason: 'Fallback UI should be not be rendered'); + } }); test('the correct callbacks are called with the correct arguments', () { @@ -824,8 +875,10 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil test('the components wrapped by the ErrorBoundary get remounted', () { expect(getByTestId(jacket.getInstance(), 'nestedFlawedComponent'), isNotNull, reason: 'The flawed component should have been remounted'); - expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + if (!isWrapper) { + expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, + reason: 'Fallback UI should be not be rendered'); + } }); test('the correct callbacks are called with the correct arguments', () { diff --git a/web/component2/index.dart b/web/component2/index.dart index eb8321500..e84f885c8 100644 --- a/web/component2/index.dart +++ b/web/component2/index.dart @@ -35,6 +35,18 @@ void main() { querySelector('$demoMountNodeSelectorPrefix--faulty-component'), ); + react_dom.render( + (ErrorBoundary() + ..fallbackUIRenderer = (error, info) { + return Dom.div()('It totally crashed.'); + } + ..onComponentDidCatch = (error, info) { + print('Consumer props.onComponentDidCatch($error, $info)'); + } + )(FaultyOnMount()()), + querySelector('$demoMountNodeSelectorPrefix--faulty-on-mount-component'), + ); + react_dom.render( (CustomErrorBoundary() ..onComponentDidCatch = (error, info) { diff --git a/web/component2/index.html b/web/component2/index.html index 64a76bd53..b2c4a84ff 100644 --- a/web/component2/index.html +++ b/web/component2/index.html @@ -58,6 +58,13 @@

Component That Throws A Caught Error

around with the component rendered below.

+
+

Component That Throws A Caught Error On Mount (everytime)

+

(Default ErrorBoundary Component)

+

Modify the source of /web/demos/faulty-component.dart to play + around with the component rendered below.

+
+

Component That Throws A Caught Error

(Custom Error Boundary Component)

diff --git a/web/component2/src/demos.dart b/web/component2/src/demos.dart index bf3a1e7d7..d29910e51 100644 --- a/web/component2/src/demos.dart +++ b/web/component2/src/demos.dart @@ -13,6 +13,7 @@ export 'demos/custom_error_boundary.dart'; export '../src/demo_components/prop_validation_wrap.dart'; export '../../src/demos/faulty-component.dart'; +export '../../src/demos/faulty-on-mount-component.dart'; export 'demos/list-group/list-group-basic.dart'; export 'demos/list-group/list-group-tags.dart'; diff --git a/web/src/demos/faulty-on-mount-component.dart b/web/src/demos/faulty-on-mount-component.dart new file mode 100644 index 000000000..f96305e67 --- /dev/null +++ b/web/src/demos/faulty-on-mount-component.dart @@ -0,0 +1,26 @@ +import 'package:over_react/over_react.dart'; + +part 'faulty-on-mount-component.over_react.g.dart'; + + +@Factory() +// ignore: undefined_identifier +UiFactory FaultyOnMount = _$FaultyOnMount; + +@Props() +class _$FaultyOnMountProps extends UiProps {} + +// AF-3369 This will be removed once the transition to Dart 2 is complete. +// ignore: mixin_of_non_class, undefined_class +class FaultyOnMountProps extends _$FaultyOnMountProps with _$FaultyOnMountPropsAccessorsMixin { + // ignore: undefined_identifier, undefined_class, const_initialized_with_non_constant_value + static const PropsMeta meta = _$metaForFaultyOnMountProps; +} + +@Component2() +class FaultyOnMountComponent extends UiComponent2 { + @override + render() { + throw 'lol'; + } +} diff --git a/web/src/demos/faulty-on-mount-component.over_react.g.dart b/web/src/demos/faulty-on-mount-component.over_react.g.dart new file mode 100644 index 000000000..2e50675c9 --- /dev/null +++ b/web/src/demos/faulty-on-mount-component.over_react.g.dart @@ -0,0 +1,147 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +// ignore_for_file: deprecated_member_use_from_same_package, unnecessary_null_in_if_null_operators, prefer_null_aware_operators +part of 'faulty-on-mount-component.dart'; + +// ************************************************************************** +// OverReactBuilder (package:over_react/src/builder.dart) +// ************************************************************************** + +// React component factory implementation. +// +// Registers component implementation and links type meta to builder factory. +final $FaultyOnMountComponentFactory = registerComponent2( + () => _$FaultyOnMountComponent(), + builderFactory: FaultyOnMount, + componentClass: FaultyOnMountComponent, + isWrapper: false, + parentType: null, + displayName: 'FaultyOnMount', +); + +abstract class _$FaultyOnMountPropsAccessorsMixin + implements _$FaultyOnMountProps { + @override + Map get props; + + /* GENERATED CONSTANTS */ + + static const List $props = []; + static const List $propKeys = []; +} + +const PropsMeta _$metaForFaultyOnMountProps = PropsMeta( + fields: _$FaultyOnMountPropsAccessorsMixin.$props, + keys: _$FaultyOnMountPropsAccessorsMixin.$propKeys, +); + +_$$FaultyOnMountProps _$FaultyOnMount([Map backingProps]) => + backingProps == null + ? _$$FaultyOnMountProps$JsMap(JsBackedMap()) + : _$$FaultyOnMountProps(backingProps); + +// Concrete props implementation. +// +// Implements constructor and backing map, and links up to generated component factory. +abstract class _$$FaultyOnMountProps extends _$FaultyOnMountProps + with _$FaultyOnMountPropsAccessorsMixin + implements FaultyOnMountProps { + _$$FaultyOnMountProps._(); + + factory _$$FaultyOnMountProps(Map backingMap) { + if (backingMap == null || backingMap is JsBackedMap) { + return _$$FaultyOnMountProps$JsMap(backingMap); + } else { + return _$$FaultyOnMountProps$PlainMap(backingMap); + } + } + + /// Let `UiProps` internals know that this class has been generated. + @override + bool get $isClassGenerated => true; + + /// The `ReactComponentFactory` associated with the component built by this class. + @override + ReactComponentFactoryProxy get componentFactory => + super.componentFactory ?? $FaultyOnMountComponentFactory; + + /// The default namespace for the prop getters/setters generated for this class. + @override + String get propKeyNamespace => 'FaultyOnMountProps.'; +} + +// Concrete props implementation that can be backed by any [Map]. +class _$$FaultyOnMountProps$PlainMap extends _$$FaultyOnMountProps { + // This initializer of `_props` to an empty map, as well as the reassignment + // of `_props` in the constructor body is necessary to work around a DDC bug: https://github.com/dart-lang/sdk/issues/36217 + _$$FaultyOnMountProps$PlainMap(Map backingMap) + : this._props = {}, + super._() { + this._props = backingMap ?? {}; + } + + /// The backing props map proxied by this class. + @override + Map get props => _props; + Map _props; +} + +// Concrete props implementation that can only be backed by [JsMap], +// allowing dart2js to compile more optimal code for key-value pair reads/writes. +class _$$FaultyOnMountProps$JsMap extends _$$FaultyOnMountProps { + // This initializer of `_props` to an empty map, as well as the reassignment + // of `_props` in the constructor body is necessary to work around a DDC bug: https://github.com/dart-lang/sdk/issues/36217 + _$$FaultyOnMountProps$JsMap(JsBackedMap backingMap) + : this._props = JsBackedMap(), + super._() { + this._props = backingMap ?? JsBackedMap(); + } + + /// The backing props map proxied by this class. + @override + JsBackedMap get props => _props; + JsBackedMap _props; +} + +// Concrete component implementation mixin. +// +// Implements typed props/state factories, defaults `consumedPropKeys` to the keys +// generated for the associated props class. +class _$FaultyOnMountComponent extends FaultyOnMountComponent { + _$$FaultyOnMountProps$JsMap _cachedTypedProps; + + @override + _$$FaultyOnMountProps$JsMap get props => _cachedTypedProps; + + @override + set props(Map value) { + assert( + getBackingMap(value) is JsBackedMap, + 'Component2.props should never be set directly in ' + 'production. If this is required for testing, the ' + 'component should be rendered within the test. If ' + 'that does not have the necessary result, the last ' + 'resort is to use typedPropsFactoryJs.'); + super.props = value; + _cachedTypedProps = typedPropsFactoryJs(getBackingMap(value)); + } + + @override + _$$FaultyOnMountProps$JsMap typedPropsFactoryJs(JsBackedMap backingMap) => + _$$FaultyOnMountProps$JsMap(backingMap); + + @override + _$$FaultyOnMountProps typedPropsFactory(Map backingMap) => + _$$FaultyOnMountProps(backingMap); + + /// Let `UiComponent` internals know that this class has been generated. + @override + bool get $isClassGenerated => true; + + /// The default consumed props, taken from _$FaultyOnMountProps. + /// Used in `ConsumedProps` if [consumedProps] is not overridden. + @override + final List $defaultConsumedProps = const [ + _$metaForFaultyOnMountProps + ]; +} From 727ebc401a438bc0f5e5525673d687d5c598104e Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Thu, 12 Dec 2019 14:59:24 -0700 Subject: [PATCH 04/13] uncomment out tests --- test/over_react/component/error_boundary_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/over_react/component/error_boundary_test.dart b/test/over_react/component/error_boundary_test.dart index d903d487c..c56d1dfeb 100644 --- a/test/over_react/component/error_boundary_test.dart +++ b/test/over_react/component/error_boundary_test.dart @@ -23,9 +23,9 @@ import 'package:test/test.dart'; import 'shared_error_boundary_tests.dart'; void main() { - // group('RecoverableErrorBoundary', () { - // sharedErrorBoundaryTests(() => RecoverableErrorBoundary()); - // }); + group('RecoverableErrorBoundary', () { + sharedErrorBoundaryTests(() => RecoverableErrorBoundary()); + }); group('ErrorBoundary', () { sharedErrorBoundaryTests(() => ErrorBoundary(), isWrapper: true); From 5ea09a8d616959f4907d234d7a8e15fe90e88a5e Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Thu, 12 Dec 2019 15:00:41 -0700 Subject: [PATCH 05/13] more test uncommenting --- test/over_react_component_test.dart | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/over_react_component_test.dart b/test/over_react_component_test.dart index 60e48841d..e99193743 100644 --- a/test/over_react_component_test.dart +++ b/test/over_react_component_test.dart @@ -41,16 +41,16 @@ void main() { enableTestMode(); - // abstract_transition_test.main(); - // abstract_transition2_test.main(); - //error_boundary_mixin_test.main(); + abstract_transition_test.main(); + abstract_transition2_test.main(); + error_boundary_mixin_test.main(); error_boundary_test.main(); - // forward_ref_test.main(); - // dom_components_test.main(); - // prop_mixins_test.main(); - // prop_typedefs_test.main(); - // resize_sensor_test.main(); - // fragment_component_test.main(); - // context_test.main(); - // typed_factory_test.main(); + forward_ref_test.main(); + dom_components_test.main(); + prop_mixins_test.main(); + prop_typedefs_test.main(); + resize_sensor_test.main(); + fragment_component_test.main(); + context_test.main(); + typed_factory_test.main(); } From d388adba71eb240e32eb4679ad9088e831bdee2c Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Fri, 13 Dec 2019 14:56:18 -0700 Subject: [PATCH 06/13] address cr feedback --- lib/src/component/error_boundary.dart | 9 ++++++--- lib/src/component/error_boundary_mixins.dart | 9 ++++++--- lib/src/component/error_boundary_recoverable.dart | 13 +++++-------- web/component2/index.dart | 3 --- web/component2/index.html | 6 +++--- web/src/demos/faulty-on-mount-component.dart | 7 ++++++- 6 files changed, 26 insertions(+), 21 deletions(-) diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart index fa954bf8d..bcc07992d 100644 --- a/lib/src/component/error_boundary.dart +++ b/lib/src/component/error_boundary.dart @@ -57,14 +57,16 @@ class ErrorBoundaryComponent (newProps() + get defaultProps => (newProps() ..identicalErrorFrequencyTolerance = Duration(seconds: 5) ..loggerName = defaultErrorBoundaryLoggerName ..shouldLogErrors = true @@ -110,7 +112,7 @@ class ErrorBoundaryComponent See: [ErrorBoundaryMixin] for a usage example. +@Deprecated('Building custom error boundaries with this mixin will no longer be supported in version 4.0.0.' + 'Use ErrorBoundary and its prop API to customize error handling instead.') @PropsMixin() abstract class _$ErrorBoundaryPropsMixin implements UiProps { @override @@ -109,6 +111,8 @@ abstract class _$ErrorBoundaryPropsMixin implements UiProps { /// within a custom component. /// /// > See: [ErrorBoundaryMixin] for a usage example. +@Deprecated('Building custom error boundaries with this mixin will no longer be supported in version 4.0.0.' + 'Use ErrorBoundary and its prop API to customize error handling instead.') @StateMixin() abstract class _$ErrorBoundaryStateMixin implements UiState { @override @@ -158,9 +162,8 @@ abstract class _$ErrorBoundaryStateMixin implements UiState { /// return Dom.h3()('Error!'); /// } /// } -/// -/// __Deprecated.__ Use `ErrorBoundary` component and its props to handle errors instead. -@Deprecated('4.0.0') +@Deprecated('Building custom error boundaries with this mixin will no longer be supported in version 4.0.0.' + 'Use ErrorBoundary and its prop API to customize error handling instead.') mixin ErrorBoundaryMixin on UiStatefulComponent2 { @override diff --git a/lib/src/component/error_boundary_recoverable.dart b/lib/src/component/error_boundary_recoverable.dart index 474b1ed3d..9ecaf5937 100644 --- a/lib/src/component/error_boundary_recoverable.dart +++ b/lib/src/component/error_boundary_recoverable.dart @@ -2,15 +2,12 @@ import 'package:over_react/over_react.dart'; part 'error_boundary_recoverable.over_react.g.dart'; -/// 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. +/// A higher-order component that will catch "recoverable" ReactJS errors, errors outside of the render/mount cycle, +/// anywhere within the child component tree and display a fallback UI instead of the component tree that crashed. /// -/// Optionally, use the [ErrorBoundaryProps.onComponentDidCatch] -/// to send error / stack trace information to a logging endpoint for your application. -/// -/// To make your own custom error boundaries, you can utilize the [ErrorBoundaryPropsMixin], -/// [ErrorBoundaryStateMixin] and [ErrorBoundaryMixin]s on any component that is annotated -/// using `@Component2(isErrorBoundary: true)`. See the [ErrorBoundaryMixin] for an example implementation. +/// __NOTE:__ +/// 1. This component is not / should never be publicly exported. +/// 2. This component should never be used, except as a child of the outer (public) `ErrorBoundary` component. @Factory() UiFactory RecoverableErrorBoundary = _$RecoverableErrorBoundary; diff --git a/web/component2/index.dart b/web/component2/index.dart index e84f885c8..6a00dfe9f 100644 --- a/web/component2/index.dart +++ b/web/component2/index.dart @@ -37,9 +37,6 @@ void main() { react_dom.render( (ErrorBoundary() - ..fallbackUIRenderer = (error, info) { - return Dom.div()('It totally crashed.'); - } ..onComponentDidCatch = (error, info) { print('Consumer props.onComponentDidCatch($error, $info)'); } diff --git a/web/component2/index.html b/web/component2/index.html index b2c4a84ff..fbe88db78 100644 --- a/web/component2/index.html +++ b/web/component2/index.html @@ -59,10 +59,10 @@

Component That Throws A Caught Error

-

Component That Throws A Caught Error On Mount (everytime)

+

Component That Throws A Caught Error On Mount (every time)

(Default ErrorBoundary Component)

-

Modify the source of /web/demos/faulty-component.dart to play - around with the component rendered below.

+

Modify the source of /web/demos/faulty-on-mount-component.dart to play + around with the component rendered below. By default nothing is rendered except an empty div.

diff --git a/web/src/demos/faulty-on-mount-component.dart b/web/src/demos/faulty-on-mount-component.dart index f96305e67..64e48e98f 100644 --- a/web/src/demos/faulty-on-mount-component.dart +++ b/web/src/demos/faulty-on-mount-component.dart @@ -21,6 +21,11 @@ class FaultyOnMountProps extends _$FaultyOnMountProps with _$FaultyOnMountPropsA class FaultyOnMountComponent extends UiComponent2 { @override render() { - throw 'lol'; + throw Shade(); } } + +class Shade extends Error { + @override + toString() => 'lol'; +} From 6ceea56bd726f464dea09c691254eda915973651 Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Fri, 13 Dec 2019 14:56:56 -0700 Subject: [PATCH 07/13] remove accidental print --- lib/src/component/error_boundary.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart index bcc07992d..b6df8e323 100644 --- a/lib/src/component/error_boundary.dart +++ b/lib/src/component/error_boundary.dart @@ -122,7 +122,6 @@ class ErrorBoundaryComponent Date: Mon, 16 Dec 2019 16:03:17 -0700 Subject: [PATCH 08/13] cleanup fallbackUIRenderer logic and update tests --- lib/src/component/error_boundary.dart | 32 +++----- .../shared_error_boundary_tests.dart | 80 ++++++++++++++----- web/component2/index.dart | 12 +++ web/component2/index.html | 7 ++ 4 files changed, 88 insertions(+), 43 deletions(-) diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart index b6df8e323..37f85d6e2 100644 --- a/lib/src/component/error_boundary.dart +++ b/lib/src/component/error_boundary.dart @@ -1,4 +1,5 @@ import 'package:logging/logging.dart'; +import 'package:meta/meta.dart'; import 'package:over_react/over_react.dart'; import 'package:over_react/src/component/error_boundary_recoverable.dart'; @@ -53,18 +54,11 @@ class ErrorBoundaryComponent (newProps() ..identicalErrorFrequencyTolerance = Duration(seconds: 5) @@ -86,9 +80,6 @@ class ErrorBoundaryComponent buil expect(jacket.getNode(), hasAttr(defaultTestIdKey, 'ErrorBoundary.unrecoverableErrorInnerHtmlContainerNode')); }); - test('and props.fallbackUIRenderer is set', () { - ReactElement _fallbackUIRenderer(_, __) { - return Dom.h4()('Something super not awesome just happened.'); - } + if (!isWrapper) { + test('and props.fallbackUIRenderer is set', () { + ReactElement _fallbackUIRenderer(_, __) { + return Dom.h4()('Something super not awesome just happened.'); + } - jacket = mount((builder()..addProps(ErrorBoundaryPropsMapView({})..fallbackUIRenderer = _fallbackUIRenderer))(dummyChild)); - final component = jacket.getDartInstance(); - component.setState(component.newState()..hasError = true); + jacket = mount((builder()..addProps(ErrorBoundaryPropsMapView({})..fallbackUIRenderer = _fallbackUIRenderer))(dummyChild)); + final component = jacket.getDartInstance(); + component.setState(component.newState()..hasError = true); - expect(jacket.getNode(), hasNodeName('H4'), reason: '${ErrorBoundaryPropsMapView(jacket.getProps()).fallbackUIRenderer}'); - expect(jacket.getNode().text, 'Something super not awesome just happened.'); - }); + expect(jacket.getNode(), hasNodeName('H4'), reason: '${ErrorBoundaryPropsMapView(jacket.getProps()).fallbackUIRenderer}'); + expect(jacket.getNode().text, 'Something super not awesome just happened.'); + }); + } group('and then switches back to rendering the child', () { setUp(() { @@ -182,21 +184,23 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil final component = jacket.getDartInstance(); component.setState(component.newState() ..hasError = true - ..showFallbackUIOnError = true ); expect(getByTestId(jacket.getInstance(), 'dummyChild'), isNull); - expect(getByTestId(jacket.getInstance(), 'fallbackNode'), isNotNull); + if (!isWrapper) { + // wrapper ErrorBoundary doesn't use `props.fallbackUIRenderer` because `ResolvableErrorBoundary` + // will display it on first error. Meaning wrapper ErrorBoundary will never be reached if it is set. + expect(getByTestId(jacket.getInstance(), 'fallbackNode'), isNotNull); + } }); - if (!isWrapper) { - test('when reset() is called', () { - (jacket.getDartInstance() as dynamic).reset(); - expect(jacket.getDartInstance().state.hasError, isFalse); - expect(jacket.getDartInstance().state.showFallbackUIOnError, isTrue); - expect(getByTestId(jacket.getInstance(), 'dummyChild'), isNotNull); - expect(getByTestId(jacket.getInstance(), 'fallbackNode'), isNull); - }); - } + test('when reset() is called', () { + dynamic component = jacket.getDartInstance(); + (component as dynamic).reset(); + expect(component.state.hasError, isFalse); + expect(component.state.showFallbackUIOnError, isTrue); + expect(getByTestId(jacket.getInstance(), 'dummyChild'), isNotNull); + expect(getByTestId(jacket.getInstance(), 'fallbackNode'), isNull); + }); group('when a new child is passed in', () { test('', () { @@ -272,7 +276,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil } } - void sharedSetup({dynamic errorBoundaryChildren}) { + void sharedSetup({dynamic errorBoundaryChildren, Map errorBoundaryProps}) { final customChildrenUsed = errorBoundaryChildren != null; errorBoundaryChildren ??= (Flawed()..addTestId('flawedComponent'))( (Flawed() @@ -293,6 +297,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil ..onComponentIsUnrecoverable = (err, info) { calls.add({'onComponentIsUnrecoverable': [err, info]}); } + ..addAll(errorBoundaryProps ?? {}) ) )(errorBoundaryChildren), attachedToDocument: true); @@ -363,6 +368,37 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil expect(errorInfoSentToComponentDidCatchCallback, errorInfoSentToComponentIsUnrecoverableCallback); }); }); + + group('and they occurred on every render and fallbackUIRenderer is set', () { + setUp(() async { + sharedSetup( + errorBoundaryChildren: FlawedOnMount()(), + errorBoundaryProps: (ErrorBoundaryPropsMapView({}) + ..fallbackUIRenderer = (_, __) => (Dom.div()..addTestId('fallbackUIRenderer'))() + ), + ); + }); + + test('the components wrapped by the ErrorBoundary get unmounted', () { + expect(getByTestId(jacket.getInstance(), 'flawedComponent'), isNull, + reason: 'The flawed component should have been unmounted'); + expect(jacket.getDartInstance().state.showFallbackUIOnError, isTrue, + reason: 'Fallback UI should be rendered instead of the flawed component tree'); + expect(jacket.getNode(), + hasAttr(defaultTestIdKey, 'fallbackUIRenderer')); + calls.clear(); + }); + + test('the correct callbacks are called with the correct arguments', () { + expect(calls.any((call) => call.keys.single == 'onComponentDidCatch'), isTrue, + reason: 'onComponentDidCatch should have been called'); + + _setCallbackVarValues(); + + expect(errorSentToComponentDidCatchCallback, isA()); + expect(errorInfoSentToComponentDidCatchCallback, isA()); + }); + }); }); } diff --git a/web/component2/index.dart b/web/component2/index.dart index 6a00dfe9f..0103a063e 100644 --- a/web/component2/index.dart +++ b/web/component2/index.dart @@ -44,6 +44,18 @@ void main() { querySelector('$demoMountNodeSelectorPrefix--faulty-on-mount-component'), ); + react_dom.render( + (ErrorBoundary() + ..onComponentDidCatch = (error, info) { + print('Consumer props.onComponentDidCatch($error, $info)'); + } + ..fallbackUIRenderer = (_, __) { + return (Dom.div()..id = 'FallbackUi')('I am a fallback.'); + } + )(FaultyOnMount()()), + querySelector('$demoMountNodeSelectorPrefix--faulty-on-mount-fallback-component'), + ); + react_dom.render( (CustomErrorBoundary() ..onComponentDidCatch = (error, info) { diff --git a/web/component2/index.html b/web/component2/index.html index fbe88db78..10e16152e 100644 --- a/web/component2/index.html +++ b/web/component2/index.html @@ -65,6 +65,13 @@

Component That Throws A Caught Error On Mount (every time)

around with the component rendered below. By default nothing is rendered except an empty div.

+
+

Component That Throws A Caught Error On Mount with Fallback (every time)

+

(Default ErrorBoundary Component)

+

Modify the source of /web/demos/faulty-on-mount-component.dart to play + around with the component rendered below. By default nothing is rendered except an empty div.

+
+

Component That Throws A Caught Error

(Custom Error Boundary Component)

From bdacdd8df475e3e8a7b805e741acb8406a386aa1 Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Mon, 16 Dec 2019 16:04:38 -0700 Subject: [PATCH 09/13] fix find replace mistake --- test/over_react/component/shared_error_boundary_tests.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/over_react/component/shared_error_boundary_tests.dart b/test/over_react/component/shared_error_boundary_tests.dart index f391d799e..b8fc835fd 100644 --- a/test/over_react/component/shared_error_boundary_tests.dart +++ b/test/over_react/component/shared_error_boundary_tests.dart @@ -28,7 +28,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil List> calls; DivElement mountNode; - void verifyReact16ErrorHandlingWithoutbuilder() { + void verifyReact16ErrorHandlingWithoutErrorBoundary() { mountNode = DivElement(); document.body.append(mountNode); var jacketOfFlawedComponentWithNoErrorBoundary = mount(Flawed()(), mountNode: mountNode); @@ -49,7 +49,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil setUp(() { // Verify the behavior of a component that throws when it is not wrapped in an error boundary first - verifyReact16ErrorHandlingWithoutbuilder(); + verifyReact16ErrorHandlingWithoutErrorBoundary(); calls = []; jacket = mount( From 997de19857df518cc4598e59249b8514e8a5a517 Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Tue, 17 Dec 2019 10:25:52 -0700 Subject: [PATCH 10/13] address cr feedback --- .../component/shared_error_boundary_tests.dart | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/test/over_react/component/shared_error_boundary_tests.dart b/test/over_react/component/shared_error_boundary_tests.dart index b8fc835fd..9c4c39694 100644 --- a/test/over_react/component/shared_error_boundary_tests.dart +++ b/test/over_react/component/shared_error_boundary_tests.dart @@ -392,6 +392,8 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil test('the correct callbacks are called with the correct arguments', () { expect(calls.any((call) => call.keys.single == 'onComponentDidCatch'), isTrue, reason: 'onComponentDidCatch should have been called'); + expect(calls.any((call) => call.keys.single == 'onComponentIsUnrecoverable'), isFalse, + reason: 'onComponentIsUnrecoverable should not have been called'); _setCallbackVarValues(); @@ -550,7 +552,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil reason: 'The flawed component should have been remounted'); if (!isWrapper) { expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + reason: 'Fallback UI should not be rendered'); } }); @@ -660,7 +662,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil reason: 'The flawed component should have been remounted'); if (!isWrapper) { expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + reason: 'Fallback UI should not be rendered'); } }); @@ -730,7 +732,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil reason: 'The flawed component should have been remounted'); if (!isWrapper) { expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + reason: 'Fallback UI should not be rendered'); } }); @@ -765,7 +767,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil reason: 'The flawed component should have been remounted'); if (!isWrapper) { expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + reason: 'Fallback UI should not be rendered'); } }); @@ -806,7 +808,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil reason: 'The flawed component should have been remounted'); if (!isWrapper) { expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + reason: 'Fallback UI should not be rendered'); } }); @@ -841,7 +843,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil reason: 'The flawed component should have been remounted'); if (!isWrapper) { expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + reason: 'Fallback UI should not be rendered'); } }); @@ -878,7 +880,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil reason: 'The flawed component should have been remounted'); if (!isWrapper) { expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + reason: 'Fallback UI should not be rendered'); } }); @@ -913,7 +915,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil reason: 'The flawed component should have been remounted'); if (!isWrapper) { expect(jacket.getDartInstance().state.showFallbackUIOnError, isFalse, - reason: 'Fallback UI should be not be rendered'); + reason: 'Fallback UI should not be rendered'); } }); From ef2e755348f394d91c41e06c6d81ff4c9e140874 Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Tue, 17 Dec 2019 10:28:54 -0700 Subject: [PATCH 11/13] remove unused async --- test/over_react/component/shared_error_boundary_tests.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/over_react/component/shared_error_boundary_tests.dart b/test/over_react/component/shared_error_boundary_tests.dart index 9c4c39694..91af34071 100644 --- a/test/over_react/component/shared_error_boundary_tests.dart +++ b/test/over_react/component/shared_error_boundary_tests.dart @@ -339,7 +339,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil if (isWrapper) { group('and the errors are during the render cycle', () { group('and they occurred on every render', () { - setUp(() async { + setUp(() { sharedSetup(errorBoundaryChildren: FlawedOnMount()()); }); @@ -370,7 +370,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil }); group('and they occurred on every render and fallbackUIRenderer is set', () { - setUp(() async { + setUp(() { sharedSetup( errorBoundaryChildren: FlawedOnMount()(), errorBoundaryProps: (ErrorBoundaryPropsMapView({}) From 3258d61655e292d2c2c951bec873e47b6119212b Mon Sep 17 00:00:00 2001 From: Keal Jones Date: Tue, 17 Dec 2019 10:53:06 -0700 Subject: [PATCH 12/13] add ErrorBoundaryApi mixin for local testing --- lib/over_react.dart | 2 +- lib/src/component/error_boundary.dart | 3 ++- lib/src/component/error_boundary_mixins.dart | 11 +++++++++++ lib/src/component/error_boundary_recoverable.dart | 3 ++- .../fixtures/custom_error_boundary_component.dart | 5 ++++- .../component/shared_error_boundary_tests.dart | 7 ++++--- 6 files changed, 24 insertions(+), 7 deletions(-) diff --git a/lib/over_react.dart b/lib/over_react.dart index 578e7562e..29a8e16bd 100644 --- a/lib/over_react.dart +++ b/lib/over_react.dart @@ -40,7 +40,7 @@ export 'src/component/abstract_transition_props.dart'; export 'src/component/aria_mixin.dart'; export 'src/component/callback_typedefs.dart'; export 'src/component/error_boundary.dart'; -export 'src/component/error_boundary_mixins.dart'; +export 'src/component/error_boundary_mixins.dart' hide ErrorBoundaryApi; export 'src/component/dom_components.dart'; export 'src/component/ref_util.dart'; export 'src/component/fragment_component.dart'; diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart index 37f85d6e2..51cd18cf2 100644 --- a/lib/src/component/error_boundary.dart +++ b/lib/src/component/error_boundary.dart @@ -1,6 +1,7 @@ import 'package:logging/logging.dart'; import 'package:meta/meta.dart'; import 'package:over_react/over_react.dart'; +import 'package:over_react/src/component/error_boundary_mixins.dart'; import 'package:over_react/src/component/error_boundary_recoverable.dart'; part 'error_boundary.over_react.g.dart'; @@ -25,7 +26,7 @@ class _$ErrorBoundaryState extends UiState with ErrorBoundaryStateMixin {} @Component2(isWrapper: true, isErrorBoundary: true) class ErrorBoundaryComponent - extends UiStatefulComponent2 { + extends UiStatefulComponent2 with ErrorBoundaryApi { // ---------------------------------------------- \/ ---------------------------------------------- // How This ErrorBoundary Works: diff --git a/lib/src/component/error_boundary_mixins.dart b/lib/src/component/error_boundary_mixins.dart index b2025a48f..d46961113 100644 --- a/lib/src/component/error_boundary_mixins.dart +++ b/lib/src/component/error_boundary_mixins.dart @@ -9,6 +9,17 @@ part 'error_boundary_mixins.over_react.g.dart'; @visibleForTesting const String defaultErrorBoundaryLoggerName = 'over_react.ErrorBoundary'; +/// An API mixin used for shared APIs in ErrorBoundary Components. +mixin ErrorBoundaryApi on UiStatefulComponent2 { + /// Resets the [ErrorBoundary] to a non-error state. + /// + /// This can be called manually on the component instance using a `ref` - + /// or by passing in a new child instance after a child has thrown an error. + void reset() { + setState(initialState); + } +} + /// A props mixin you can use to implement / extend from the behaviors of an [ErrorBoundary] /// within a custom component. /// diff --git a/lib/src/component/error_boundary_recoverable.dart b/lib/src/component/error_boundary_recoverable.dart index 9ecaf5937..4be1bcc93 100644 --- a/lib/src/component/error_boundary_recoverable.dart +++ b/lib/src/component/error_boundary_recoverable.dart @@ -1,4 +1,5 @@ import 'package:over_react/over_react.dart'; +import 'package:over_react/src/component/error_boundary_mixins.dart'; part 'error_boundary_recoverable.over_react.g.dart'; @@ -19,4 +20,4 @@ class _$RecoverableErrorBoundaryState extends UiState with ErrorBoundaryStateMix @Component2(isWrapper: true, isErrorBoundary: true) class RecoverableErrorBoundaryComponent - extends UiStatefulComponent2 with ErrorBoundaryMixin {} + extends UiStatefulComponent2 with ErrorBoundaryMixin, ErrorBoundaryApi {} diff --git a/test/over_react/component/fixtures/custom_error_boundary_component.dart b/test/over_react/component/fixtures/custom_error_boundary_component.dart index 0df6800fb..19f219d89 100644 --- a/test/over_react/component/fixtures/custom_error_boundary_component.dart +++ b/test/over_react/component/fixtures/custom_error_boundary_component.dart @@ -1,4 +1,5 @@ import 'package:over_react/over_react.dart'; +import 'package:over_react/src/component/error_boundary_mixins.dart'; // ignore: uri_has_not_been_generated part 'custom_error_boundary_component.over_react.g.dart'; @@ -15,4 +16,6 @@ class _$CustomErrorBoundaryState extends UiState with ErrorBoundaryStateMixin {} @Component2(isErrorBoundary: true) class CustomErrorBoundaryComponent extends UiStatefulComponent2 - with ErrorBoundaryMixin {} + with + ErrorBoundaryMixin, + ErrorBoundaryApi {} diff --git a/test/over_react/component/shared_error_boundary_tests.dart b/test/over_react/component/shared_error_boundary_tests.dart index 91af34071..c44d5eb85 100644 --- a/test/over_react/component/shared_error_boundary_tests.dart +++ b/test/over_react/component/shared_error_boundary_tests.dart @@ -1,6 +1,7 @@ import 'dart:html'; import 'package:logging/logging.dart'; import 'package:over_react/over_react.dart'; +import 'package:over_react/src/component/error_boundary_mixins.dart'; import 'package:over_react_test/over_react_test.dart'; import 'package:test/test.dart'; @@ -12,7 +13,7 @@ import './fixtures/flawed_component_that_renders_nothing.dart'; /// `isWrapper` identifies an ErrorBoundary that wraps another Error Boundary in order to handle /// render cycle "unrecoverable" errors. void sharedErrorBoundaryTests(BuilderOnlyUiFactory builder, { bool isWrapper = false }) { - TestJacket> jacket; + TestJacket jacket; ReactElement dummyChild; setUp(() { @@ -194,8 +195,8 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil }); test('when reset() is called', () { - dynamic component = jacket.getDartInstance(); - (component as dynamic).reset(); + var component = jacket.getDartInstance(); + component.reset(); expect(component.state.hasError, isFalse); expect(component.state.showFallbackUIOnError, isTrue); expect(getByTestId(jacket.getInstance(), 'dummyChild'), isNotNull); From 885d948d6d3f95430f240ae0ff76e9ffd9f5a3cf Mon Sep 17 00:00:00 2001 From: Keal Jones <41018730+kealjones-wk@users.noreply.github.com> Date: Tue, 17 Dec 2019 10:59:49 -0700 Subject: [PATCH 13/13] Apply suggestions from code review Co-Authored-By: Greg Littlefield --- lib/src/component/error_boundary.dart | 2 +- test/over_react/component/shared_error_boundary_tests.dart | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/src/component/error_boundary.dart b/lib/src/component/error_boundary.dart index 51cd18cf2..0fc5775bd 100644 --- a/lib/src/component/error_boundary.dart +++ b/lib/src/component/error_boundary.dart @@ -139,7 +139,7 @@ class ErrorBoundaryComponent builder, { bool isWrapper = false }) { TestJacket jacket; @@ -268,7 +268,7 @@ void sharedErrorBoundaryTests(BuilderOnlyUiFactory buil // At this point the component is "Unrecoverable" if (calls.length > 1) { - expect(calls[calls.length - 1].keys.single, 'onComponentIsUnrecoverable', reason: 'test setup sanity check'); + expect(calls.last.keys.single, 'onComponentIsUnrecoverable', reason: 'test setup sanity check'); final componentIsUnrecoverableCallbackArguments = calls[1]['onComponentIsUnrecoverable']; if (componentIsUnrecoverableCallbackArguments != null) { errorSentToComponentIsUnrecoverableCallback = componentIsUnrecoverableCallbackArguments[0];