Add forwardRef2/memo2 which fixes jsification of Dart props

.travis.yml

@@ -12,7 +12,10 @@ cache:
 
 before_script:
   - dartanalyzer .
-  - dartfmt --line-length=120 --dry-run --set-exit-if-changed .
+  - |
+    if [ "$TRAVIS_DART_VERSION" != "dev" ]; then
+      dartfmt --line-length=120 --dry-run --set-exit-if-changed .
+    fi
   - pub run dependency_validator -i build_runner,build_test,build_web_compilers
 
 script:

example/test/function_component_test.dart

@@ -68,7 +68,7 @@ class _MemoTestDemoWrapper extends react.Component2 {
   }
 }
 
-final MemoTest = react.memo(react.registerFunctionComponent((Map props) {
+final MemoTest = react.memo2(react.registerFunctionComponent((Map props) {
   final context = useContext(TestNewContext);
   return react.div(
     {},
@@ -335,7 +335,7 @@ class FancyInputApi {
   FancyInputApi(this.focus);
 }
 
-final FancyInput = react.forwardRef((props, ref) {
+final FancyInput = react.forwardRef2((props, ref) {
   final inputRef = useRef();
 
   useImperativeHandle(
@@ -412,7 +412,7 @@ UseImperativeHandleTestComponent(Map props) {
   ]);
 }
 
-final FancyCounter = react.forwardRef((props, ref) {
+final FancyCounter = react.forwardRef2((props, ref) {
   final count = useState(0);
 
   useImperativeHandle(

example/test/ref_test.dart

@@ -17,7 +17,7 @@ class _ChildComponent extends react.Component {
   render() => react.span({}, "Child element with value ${somevalue}");
 }
 
-var InputComponentForm = react.forwardRef((props, ref) {
+var InputComponentForm = react.forwardRef2((props, ref) {
   return react.form({
     'key': 'inputForm',
     'className': 'form-inline'
@@ -37,7 +37,7 @@ var InputComponentForm = react.forwardRef((props, ref) {
   ]);
 });
 
-var ChildComponentForm = react.forwardRef((props, ref) {
+var ChildComponentForm = react.forwardRef2((props, ref) {
   return react.Fragment({}, [
     react.h4({'key': 'create-child-h4'}, "ChildComponent"),
     react.form({
@@ -64,7 +64,7 @@ var ChildComponentForm = react.forwardRef((props, ref) {
       }, 'Increment child value'),
     ])
   ]);
-});
+}, displayName: 'ChildComponentForm');
 
 var ParentComponent = react.registerComponent(() => new _ParentComponent());
 

lib/hooks.dart

@@ -475,7 +475,7 @@ void useLayoutEffect(dynamic Function() sideEffect, [List<Object> dependencies])
   return React.useLayoutEffect(wrappedSideEffect, dependencies);
 }
 
-/// Customizes the [ref] value that is exposed to parent components when using [forwardRef] by setting [ref.current]
+/// Customizes the [ref] value that is exposed to parent components when using [forwardRef2] by setting `ref.current`
 /// to the return value of [createHandle].
 ///
 /// In most cases, imperative code using refs should be avoided.
@@ -494,7 +494,7 @@ void useLayoutEffect(dynamic Function() sideEffect, [List<Object> dependencies])
 ///   FancyInputApi(this.focus);
 /// }
 ///
-/// final FancyInput = react.forwardRef((props, ref) {
+/// final FancyInput = react.forwardRef2((props, ref) {
 ///   final inputRef = useRef<InputElement>();
 ///
 ///   useImperativeHandle(
@@ -529,10 +529,15 @@ void useLayoutEffect(dynamic Function() sideEffect, [List<Object> dependencies])
 /// ```
 ///
 /// Learn more: <https://reactjs.org/docs/hooks-reference.html#useimperativehandle>.
-void useImperativeHandle(Ref ref, dynamic Function() createHandle, [List<dynamic> dependencies]) {
-  if (ref == null) return;
-  React.useImperativeHandle(ref.jsRef, allowInterop(createHandle), dependencies);
-}
+void useImperativeHandle(dynamic ref, dynamic Function() createHandle, [List<dynamic> dependencies]) =>
+    // ref will be a JsRef in forwardRef2, or a Ref in forwardRef. (Or null if no ref is provided)
+    //
+    // For some reason the ref argument to React.forwardRef is usually a JsRef object no matter the input ref type,
+    // but according to React the ref argument to useImperativeHandle and React.forwardRef can be any ref type...
+    // - https://github.com/facebook/flow/blob/master@%7B2020-09-08%7D/lib/react.js#L373
+    // - https://github.com/facebook/flow/blob/master@%7B2020-09-08%7D/lib/react.js#L305
+    // and not just a ref object, so we type it as dynamic here.
+    React.useImperativeHandle(ref is Ref ? ref.jsRef : ref, allowInterop(createHandle), dependencies);
 
 /// Displays [value] as a label for a custom hook in React DevTools.
 ///

lib/react.dart

@@ -21,17 +21,26 @@ import 'package:react/src/react_client/private_utils.dart' show validateJsApiThe
 
 export 'package:react/src/context.dart';
 export 'package:react/src/prop_validator.dart';
-export 'package:react/react_client/react_interop.dart' show forwardRef, createRef, memo;
+export 'package:react/react_client/react_interop.dart' show forwardRef, forwardRef2, createRef, memo, memo2;
 
 typedef Error PropValidator<TProps>(TProps props, PropValidatorInfo info);
 
 /// A React component declared using a function that takes in [props] and returns rendered output.
 ///
 /// See <https://facebook.github.io/react/docs/components-and-props.html#functional-and-class-components>.
 ///
-/// [props] is typed as [JsBackedMap] so that dart2js can make optimize props accesses.
+/// [props] is typed as [JsBackedMap] so that dart2js can optimize props accesses.
 typedef DartFunctionComponent = dynamic Function(JsBackedMap props);
 
+/// The callback to a React forwardRef component. See [forwardRef2] for more details.
+///
+/// [props] is typed as [JsBackedMap] so that dart2js can optimize props accesses.
+///
+/// In the current JS implementation, the ref argument to [React.forwardRef] is usually a JsRef object no matter the input ref type,
+/// but according to React the ref argument can be any ref type: https://github.com/facebook/flow/blob/master@%7B2020-09-08%7D/lib/react.js#L305
+/// and not just a ref object, so we type [ref] as dynamic here.
+typedef DartForwardRefFunctionComponent = dynamic Function(JsBackedMap props, dynamic ref);
+
 typedef T ComponentFactory<T extends Component>();
 
 typedef ReactComponentFactoryProxy ComponentRegistrar(ComponentFactory componentFactory,

lib/react_client/component_factory.dart

@@ -14,10 +14,11 @@ import 'package:react/src/context.dart';
 import 'package:react/src/ddc_emulated_function_name_bug.dart' as ddc_emulated_function_name_bug;
 import 'package:react/src/js_interop_util.dart';
 import 'package:react/src/typedefs.dart';
-import 'package:react/src/react_client/event_factory.dart';
 import 'package:react/src/react_client/event_prop_key_to_event_factory.dart';
 import 'package:react/src/react_client/factory_util.dart';
 
+export 'package:react/src/react_client/factory_util.dart' show unconvertJsEventHandler;
+
 /// Prepares [children] to be passed to the ReactJS [React.createElement] and
 /// the Dart [react.Component].
 ///
@@ -36,20 +37,6 @@ dynamic listifyChildren(dynamic children) {
   }
 }
 
-/// Returns the original Dart handler function that, within [_convertEventHandlers],
-/// was converted/wrapped into the function [jsConvertedEventHandler] to be passed to the JS.
-///
-/// Returns `null` if [jsConvertedEventHandler] is `null`.
-///
-/// Returns `null` if [jsConvertedEventHandler] does not represent such a function
-///
-/// Useful for chaining event handlers on DOM or JS composite [ReactElement]s.
-Function unconvertJsEventHandler(Function jsConvertedEventHandler) {
-  if (jsConvertedEventHandler == null) return null;
-
-  return originalEventHandlers[jsConvertedEventHandler];
-}
-
 /// Returns the props for a [ReactElement] or composite [ReactComponent] [instance],
 /// shallow-converted to a Dart Map for convenience.
 ///
@@ -172,10 +159,14 @@ class ReactDartComponentFactoryProxy<TComponent extends Component> extends React
       // would fail the `is CallbackRef<dynamic>` check.
       // See https://github.com/dart-lang/sdk/issues/34593 for more information on arity checks.
       if (ref is CallbackRef<Null>) {
-        interopProps.ref = allowInterop((ReactComponent instance) {
+        interopProps.ref = allowInterop((dynamic instance) {
           // Call as dynamic to perform dynamic dispatch, since we can't cast to CallbackRef<dynamic>,
           // and since calling with non-null values will fail at runtime due to the CallbackRef<Null> typing.
-          return (ref as dynamic)(instance?.dartComponent);
+          if (instance is ReactComponent && instance.dartComponent != null) {
+            return (ref as dynamic)(instance.dartComponent);
+          }
+
+          return (ref as dynamic)(instance);
         });
       } else if (ref is Ref) {
         interopProps.ref = ref.jsRef;
@@ -369,34 +360,71 @@ class ReactDartFunctionComponentFactoryProxy extends ReactComponentFactoryProxy
 
   @override
   JsFunctionComponent get type => reactFunction;
+}
 
-  static String _getJsFunctionName(Function object) =>
-      getProperty(object, 'name') ?? getProperty(object, '\$static_name');
+/// A wrapper for a Dart component that's been wrapped by a JS component/HOC (e.g., [React.memo], [React.forwardRef]).
+class ReactDartWrappedComponentFactoryProxy extends ReactComponentFactoryProxy with JsBackedMapComponentFactoryMixin {
+  /// The React JS component definition of this Function Component.
+  @override
+  final ReactClass type;
 
-  /// Creates a function component from the given [dartFunctionComponent] that can be used with React.
-  ///
-  /// [displayName] Sets the component name for debugging purposes.
-  ///
-  /// In DDC, this will be the [DartFunctionComponent] name, but in dart2js it will be null unless
-  /// overridden, since using runtimeType can lead to larger dart2js output.
-  ///
-  /// This will result in the dart2js name being `ReactDartComponent2` (the
-  /// name of the proxying JS component defined in _dart_helpers.js).
-  static JsFunctionComponent _wrapFunctionComponent(DartFunctionComponent dartFunctionComponent, {String displayName}) {
-    // dart2js uses null and undefined interchangeably, meaning returning `null` from dart
-    // may show up in js as `undefined`, ReactJS doesnt like that and expects a js `null` to be returned,
-    // and throws if it gets `undefined`. `jsNull` is an interop variable that holds a JS `null` value
-    // to force `null` as the return value if user returns a Dart `null`.
-    // See: https://github.com/dart-lang/sdk/issues/27485
-    jsFunctionComponent(JsMap jsProps, [JsMap _legacyContext]) =>
-        componentZone.run(() => dartFunctionComponent(JsBackedMap.backedBy(jsProps)) ?? jsNull);
-    JsFunctionComponent interopFunction = allowInterop(jsFunctionComponent);
-    if (displayName != null) {
-      // This is a work-around to display the correct name in the React DevTools.
-      defineProperty(interopFunction, 'name', jsify({'value': displayName}));
-    }
-    // ignore: invalid_use_of_protected_member
-    setProperty(interopFunction, 'dartComponentVersion', ReactDartComponentVersion.component2);
-    return interopFunction;
+  ReactDartWrappedComponentFactoryProxy(this.type);
+
+  ReactDartWrappedComponentFactoryProxy.forwardRef(DartForwardRefFunctionComponent dartFunctionComponent,
+      {String displayName})
+      : this.type = _wrapForwardRefFunctionComponent(dartFunctionComponent,
+            displayName: displayName ?? _getJsFunctionName(dartFunctionComponent));
+}
+
+String _getJsFunctionName(Function object) => getProperty(object, 'name') ?? getProperty(object, '\$static_name');
+
+/// Creates a function component from the given [dartFunctionComponent] that can be used with React.
+///
+/// [displayName] Sets the component name for debugging purposes.
+///
+/// In DDC, this will be the [DartFunctionComponent] name, but in dart2js it will be null unless
+/// overridden, since using runtimeType can lead to larger dart2js output.
+JsFunctionComponent _wrapFunctionComponent(DartFunctionComponent dartFunctionComponent, {String displayName}) {
+  // dart2js uses null and undefined interchangeably, meaning returning `null` from dart
+  // may show up in js as `undefined`, ReactJS doesnt like that and expects a js `null` to be returned,
+  // and throws if it gets `undefined`. `jsNull` is an interop variable that holds a JS `null` value
+  // to force `null` as the return value if user returns a Dart `null`.
+  // See: https://github.com/dart-lang/sdk/issues/27485
+  jsFunctionComponent(JsMap jsProps, [JsMap _legacyContext]) =>
+      componentZone.run(() => dartFunctionComponent(JsBackedMap.backedBy(jsProps)) ?? jsNull);
+  JsFunctionComponent interopFunction = allowInterop(jsFunctionComponent);
+  if (displayName != null) {
+    // This is a work-around to display the correct name in the React DevTools.
+    defineProperty(interopFunction, 'name', jsify({'value': displayName}));
+  }
+  // ignore: invalid_use_of_protected_member
+  setProperty(interopFunction, 'dartComponentVersion', ReactDartComponentVersion.component2);
+  return interopFunction;
+}
+
+/// Creates a function component from the given [dartFunctionComponent] that can be used with React.
+///
+/// [displayName] Sets the component name for debugging purposes.
+///
+/// In DDC, this will be the [DartFunctionComponent] name, but in dart2js it will be null unless
+/// overridden, since using runtimeType can lead to larger dart2js output.
+ReactClass _wrapForwardRefFunctionComponent(DartForwardRefFunctionComponent dartFunctionComponent,
+    {String displayName}) {
+  // dart2js uses null and undefined interchangeably, meaning returning `null` from dart
+  // may show up in js as `undefined`, ReactJS doesnt like that and expects a js `null` to be returned,
+  // and throws if it gets `undefined`. `jsNull` is an interop variable that holds a JS `null` value
+  // to force `null` as the return value if user returns a Dart `null`.
+  // See: https://github.com/dart-lang/sdk/issues/27485
+  jsFunctionComponent(JsMap props, dynamic ref) =>
+      componentZone.run(() => dartFunctionComponent(JsBackedMap.backedBy(props), ref) ?? jsNull);
+
+  final interopFunction = allowInterop(jsFunctionComponent);
+  if (displayName != null) {
+    // This is a work-around to display the correct name in the React DevTools.
+    defineProperty(interopFunction, 'name', jsify({'value': displayName}));
   }
+  final jsForwardRefFunction = React.forwardRef(interopFunction);
+  // ignore: invalid_use_of_protected_member
+  setProperty(jsForwardRefFunction, 'dartComponentVersion', ReactDartComponentVersion.component2);
+  return jsForwardRefFunction;
 }