Merge pull request #733 from Workiva/saferendermanager-retention-workaround

SafeRenderManager retention workaround

Author: rmconsole6-wk

lib/src/util/safe_render_manager/safe_render_manager.dart

@@ -52,20 +52,39 @@ class SafeRenderManager extends Disposable {
   /// If not specified, a new div will be used.
   final Element mountNode;
 
+  final _contentRefObject = createRef<dynamic>();
+
   /// 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;
+  dynamic get contentRef => _contentRefObject.current;
+
+  // This setter only exists for backwards compatibility purposes.
+  @Deprecated('This ref should only be read, since it gets set by the rendered content')
+  set contentRef(dynamic value) => _contentRefObject.current = value;
 
   _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 ?? DivElement();
+  /// A ref to `this`, allowing us to create closures with references to this object that can be "revoked",
+  /// so that any trees retained before React clears out old children https://github.com/facebook/react/issues/16087
+  /// don't retain a reference to this object.
+  ///
+  /// In those cases, usually it's only SafeRenderManagerHelper/SafeRenderManager that get retained,
+  /// potentially since SafeRenderManagerHelper's tryUnmountContent plus the actual unmount are enough for React to
+  /// clear out old children.
+  ///
+  /// This is mostly in place to ease automated memory testing by preventing SafeRenderManager and its LeakFlags from
+  /// being retained; any content retained by React will still be able to flag leaks.
+  final _selfRef = createRef<SafeRenderManager>();
+
+  SafeRenderManager({Element mountNode, this.autoAttachMountNode = false}) : mountNode = mountNode ?? DivElement() {
+    _selfRef.current = this;
+  }
 
   /// Renders [content] into [mountNode], chaining existing callback refs to
   /// provide access to the rendered component via [contentRef].
@@ -97,15 +116,19 @@ class SafeRenderManager extends Disposable {
       if (autoAttachMountNode && !document.contains(mountNode)) {
         document.body.append(mountNode);
       }
+      // Create a closure variable so we're not retaining a reference to `this` inside `ref`.
+      final _selfRef = this._selfRef;
       react_dom.render((SafeRenderManagerHelper()
-        ..ref = _helperRef
+        ..ref = (ref) {
+          _selfRef.current?._helperRef(ref);
+        }
         ..getInitialContent = () {
           final value = content;
           // Clear this closure variable out so it isn't retained.
           content = null;
           return value;
         }
-        ..contentRef = _contentCallbackRef
+        ..contentRef = _contentRefObject
       )(), mountNode);
     } catch (_) {
       _state = _RenderState.unmounted;
@@ -203,10 +226,6 @@ class SafeRenderManager extends Disposable {
     }
   }
 
-  void _contentCallbackRef(ref) {
-    contentRef = ref;
-  }
-
   @override
   Future<Null> onDispose() async {
     var completer = Completer<Null>();
@@ -232,6 +251,9 @@ class SafeRenderManager extends Disposable {
 
     await completerFuture;
 
+    // Prevent any closures retaining `_selfRef` from retaining `this`.
+    _selfRef.current = null;
+
     await super.onDispose();
   }
 }

lib/src/util/safe_render_manager/safe_render_manager_helper.dart

@@ -18,21 +18,18 @@ 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;
+    castUiFactory(_$SafeRenderManagerHelper);
 
-@Props()
-class _$SafeRenderManagerHelperProps extends UiProps {
+mixin SafeRenderManagerHelperProps on UiProps {
   @requiredProp
   ReactElement Function() getInitialContent;
 
-  CallbackRef contentRef;
+  dynamic contentRef;
 }
 
-@State()
-class _$SafeRenderManagerHelperState extends UiState {
+mixin SafeRenderManagerHelperState on UiState {
   ReactElement content;
 }
 
@@ -57,24 +54,6 @@ class SafeRenderManagerHelperComponent extends UiStatefulComponent2<SafeRenderMa
   render() {
     if (state.content == null) return null;
 
-    return cloneElement(state.content, domProps()..ref = chainRef(state.content, _contentRef));
+    return cloneElement(state.content, domProps()..ref = chainRefs(state.content.ref, props.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;
 }