Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CPLAT-3094 Lifecycle Error Boundary #175

Merged
merged 30 commits into from
Apr 17, 2019
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
e04649d
Add getDerivedStateFromProps and componentDidCatch
joebingham-wk Mar 25, 2019
e86d16a
Refine lifecycle functionality (wip)
joebingham-wk Mar 27, 2019
de1cf55
Add skipMethods and refine lifecycle events
joebingham-wk Mar 29, 2019
a58bcdb
Fix failing test
joebingham-wk Mar 29, 2019
1b8f712
Fix dart2js errors
joebingham-wk Mar 29, 2019
1d9888e
Add doc comment noting to override skipMethods
joebingham-wk Mar 29, 2019
6d868e0
Merge in Keal's context updates
joebingham-wk Mar 29, 2019
e78feb9
foorrmmmattt
joebingham-wk Mar 29, 2019
952d0f4
Merge in 5.0.0-wip
joebingham-wk Apr 3, 2019
d11e68e
Remove unnecessary changes to test components
joebingham-wk Apr 3, 2019
1632a29
Upgrade React to 16.8
joebingham-wk Apr 3, 2019
c1b05c9
Remove console.log
joebingham-wk Apr 3, 2019
97c633f
Merge 'upstream/5.0.0-wip' into CPLAT-3094-lifecycle-error-boundary
aaronlademann-wf Apr 4, 2019
785f74c
Merge remote-tracking branch 'upstream/5.0.0-wip' into react16+js-map…
aaronlademann-wf Apr 4, 2019
7a2ac4a
Address Aaron's feedback in react.dart
joebingham-wk Apr 4, 2019
7cd1da5
Address Aaron's feedback in react_client
joebingham-wk Apr 4, 2019
ce84c7b
Merge branch 'CPLAT-3094-lifecycle-error-boundary' of github.com:clea…
joebingham-wk Apr 4, 2019
21c5c15
Implement the rest of Aaron's suggestions
joebingham-wk Apr 5, 2019
605f854
Modify test for dart2js
joebingham-wk Apr 5, 2019
019a681
Rollback react_test to remove SetState example
joebingham-wk Apr 5, 2019
28fae50
Update the name of componentInstance
joebingham-wk Apr 5, 2019
205320b
Remove generic from ComponentStatics2
joebingham-wk Apr 16, 2019
0a02c7d
Fix nits
joebingham-wk Apr 16, 2019
bc08d14
Address feedback
joebingham-wk Apr 16, 2019
284885d
Fix failing test
joebingham-wk Apr 16, 2019
26fadd6
Add additional test coverage
joebingham-wk Apr 16, 2019
f2e9ce7
Apply suggestions from code review
greglittlefield-wf Apr 17, 2019
9dad420
Add Greg's suggestions
joebingham-wk Apr 17, 2019
de1ce2e
Clean up example exception
joebingham-wk Apr 17, 2019
a5e08d9
Fix broken test
joebingham-wk Apr 17, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions example/test/react_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ void main() {
helloGreeter({'key': 'hello'}, []),
listComponent({'key': 'list'}, []),
component2TestComponent({'key': 'c2-list'}, []),
component2ErrorTestComponent({'key': 'error-boundary'}, []),
//clockComponent({"name": 'my-clock'}, []),
checkBoxComponent({'key': 'checkbox'}, [])
]),
Expand Down
96 changes: 96 additions & 0 deletions example/test/react_test_components.dart
Original file line number Diff line number Diff line change
Expand Up @@ -448,3 +448,99 @@ class _Component2TestComponent extends react.Component2 with react.TypedSnapshot

var newContextTypeConsumerComponentComponent = react.registerComponent(() => new _NewContextTypeConsumerComponent());
var component2TestComponent = react.registerComponent(() => new _Component2TestComponent());

class _ErrorComponent extends react.Component2 {
void componentDidMount() {
if (!props["errored"]) {
throw new _JoesException("It broke!", 2);
}
}

dynamic render() {
return react.div(
{'key': 'eb-d1-e'},
"Oh no, I'm an error! Check your "
"console.");
}
}

var ErrorComponent = react.registerComponent(() => new _ErrorComponent());

class _JoesException implements Exception {
int code;
String message;
String randoMessage;

_JoesException(this.message, this.code) {
switch (code) {
case 1:
randoMessage = "The code is a 1";
break;
case 2:
randoMessage = "The Code is a 2";
break;
default:
randoMessage = "Whaaaaaa";
}
}
}

class _Component2ErrorTestComponent extends react.Component2 {
Map getInitialState() {
return {
"clicked": false,
"errored": false,
"error": null,
};
}

void componentDidCatch(error, info) {
setState({
"error": "We can capture the error, store it in state and "
"display it here."
});
}

Map getDerivedStateFromError(error) {
return {"errored": true};
}

void error(event) {
setState({"clicked": true});
}

void clearError(event) {
setState({"clicked": false, "error": null, "errored": false});
}

dynamic render() {
dynamic errorMessage = state["error"] ?? "No error yet";

return react.div({
"key": "e-cont"
}, [
react.h3({"key": "e-header"}, "Error Boundary Test"),
state["clicked"] ? ErrorComponent({'key': 'ec-1', 'errored': state['errored']}) : null,
errorMessage != null ? react.div({'key': 'ec-m-1'}, '$errorMessage') : null,
!state["errored"]
? react.button({
'type': 'button',
'key': 'c3-r-button',
'className': 'btn btn-primary',
'onClick': error,
}, 'Trigger Error')
: null,
state["errored"]
? react.button({
'type': 'button',
'key': 'c3-c-button',
'className': 'btn btn-primary',
'onClick': clearError,
}, 'Clear Error')
: null,
react.hr({"key": "e-hr"}),
]);
}
}

var component2ErrorTestComponent = react.registerComponent(() => new _Component2ErrorTestComponent(), ['render']);
16 changes: 15 additions & 1 deletion js_src/_dart_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ function _createReactDartComponentClass(dartInteropStatics, componentStatics, js
return ReactDartComponent;
}

function _createReactDartComponentClass2(dartInteropStatics, componentStatics, jsConfig) {
function _createReactDartComponentClass2(dartInteropStatics, componentStatics, jsConfig, skipMethods) {
class ReactDartComponent2 extends React.Component {
constructor(props, context) {
super(props, context);
Expand All @@ -103,12 +103,26 @@ function _createReactDartComponentClass2(dartInteropStatics, componentStatics, j
componentWillUnmount() {
dartInteropStatics.handleComponentWillUnmount(this.dartComponent);
}
componentDidCatch(error, info) {
dartInteropStatics.handleComponentDidCatch(this.dartComponent, error, info);
}
static getDerivedStateFromError(error) {
return dartInteropStatics.handleGetDerivedStateFromError(componentStatics, error);
}
render() {
var result = dartInteropStatics.handleRender(this.dartComponent, this.props, this.state, this.context);
if (typeof result === 'undefined') result = null;
return result;
}
}
console.log(skipMethods);
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
// Delete methods that the user does not want to include (such as error boundary event).
skipMethods.forEach((method) => {
if (ReactDartComponent2[method] !== undefined) {
delete ReactDartComponent2[method];
}
});


if (jsConfig) {
if (jsConfig.contextType) {
Expand Down
40 changes: 40 additions & 0 deletions lib/react.dart
Original file line number Diff line number Diff line change
Expand Up @@ -714,6 +714,36 @@ abstract class Component2 implements Component {
/// See: <https://facebook.github.io/react/docs/react-component.html#unmounting-componentwillunmount>
void componentWillUnmount() {}

/// ReactJS lifecycle method that is invoked after an error is thrown by a
/// descendant.
///
/// Use this method primarily for logging errors, but because it takes
/// place after the commit phase side-effects are permitted.
///
/// __Note__: This method, along with [getDerivedStateFromError] will only
/// be called if `skipMethods` in [_registerComponent] is overridden with
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved
/// a list (which can be empty). Otherwise, in order to prevent every
/// component from being an error boundary, [componentDidCatch] and
/// [getDerivedStateFromError] will be ignored.
///
/// See: <https://facebook.github.io/react/docs/react-component.html#componentdidcatch>
void componentDidCatch(dynamic error, dynamic info) {}
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved

/// ReactJS lifecycle method that is invoked after an error is thrown by a
/// descendant.
///
/// Use this method to capture the error and update component state after an
/// error is thrown.
///
/// __Note__: This method, along with [componentDidCatch] will only
/// be called if `skipMethods` in [_registerComponent] is overridden with
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved
/// a list (which can be empty). Otherwise, in order to prevent every
/// component from being an error boundary, [componentDidCatch] and
/// [getDerivedStateFromError] will be ignored.
///
/// See: <https://facebook.github.io/react/docs/react-component.html#componentdidcatch>
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved
Map getDerivedStateFromError(dynamic error) {}

/// Invoked once before the `Component` is mounted. The return value will be used as the initial value of [state].
///
/// See: <https://facebook.github.io/react/docs/react-component.html#getinitialstate>
Expand Down Expand Up @@ -1461,6 +1491,16 @@ class SyntheticWheelEvent extends SyntheticEvent {
throw new Exception('setClientConfiguration must be called before registerComponent.');
};

/// Registers [componentFactory] on both client and server.
/*ComponentRegistrar*/ Function registerComponent2 = (/*ComponentFactory*/ componentFactory,
[/*Iterable<String>*/ skipMethods = const [
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
'getDerivedStateFromError',
'compo'
'nentDidCatch'
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved
]]) {
throw new Exception('setClientConfiguration must be called before registerComponent.');
};

/// The HTML `<a>` [AnchorElement].
var a;

Expand Down
16 changes: 15 additions & 1 deletion lib/react.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/react.js.map

Large diffs are not rendered by default.

64 changes: 59 additions & 5 deletions lib/react_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,44 @@ dynamic _convertArgsToChildren(List childrenArgs) {
}
}

/// Util used with [_registerComponent2] to ensure no imporant lifecycle
/// methods are skipped. This includes [shouldComponentUpdate],
/// [componentDidUpdate], and [render] because they utilize
/// [_updatePropsAndStateWithJs].
///
/// Returns the life of lifecycle methods to skip, having removed the
/// important ones. If an important lifecycle event was set for skipping, a
/// warning is issued.

joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved
List<String> _filterSkipMethods(List<String> methods) {
List<String> finalList = List.from(methods);
bool warn = false;

if (finalList.contains('shouldComponentUpdate')) {
finalList.remove('shouldComponentUpdate');
warn = true;
}

if (finalList.contains('componentDidUpdate')) {
finalList.remove('componentDidUpdate');
warn = true;
}

if (finalList.contains('render')) {
finalList.remove('render');
warn = true;
}

if (warn) {
window.console.warn("WARNING: Crucial lifecycle methods passed into "
"skipMethod. shouldComponentUpdate, componentDidUpdate, and render "
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved
"cannot be skipped and will still be added to the new component. Please "
"remove them from skipMethods.");
}

return finalList;
}

@JS('Object.keys')
external List<String> _objectKeys(Object object);

Expand Down Expand Up @@ -564,6 +602,14 @@ final ReactDartInteropStatics2 _dartInteropStatics2 = (() {
component.componentWillUnmount();
});

void handleComponentDidCatch(Component2 component, dynamic error, dynamic info) => zone.run(() {
component.componentDidCatch(error, info);
});
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved

JsMap handleGetDerivedStateFromError(ComponentStatics componentStatics, dynamic error) => zone.run(() {
return jsBackingMapOrJsCopy(componentStatics.componentInstance.getDerivedStateFromError(error));
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
});

dynamic handleRender(Component2 component, JsMap jsProps, JsMap jsState, dynamic jsContext) => zone.run(() {
_updatePropsAndStateWithJs(component, jsProps, jsState, jsContext);
return component.render();
Expand All @@ -579,6 +625,8 @@ final ReactDartInteropStatics2 _dartInteropStatics2 = (() {
handleGetSnapshotBeforeUpdate: allowInterop(handleGetSnapshotBeforeUpdate),
handleComponentDidUpdate: allowInterop(handleComponentDidUpdate),
handleComponentWillUnmount: allowInterop(handleComponentWillUnmount),
handleComponentDidCatch: allowInterop(handleComponentDidCatch),
handleGetDerivedStateFromError: allowInterop(handleGetDerivedStateFromError),
handleRender: allowInterop(handleRender),
);
})();
Expand All @@ -587,7 +635,7 @@ final ReactDartInteropStatics2 _dartInteropStatics2 = (() {
/// which produces a new JS [`ReactClass` component class](https://facebook.github.io/react/docs/top-level-api.html#react.createclass).
@Deprecated('6.0.0')
ReactDartComponentFactoryProxy _registerComponent(ComponentFactory componentFactory,
[Iterable<String> skipMethods = const []]) {
[Iterable<String> skipMethods = const ['getDerivedStateFromError', 'componentDidCatch']]) {
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
var componentInstance = componentFactory();

if (componentInstance is Component2) {
Expand Down Expand Up @@ -708,9 +756,14 @@ class ReactJsComponentFactoryProxy extends ReactComponentFactoryProxy {
/// Creates and returns a new [ReactDartComponentFactoryProxy] from the provided [componentFactory]
/// which produces a new JS [`ReactClass` component class](https://facebook.github.io/react/docs/top-level-api.html#react.createclass).
ReactDartComponentFactoryProxy2 _registerComponent2(ComponentFactory<Component2> componentFactory,
[Iterable<String> skipMethods = const []]) {
[Iterable<String> skipMethods = const [
'getDerivedStateFromError',
'compo'
'nentDidCatch'
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved
]]) {
final componentInstance = componentFactory();
final componentStatics = new ComponentStatics(componentFactory);
final componentStatics = new ComponentStatics(componentFactory, componentInstance: componentInstance);
final filteredSkipMethods = _filterSkipMethods(skipMethods);

// Cache default props and store them on the ReactClass so they can be used
// by ReactDartComponentFactoryProxy and externally.
Expand All @@ -723,8 +776,9 @@ ReactDartComponentFactoryProxy2 _registerComponent2(ComponentFactory<Component2>

/// Create the JS [`ReactClass` component class](https://facebook.github.io/react/docs/top-level-api.html#react.createclass)
/// with custom JS lifecycle methods.
var reactComponentClass = createReactDartComponentClass2(_dartInteropStatics2, componentStatics, jsConfig2)
..displayName = componentInstance.displayName;
var reactComponentClass =
createReactDartComponentClass2(_dartInteropStatics2, componentStatics, jsConfig2, filteredSkipMethods)
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved
..displayName = componentInstance.displayName;

reactComponentClass.dartComponentVersion = '2';

Expand Down
8 changes: 5 additions & 3 deletions lib/react_client/react_interop.dart
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ external ReactClass createReactDartComponentClass(
@JS('_createReactDartComponentClass2')
external ReactClass createReactDartComponentClass2(
ReactDartInteropStatics2 dartInteropStatics, ComponentStatics<Component2> componentStatics,
[JsComponentConfig2 jsConfig]);
[JsComponentConfig2 jsConfig, List<String> skipMethods]);

@JS('React.__isDevelopment')
external bool get _inReactDevMode;
Expand Down Expand Up @@ -411,6 +411,8 @@ class ReactDartInteropStatics2 implements ReactDartInteropStatics {
)
handleComponentDidUpdate,
void Function(Component2 component) handleComponentWillUnmount,
void Function(Component2 component, dynamic error, dynamic info) handleComponentDidCatch,
void Function(ComponentStatics componentInstance, dynamic error) handleGetDerivedStateFromError,
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved
dynamic Function(Component2 component, JsMap jsProps, JsMap jsState, dynamic jsContext) handleRender,
});
}
Expand All @@ -423,8 +425,8 @@ class ReactDartInteropStatics2 implements ReactDartInteropStatics {
/// See [ReactDartInteropStatics], [createReactDartComponentClass].
class ComponentStatics<T extends Component> {
joebingham-wk marked this conversation as resolved.
Show resolved Hide resolved
final ComponentFactory<T> componentFactory;

ComponentStatics(this.componentFactory);
final Component2 componentInstance;
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
ComponentStatics(this.componentFactory, {this.componentInstance});
aaronlademann-wf marked this conversation as resolved.
Show resolved Hide resolved
}

/// Additional configuration passed to [createReactDartComponentClass]
Expand Down
Loading