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-7979: Add SafeRenderManager #390

Merged
merged 5 commits into from
Oct 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion analysis_options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ analyzer:
linter:
rules:
- annotate_overrides
- avoid_as
- avoid_empty_else
- avoid_init_to_null
- avoid_return_types_on_setters
Expand All @@ -24,3 +23,4 @@ linter:
- type_init_formals
- unnecessary_brace_in_string_interps
- unnecessary_getters_setters
- unnecessary_statements
1 change: 1 addition & 0 deletions lib/over_react.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export 'src/util/react_util.dart';
export 'src/util/react_wrappers.dart';
export 'src/util/rem_util.dart';
export 'src/util/string_util.dart';
export 'src/util/safe_render_manager/safe_render_manager.dart';
export 'src/util/test_mode.dart';
export 'src/util/typed_default_props_for.dart';
export 'src/util/validation_util.dart';
224 changes: 224 additions & 0 deletions lib/src/util/safe_render_manager/safe_render_manager.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,224 @@
import 'dart:async';
import 'dart:html';

import 'package:meta/meta.dart';
import 'package:over_react/over_react.dart';
import 'package:over_react/react_dom.dart' as react_dom;
import 'package:w_common/disposable.dart';

import './safe_render_manager_helper.dart';

/// A class that manages the top-level rendering of a [ReactElement] into a given node,
/// with support for safely rendering/updating via [render] and safely unmounting via [tryUnmount].
///
/// Content is also unmounted when this object is [dispose]d.
///
/// This is useful in cases where [react_dom.render] or [react_dom.unmountComponentAtNode]
/// may or may not be called from React events or lifecycle methods, which can have
/// undesirable/unintended side effects.
///
/// For instance, calling [react_dom.unmountComponentAtNode] can unmount a component
/// while an event is being propagated through a component, which normally would never happen.
/// This could result in null errors in the component as the event logic continues.
///
/// SafeRenderManager uses a helper component under the hood to manage the rendering of content
/// via Component state changes, ensuring that the content is mounted/unmounted as it
/// normally would be.
class SafeRenderManager extends Disposable {
SafeRenderManagerHelperComponent _helper;

/// Whether to automatically add [mountNode] to the document body when
/// rendered, and remove it when unmounted.
///
/// Useful when manually managing the mount node isn't necessary.
final bool autoAttachMountNode;

/// The mount node for content rendered by [render].
///
/// If not specified, a new div will be used.
final Element mountNode;

/// The ref to the component rendered by [render].
///
/// Due to react_dom.render calls not being guaranteed to be synchronous.
/// this may not be populated until later than expected.
dynamic contentRef;

_RenderState _state = _RenderState.unmounted;

/// A list of [render] calls queued up while the component is in the process
/// of rendering.
List<ReactElement> _renderQueue = [];

SafeRenderManager({Element mountNode, this.autoAttachMountNode = false})
: mountNode = mountNode ?? new DivElement();

/// Renders [content]into [mountNode], chaining existing callback refs to
greglittlefield-wf marked this conversation as resolved.
Show resolved Hide resolved
/// provide access to the rendered component via [contentRef].
void render(ReactElement content) {
_checkDisposalState();

switch (_state) {
case _RenderState.mounting:
_renderQueue.add(content);
break;
case _RenderState.mountedOrErrored:
// Handle if _helper was unmounted due to an uncaught error.
if (_helper == null) {
_mountContent(content);
} else {
_helper.renderContent(content);
}
break;
case _RenderState.unmounted:
_mountContent(content);
break;
}
}

void _mountContent(ReactElement content) {
try {
_state = _RenderState.mounting;
// Use document.contains since `.isConnected` isn't supported in IE11.
if (autoAttachMountNode && !document.contains(mountNode)) {
document.body.append(mountNode);
}
react_dom.render((SafeRenderManagerHelper()
..ref = _helperRef
..getInitialContent = () {
final value = content;
// Clear this closure variable out so it isn't retained.
content = null;
return value;
}
..contentRef = _contentCallbackRef
)(), mountNode);
} catch (_) {
_state = _RenderState.unmounted;
rethrow;
}
}

/// Attempts to unmount the rendered component, calling [onMaybeUnmounted]
/// with whether the component was actually unmounted.
///
/// Unmounting could fail if a call to [render] is batched in with this
/// unmount during the propagation of this event. In that case, something
greglittlefield-wf marked this conversation as resolved.
Show resolved Hide resolved
/// other call wanted something rendered and trumped the unmount request.
///
/// This behavior allows the same SafeRenderManager instance to be used to
/// render/unmount a single content area without calls interfering with each
/// other.
///
/// If nothing is currently rendered, [onMaybeUnmounted] will be called immediately.
void tryUnmount({void onMaybeUnmounted(bool isUnmounted)}) {
// Check here since we call _tryUnmountContent in this class's disposal logic.
_checkDisposalState();
_safeUnmountContent(onMaybeUnmounted: onMaybeUnmounted, force: false);
}

void _unmountContent() {
try {
_state = _RenderState.unmounted;
_renderQueue = [];
react_dom.unmountComponentAtNode(mountNode);
} finally {
if (autoAttachMountNode) {
mountNode.remove();
}
}
}

void _safeUnmountContent(
{void onMaybeUnmounted(bool isUnmounted), @required bool force}) {
var _hasBeenCalled = false;
/// Helper to call onMaybeUnmounted at most one time, for cases
/// where there have to be error handlers at multiple levels
void callOnMaybeUnmounted(bool value) {
if (!_hasBeenCalled) {
_hasBeenCalled = true;
onMaybeUnmounted?.call(true);
}
}

if (_state == _RenderState.unmounted) {
callOnMaybeUnmounted(true);
} else if (_state == _RenderState.mountedOrErrored && _helper != null) {
try {
_helper.tryUnmountContent(onMaybeUnmounted: (isUnmounted) {
if (isUnmounted || force) {
try {
_unmountContent();
} finally {
callOnMaybeUnmounted(true);
}
} else {
callOnMaybeUnmounted(false);
}
});
} catch (_) {
// Handle _helper.tryUnmountContent throwing synchronously without
// calling onMaybeUnmounted.
// Don't do this in a finally since onMaybeUnmounted can get called async.
callOnMaybeUnmounted(true);
rethrow;
}
} else {
try {
_unmountContent();
} finally {
callOnMaybeUnmounted(true);
}
Comment on lines +162 to +171
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the comment on line 162 apply to line 170?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, since there isn't a onMaybeUnmounted callback being used here.

}
}

void _checkDisposalState() {
if (isOrWillBeDisposed) {
throw new ObjectDisposedException();
}
}

void _helperRef(ref) {
_helper = ref;
if (_helper != null) {
if (_state == _RenderState.mounting) {
_state = _RenderState.mountedOrErrored;
}
_renderQueue.forEach(_helper.renderContent);
_renderQueue = [];
}
}

void _contentCallbackRef(ref) {
contentRef = ref;
}

@override
Future<Null> onDispose() async {
var completer = new Completer<Null>();
final completerFuture = completer.future;

// Set up an onError handler in case onMaybeUnmounted isn't called due to
// an error, and an async error is thrown instead.
runZoned(() {
// Attempt to unmount the content safely
_safeUnmountContent(force: true, onMaybeUnmounted: (_) {
completer?.complete();
// Clear out to not retain it in the onError closure, which has
// an indefinitely long lifetime.
completer = null;
});
}, onError: (error, stackTrace) {
completer?.completeError(error, stackTrace);
// Clear out to not retain it in the onError closure, which has
// an indefinitely long lifetime.
completer = null;
});

await completerFuture;

await super.onDispose();
}
}

enum _RenderState { mounting, mountedOrErrored, unmounted }
70 changes: 70 additions & 0 deletions lib/src/util/safe_render_manager/safe_render_manager_helper.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import 'package:over_react/over_react.dart';

// ignore: uri_has_not_been_generated
part 'safe_render_manager_helper.over_react.g.dart';

/// A component that allows for safe unmounting of its single child by waiting for state changes
/// sometimes queued by ReactJS to be applied.
@Factory()
UiFactory<SafeRenderManagerHelperProps> SafeRenderManagerHelper =
// ignore: undefined_identifier
_$SafeRenderManagerHelper;

typedef ReactElement _GetInitialContent();

@Props()
class _$SafeRenderManagerHelperProps extends UiProps {
@requiredProp
_GetInitialContent getInitialContent;

CallbackRef contentRef;
}

@State()
class _$SafeRenderManagerHelperState extends UiState {
ReactElement content;
}

@Component()
class SafeRenderManagerHelperComponent extends UiStatefulComponent<SafeRenderManagerHelperProps, SafeRenderManagerHelperState> {
@override
getInitialState() => (newState()..content = props.getInitialContent());

void renderContent(ReactElement content) {
setState(newState()..content = content);
}

void tryUnmountContent({void onMaybeUnmounted(bool isUnmounted)}) {
greglittlefield-wf marked this conversation as resolved.
Show resolved Hide resolved
setState(newState()..content = null, () {
onMaybeUnmounted?.call(state.content == null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity - is it possible for state.content to be non-null within a callback of a call to setState that sets state.content to null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes; this can happen if this setState is batched with another setState that sets it to something else.

});
}

bool get hasContent => state.content != null;

@override
render() {
final content = state.content;
greglittlefield-wf marked this conversation as resolved.
Show resolved Hide resolved
if (content == null) return null;

return cloneElement(content, domProps()..ref = chainRef(content, _contentRef));
}

void _contentRef(ref) {
props.contentRef?.call(ref);
}
}

// AF-3369 This will be removed once the transition to Dart 2 is complete.
// ignore: mixin_of_non_class, undefined_class
class SafeRenderManagerHelperProps extends _$SafeRenderManagerHelperProps with _$SafeRenderManagerHelperPropsAccessorsMixin {
// ignore: undefined_identifier, undefined_class, const_initialized_with_non_constant_value
static const PropsMeta meta = _$metaForSafeRenderManagerHelperProps;
}

// AF-3369 This will be removed once the transition to Dart 2 is complete.
// ignore: mixin_of_non_class, undefined_class
class SafeRenderManagerHelperState extends _$SafeRenderManagerHelperState with _$SafeRenderManagerHelperStateAccessorsMixin {
// ignore: undefined_identifier, undefined_class, const_initialized_with_non_constant_value
static const StateMeta meta = _$metaForSafeRenderManagerHelperState;
}
Loading