diff --git a/.travis.yml b/.travis.yml index 43371929..ed4b8ad4 100644 --- a/.travis.yml +++ b/.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: diff --git a/example/test/function_component_test.dart b/example/test/function_component_test.dart index 6427d85e..60326dc9 100644 --- a/example/test/function_component_test.dart +++ b/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( diff --git a/example/test/ref_test.dart b/example/test/ref_test.dart index 6f02d3d4..690c0832 100644 --- a/example/test/ref_test.dart +++ b/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()); diff --git a/lib/hooks.dart b/lib/hooks.dart index 3b60cd1f..9fe1df67 100644 --- a/lib/hooks.dart +++ b/lib/hooks.dart @@ -475,7 +475,7 @@ void useLayoutEffect(dynamic Function() sideEffect, [List 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 dependencies]) /// FancyInputApi(this.focus); /// } /// -/// final FancyInput = react.forwardRef((props, ref) { +/// final FancyInput = react.forwardRef2((props, ref) { /// final inputRef = useRef(); /// /// useImperativeHandle( @@ -529,10 +529,15 @@ void useLayoutEffect(dynamic Function() sideEffect, [List dependencies]) /// ``` /// /// Learn more: . -void useImperativeHandle(Ref ref, dynamic Function() createHandle, [List dependencies]) { - if (ref == null) return; - React.useImperativeHandle(ref.jsRef, allowInterop(createHandle), dependencies); -} +void useImperativeHandle(dynamic ref, dynamic Function() createHandle, [List 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. /// diff --git a/lib/react.dart b/lib/react.dart index bd1a3b51..55130419 100644 --- a/lib/react.dart +++ b/lib/react.dart @@ -21,7 +21,7 @@ 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 props, PropValidatorInfo info); @@ -29,9 +29,18 @@ typedef Error PropValidator(TProps props, PropValidatorInfo info); /// /// See . /// -/// [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(); typedef ReactComponentFactoryProxy ComponentRegistrar(ComponentFactory componentFactory, diff --git a/lib/react_client/component_factory.dart b/lib/react_client/component_factory.dart index fee9628c..12198dbc 100644 --- a/lib/react_client/component_factory.dart +++ b/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 extends React // would fail the `is CallbackRef` check. // See https://github.com/dart-lang/sdk/issues/34593 for more information on arity checks. if (ref is CallbackRef) { - interopProps.ref = allowInterop((ReactComponent instance) { + interopProps.ref = allowInterop((dynamic instance) { // Call as dynamic to perform dynamic dispatch, since we can't cast to CallbackRef, // and since calling with non-null values will fail at runtime due to the CallbackRef 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; } diff --git a/lib/react_client/react_interop.dart b/lib/react_client/react_interop.dart index adb20e21..f318a54d 100644 --- a/lib/react_client/react_interop.dart +++ b/lib/react_client/react_interop.dart @@ -19,7 +19,7 @@ import 'package:react/react_client.dart' show ComponentFactory; import 'package:react/react_client/bridge.dart'; import 'package:react/react_client/js_backed_map.dart'; import 'package:react/react_client/component_factory.dart' - show ReactDartFunctionComponentFactoryProxy, ReactJsComponentFactoryProxy; + show ReactDartWrappedComponentFactoryProxy, ReactDartFunctionComponentFactoryProxy, ReactJsComponentFactoryProxy; import 'package:react/react_client/js_interop_helpers.dart'; import 'package:react/react_client/zone.dart'; import 'package:react/src/js_interop_util.dart'; @@ -47,9 +47,9 @@ abstract class React { external static ReactJsComponentFactory createFactory(type); external static ReactElement createElement(dynamic type, props, [dynamic children]); external static JsRef createRef(); - external static ReactClass forwardRef(Function(JsMap props, JsRef ref) wrapperFunction); + external static ReactClass forwardRef(Function(JsMap props, dynamic ref) wrapperFunction); external static ReactClass memo( - dynamic Function(JsMap props, [JsMap legacyContext]) wrapperFunction, [ + dynamic wrapperFunction, [ bool Function(JsMap prevProps, JsMap nextProps) areEqual, ]); @@ -66,7 +66,7 @@ abstract class React { external static JsRef useRef([dynamic initialValue]); external static dynamic useMemo(dynamic Function() createFunction, [List dependencies]); external static void useLayoutEffect(dynamic Function() sideEffect, [List dependencies]); - external static void useImperativeHandle(JsRef ref, dynamic Function() createHandle, [List dependencies]); + external static void useImperativeHandle(dynamic ref, dynamic Function() createHandle, [List dependencies]); // NOTE: The use of generics on the `useDebugValue` interop will break the hook. external static dynamic useDebugValue(dynamic value, [Function format]); } @@ -159,7 +159,7 @@ class JsRef { /// import 'package:react/react.dart' as react; /// /// // ---------- Component Definition ---------- -/// final FancyButton = react.forwardRef((props, ref) { +/// final FancyButton = react.forwardRef2((props, ref) { /// return react.button({'ref': ref, 'className': 'FancyButton'}, 'Click me!'); /// }, displayName: 'FancyButton'); /// @@ -186,7 +186,7 @@ class JsRef { /// /// // ---------- Component Definitions ---------- /// -/// final FancyButton = react.forwardRef((props, ref) { +/// final FancyButton = react.forwardRef2((props, ref) { /// return react.button({'ref': ref, 'className': 'FancyButton'}, 'Click me!'); /// }, displayName: 'FancyButton'); /// @@ -207,8 +207,8 @@ class JsRef { /// } /// final _logPropsHoc = react.registerComponent2(() => _LogProps()); /// -/// final LogProps = react.forwardRef((props, ref) { -/// // Note the second param "ref" provided by react.forwardRef. +/// final LogProps = react.forwardRef2((props, ref) { +/// // Note the second param "ref" provided by react.forwardRef2. /// // We can pass it along to LogProps as a regular prop, e.g. "forwardedRef" /// // And it can then be attached to the Component. /// return _logPropsHoc({...props, 'forwardedRef': ref}); @@ -229,6 +229,13 @@ class JsRef { /// } /// ``` /// See: . +ReactComponentFactoryProxy forwardRef2( + DartForwardRefFunctionComponent wrapperFunction, { + String displayName, +}) => + ReactDartWrappedComponentFactoryProxy.forwardRef(wrapperFunction, displayName: displayName); + +@Deprecated('Use forwardRef2') ReactJsComponentFactoryProxy forwardRef( Function(Map props, Ref ref) wrapperFunction, { String displayName = 'Anonymous', @@ -266,12 +273,12 @@ ReactJsComponentFactoryProxy forwardRef( /// ```dart /// import 'package:react/react.dart' as react; /// -/// final MyComponent = react.memo(react.registerFunctionComponent((props) { +/// final MyComponent = react.memo2(react.registerFunctionComponent((props) { /// /* render using props */ /// })); /// ``` /// -/// `memo` only affects props changes. If your function component wrapped in `memo` has a +/// `memo2` only affects props changes. If your function component wrapped in `memo2` has a /// [useState] or [useContext] Hook in its implementation, it will still rerender when `state` or `context` change. /// /// By default it will only shallowly compare complex objects in the props map. @@ -281,7 +288,7 @@ ReactJsComponentFactoryProxy forwardRef( /// ```dart /// import 'package:react/react.dart' as react; /// -/// final MyComponent = react.memo(react.registerFunctionComponent((props) { +/// final MyComponent = react.memo2(react.registerFunctionComponent((props) { /// // render using props /// }), areEqual: (prevProps, nextProps) { /// // Do some custom comparison logic to return a bool based on prevProps / nextProps @@ -293,6 +300,22 @@ ReactJsComponentFactoryProxy forwardRef( /// > Do not rely on it to “prevent” a render, as this can lead to bugs. /// /// See: . +ReactComponentFactoryProxy memo2(ReactComponentFactoryProxy factory, + {bool Function(Map prevProps, Map nextProps) areEqual}) { + final _areEqual = areEqual == null + ? null + : allowInterop((JsMap prevProps, JsMap nextProps) { + final dartPrevProps = JsBackedMap.backedBy(prevProps); + final dartNextProps = JsBackedMap.backedBy(nextProps); + return areEqual(dartPrevProps, dartNextProps); + }); + final hoc = React.memo(factory.type, _areEqual); + setProperty(hoc, 'dartComponentVersion', ReactDartComponentVersion.component2); + + return ReactDartWrappedComponentFactoryProxy(hoc); +} + +@Deprecated('Use memo2') ReactJsComponentFactoryProxy memo(ReactDartFunctionComponentFactoryProxy factory, {bool Function(Map prevProps, Map nextProps) areEqual}) { final _areEqual = areEqual == null diff --git a/lib/src/react_client/chain_refs.dart b/lib/src/react_client/chain_refs.dart index 0e741394..1a4387cf 100644 --- a/lib/src/react_client/chain_refs.dart +++ b/lib/src/react_client/chain_refs.dart @@ -1,4 +1,5 @@ import 'package:js/js_util.dart'; +import 'package:react/react.dart' as react; import 'package:react/react_client/react_interop.dart'; /// Returns a ref that updates both [ref1] and [ref2], effectively @@ -73,8 +74,8 @@ dynamic chainRefList(List refs) { // ignore: invalid_use_of_protected_member ref.current = value; } else { - // ignore: invalid_use_of_protected_member - (ref as JsRef).current = value; + // ignore: invalid_use_of_protected_member, deprecated_member_use_from_same_package + (ref as JsRef).current = value is react.Component ? value.jsThis : value; } } } diff --git a/lib/src/react_client/dart_interop_statics.dart b/lib/src/react_client/dart_interop_statics.dart index 8e5a22bf..cc43fa53 100644 --- a/lib/src/react_client/dart_interop_statics.dart +++ b/lib/src/react_client/dart_interop_statics.dart @@ -33,8 +33,9 @@ final ReactDartInteropStatics dartInteropStatics = (() { var ref = getProperty(jsThis.refs, name); if (ref == null) return null; if (ref is Element) return ref; + if (ref is ReactComponent) return ref.dartComponent ?? ref; - return (ref as ReactComponent).dartComponent ?? ref; + return ref; }; Component component = componentStatics.componentFactory() diff --git a/lib/src/react_client/event_factory.dart b/lib/src/react_client/event_factory.dart index fd48f35f..2f2ffcda 100644 --- a/lib/src/react_client/event_factory.dart +++ b/lib/src/react_client/event_factory.dart @@ -3,10 +3,6 @@ import 'dart:html'; import 'package:react/react.dart'; import 'package:react/src/react_client/synthetic_event_wrappers.dart' as events; -/// A mapping from converted/wrapped JS handler functions (the result of [_convertEventHandlers]) -/// to the original Dart functions (the input of [_convertEventHandlers]). -final Expando originalEventHandlers = new Expando(); - /// Wrapper for [SyntheticEvent]. SyntheticEvent syntheticEventFactory(events.SyntheticEvent e) { return new SyntheticEvent(e.bubbles, e.cancelable, e.currentTarget, e.defaultPrevented, () => e.preventDefault(), diff --git a/lib/src/react_client/factory_util.dart b/lib/src/react_client/factory_util.dart index 9fad7d5e..eeb63185 100644 --- a/lib/src/react_client/factory_util.dart +++ b/lib/src/react_client/factory_util.dart @@ -11,7 +11,6 @@ import 'package:react/react_client/js_backed_map.dart'; import 'package:react/react_client/js_interop_helpers.dart'; import 'package:react/react_client/react_interop.dart'; -import 'package:react/src/react_client/event_factory.dart'; import 'package:react/src/react_client/synthetic_event_wrappers.dart' as events; import 'package:react/src/typedefs.dart'; @@ -35,6 +34,10 @@ dynamic convertArgsToChildren(List childrenArgs) { } } +/// A mapping from converted/wrapped JS handler functions (the result of [_convertEventHandlers]) +/// to the original Dart functions (the input of [_convertEventHandlers]). +final Expando originalEventHandler = new Expando(); + /// Convert packed event handler into wrapper and pass it only the Dart [SyntheticEvent] object converted from the /// [events.SyntheticEvent] event. convertEventHandlers(Map args) { @@ -59,12 +62,26 @@ convertEventHandlers(Map args) { }); args[propKey] = reactDartConvertedEventHandler; - originalEventHandlers[reactDartConvertedEventHandler] = value; + originalEventHandler[reactDartConvertedEventHandler] = value; } } }); } +/// 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 originalEventHandler[jsConvertedEventHandler]; +} + void convertRefValue(Map args) { var ref = args['ref']; if (ref is Ref) { diff --git a/lib/src/typedefs.dart b/lib/src/typedefs.dart index 74e8047f..0f7f0e48 100644 --- a/lib/src/typedefs.dart +++ b/lib/src/typedefs.dart @@ -15,6 +15,8 @@ typedef CallbackRef(T componentOrDomNode); /// to match Dart's generated call signature based on the number of args React provides. typedef JsFunctionComponent = dynamic Function(JsMap props, [JsMap legacyContext]); +typedef JsForwardRefFunctionComponent = dynamic Function(JsMap props, dynamic ref); + /// Typedef for `react.Component.ref`, which should return one of the following specified by the provided [ref]: /// /// * `react.Component` if it is a Dart component. diff --git a/pubspec.yaml b/pubspec.yaml index 2dfdf90d..4800fd3d 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -12,7 +12,7 @@ authors: description: Bindings of the ReactJS library for building interactive interfaces. homepage: https://github.com/cleandart/react-dart environment: - sdk: '>=2.4.0 <3.0.0' + sdk: '>=2.7.0 <3.0.0' dependencies: js: ^0.6.0 meta: ^1.1.6 diff --git a/test/factory/common_factory_tests.dart b/test/factory/common_factory_tests.dart index 541787d1..a8a42929 100644 --- a/test/factory/common_factory_tests.dart +++ b/test/factory/common_factory_tests.dart @@ -20,6 +20,7 @@ import 'package:react/react_dom.dart' as react_dom; import 'package:react/react_test_utils.dart' as rtu; import 'package:react/react_client/react_interop.dart'; +import '../shared_type_tester.dart'; import '../util.dart'; /// Runs common tests for [factory]. @@ -27,7 +28,7 @@ import '../util.dart'; /// [dartComponentVersion] should be specified for all components with Dart render code in order to /// properly test `props.children`, forwardRef compatibility, etc. void commonFactoryTests(ReactComponentFactoryProxy factory, - {bool isFunctionComponent = false, String dartComponentVersion}) { + {String dartComponentVersion, bool skipPropValuesTest = false}) { _childKeyWarningTests( factory, renderWithUniqueOwnerName: _renderWithUniqueOwnerName, @@ -140,6 +141,36 @@ void commonFactoryTests(ReactComponentFactoryProxy factory, }), ); }); + + if (!isDartComponent1(factory({}))) { + test('passes through props as a JsBackedMap:', () { + dynamic receivedProps; + rtu.renderIntoDocument(factory({ + 'onDartRender': (Map props) { + receivedProps = props; + } + })); + + expect(receivedProps, isA(), reason: 'props should be a JsBackedMap'); + }); + } + + if (!skipPropValuesTest) { + group('does not convert/wrap values for interop:', () { + sharedTypeTests((dynamic testValue) { + dynamic receivedValue; + + rtu.renderIntoDocument(factory({ + 'testValue': testValue, + 'onDartRender': (Map props) { + receivedValue = props['testValue']; + } + })); + + expect(receivedValue, same(testValue)); + }); + }); + } } if (isDartComponent2(factory({}))) { @@ -185,18 +216,80 @@ void domEventHandlerWrappingTests(ReactComponentFactoryProxy factory) { 'renders a single DOM node and passes props.onClick to it'); }); - test('wraps the handler with a function that converts the synthetic event', () { - var actualEvent; + group('wraps event handlers properly,', () { + const dart = EventTestCase.dart('onMouseUp', 'set on component'); + const dartCloned = EventTestCase.dart('onMouseLeave', 'cloned onto component'); + const jsCloned = EventTestCase.js('onClick', 'cloned onto component '); + const eventCases = {dart, jsCloned, dartCloned}; - final nodeWithClickHandler = renderAndGetRootNode(factory({ - 'onClick': (event) { - actualEvent = event; + Element node; + Map events; + Map propsFromDartRender; + + setUpAll(() { + expect(eventCases.map((h) => h.eventPropKey).toSet(), hasLength(eventCases.length), + reason: 'test setup: each helper should have a unique event key'); + + node = null; + events = {}; + propsFromDartRender = null; + + var element = factory({ + 'onDartRender': (p) { + propsFromDartRender = p; + }, + dart.eventPropKey: (event) => events[dart] = event, + }); + + // Simulate a JS component cloning a handler onto a ReactElement created by a Dart component. + element = React.cloneElement( + element, + jsifyAndAllowInterop({ + jsCloned.eventPropKey: (event) => events[jsCloned] = event, + }), + ); + + element = React.cloneElement( + element, + // Invoke the factory corresponding to element's type + // to get the correct version of the handler (converted or non-converted) + // before passing it straight to the JS. + factory({ + dartCloned.eventPropKey: (event) => events[dartCloned] = event, + }).props, + ); + + node = renderAndGetRootNode(element); + }); + + group('passing Dart events to Dart handlers, and JS events to handlers originating from JS:', () { + for (var eventCase in eventCases) { + test(eventCase.description, () { + eventCase.simulate(node); + expect(events[eventCase], isNotNull, reason: 'handler should have been called'); + expect(events[eventCase], + eventCase.isDart ? isA() : isNot(isA())); + }); } - })); + }); - rtu.Simulate.click(nodeWithClickHandler); + if (isDartComponent(factory({}))) { + group('in a way that the handlers are callable from within the Dart component:', () { + setUpAll(() { + expect(propsFromDartRender, isNotNull, + reason: 'test setup: component must pass props into props.onDartRender'); + }); - expect(actualEvent, isA()); + final dummyEvent = react.SyntheticMouseEvent(null, null, null, null, null, null, null, null, null, null, null, + null, null, null, null, null, null, null, null, null, null, null, null, null, null, null); + + for (var eventCase in eventCases.where((helper) => helper.isDart)) { + test(eventCase.description, () { + expect(() => propsFromDartRender[eventCase.eventPropKey](dummyEvent), returnsNormally); + }); + } + }); + } }); group('wraps the handler with a function that proxies ReactJS event "persistence" as expected', () { @@ -290,244 +383,189 @@ void domEventHandlerWrappingTests(ReactComponentFactoryProxy factory) { }); } -void refTests(ReactComponentFactoryProxy factory, {void verifyRefValue(dynamic refValue)}) { +/// Tests that refs work properly with [factory], including [forwardRef2] and [chainRefs] integration tests. +/// +/// [verifyRefValue] will be called with the actual ref value, and should contain an `expect` +/// that the value is of the correct type. +/// +/// [verifyJsRefValue] is optional, and only necessary when the ref value is wrapped and will be different in Dart vs +/// JS (e.g., react.Component vs ReactComponent for class based-components). +/// If omitted, it defaults to [verifyRefValue]. +/// It will be called with the actual ref value for JS refs, (e.g., JS ref objects as opposed to Dart objects) +void refTests( + ReactComponentFactoryProxy factory, { + @required void verifyRefValue(dynamic refValue), + void verifyJsRefValue(dynamic refValue), +}) { if (T == dynamic) { throw ArgumentError('Generic parameter T must be specified'); } - test('callback refs are called with the correct value', () { - var called = false; - var refValue; - - rtu.renderIntoDocument(factory({ - 'ref': (ref) { - called = true; - refValue = ref; - } - })); - - expect(called, isTrue, reason: 'should have called the callback ref'); - verifyRefValue(refValue); - }); + verifyJsRefValue ??= verifyRefValue; - test('string refs are created with the correct value', () { - ReactComponent renderedInstance = _renderWithStringRefSupportingOwner(() => factory({'ref': 'test'})); + // + // [1] Setting a JS callback ref on a Dart component results in the ref being called with the Dart component, + // not the JS one, so these tests will fail. + // + // This is because the callback ref will get treated like a Dart callback when the Dart factory performs its conversion. + // + // We don't need to support that use-case, so we won't test it. + // - // ignore: deprecated_member_use_from_same_package - verifyRefValue(renderedInstance.dartComponent.ref('test')); - }); - - test('createRef function creates ref with correct value', () { - final Ref ref = createRef(); - - rtu.renderIntoDocument(factory({ - 'ref': ref, - })); - - verifyRefValue(ref.current); - }); - - test('forwardRef function passes a ref through a component to one of its children', () { - dynamic actualRef; - var ForwardRefTestComponent = forwardRef((props, ref) { - actualRef = ref; + final factoryIsDartComponent = isDartComponent(factory({})); + final testCaseCollection = RefTestCaseCollection( + includeJsCallbackRefCase: !factoryIsDartComponent, // [1] + ); - return factory({ - 'ref': ref, - 'id': props['childId'], + group('supports all ref types:', () { + for (final name in testCaseCollection.allTestCaseNames) { + test(name, () { + final testCase = testCaseCollection.createCaseByName(name); + rtu.renderIntoDocument(factory({ + 'ref': testCase.ref, + })); + final verifyFunction = testCase.isJs ? verifyJsRefValue : verifyRefValue; + verifyFunction(testCase.getCurrent()); }); - }); - - final Ref refObject = createRef(); - - rtu.renderIntoDocument(ForwardRefTestComponent({ - 'ref': refObject, - 'childId': 'test', - })); - - // Extra type checking since JS refs being passed through - // aren't caught by built-in type checking. - expect(actualRef, isA()); - - // Props are accessed differently for DOM, Dart, and JS components. - var idValue; - final current = refObject.current; - expect(current, isNotNull); - if (current is Element) { - idValue = current.id; - } else if (current is react.Component) { - idValue = current.props['id']; - } else if (rtu.isCompositeComponent(current)) { - idValue = JsBackedMap.fromJs((current as ReactComponent).props)['id']; - } else { - fail('Unknown instance type: current'); } - expect(idValue, equals('test'), reason: 'child component should have access to parent props'); - verifyRefValue(refObject.current); - }); - - group('forwardRef sets displayName on the rendered component as expected', () { - test('when displayName argument is not passed to forwardRef', () { - var ForwardRefTestComponent = forwardRef((props, ref) { - // Extra type checking since JS refs being passed through - // aren't caught by built-in type checking. - expect(ref, isA()); - - return factory({'ref': ref}); - }); - - expect(getProperty(getProperty(ForwardRefTestComponent.type, 'render'), 'displayName'), 'Anonymous'); - }); + test('string refs', () { + ReactComponent renderedInstance = _renderWithStringRefSupportingOwner(() => factory({'ref': 'test'})); - test('when displayName argument is passed to forwardRef', () { - var ForwardRefTestComponent = forwardRef((props, ref) { - // Extra type checking since JS refs being passed through - // aren't caught by built-in type checking. - expect(ref, isA()); - - return factory({'ref': ref}); - }, displayName: 'ForwardRefTestComponent'); - - expect( - getProperty(getProperty(ForwardRefTestComponent.type, 'render'), 'displayName'), 'ForwardRefTestComponent'); + // ignore: deprecated_member_use_from_same_package + verifyRefValue(renderedInstance.dartComponent.ref('test')); }); }); - group('forwardRef wraps event handlers properly,', () { - const dartInside = EventTestCase.dart('onMouseDown', 'inside forwardRef'); - const dart = EventTestCase.dart('onMouseUp', 'set on forwardRef hoc'); - const dartCloned = EventTestCase.dart('onMouseLeave', 'cloned onto forwardRef hoc'); - const jsCloned = EventTestCase.js('onClick', 'cloned onto forwardRef hoc '); - const eventCases = {dartInside, dart, jsCloned, dartCloned}; - - Element node; - Map events; - Map propsFromDartRender; - - setUpAll(() { - expect(eventCases.map((h) => h.eventPropKey).toSet(), hasLength(eventCases.length), - reason: 'test setup: each helper should have a unique event key'); - - node = null; - events = {}; - propsFromDartRender = null; - - final ForwardRefTestComponent = forwardRef((props, ref) { - return factory({ - ...props, - 'onDartRender': (p) { - propsFromDartRender = p; - }, - dartInside.eventPropKey: (event) => events[dartInside] = event, - isDartComponent(factory({})) ? 'forwardedRef' : 'ref': ref, - }, props['children']); - }); - - final refObject = createRef(); - var element = ForwardRefTestComponent({ - 'ref': refObject, - dart.eventPropKey: (event) => events[dart] = event, - }); - - element = React.cloneElement( - element, - jsifyAndAllowInterop({ - jsCloned.eventPropKey: (event) => events[jsCloned] = event, - }), - ); - - element = React.cloneElement( - element, - // Invoke the factory corresponding to element's type - // to get the correct version of the handler (converted or non-converted) - // before passing it straight to the JS. - ForwardRefTestComponent({ - dartCloned.eventPropKey: (event) => events[dartCloned] = event, - }).props, - ); - - rtu.renderIntoDocument(element); - - node = react_dom.findDOMNode(refObject.current); - }); + group('forwardRef function passes a ref through a component to one of its children, when the ref is a:', () { + for (final name in testCaseCollection.allTestCaseNames) { + // Callback refs don't work properly with forwardRef. + // This is part of why forwardRef is deprecated. + if (!name.contains('callback ref')) { + test(name, () { + final testCase = testCaseCollection.createCaseByName(name); + var ForwardRefTestComponent = forwardRef((props, ref) { + return factory({'ref': ref}); + }); - group('passing Dart events to Dart handlers, and JS events to handlers originating from JS:', () { - for (var eventCase in eventCases) { - test(eventCase.description, () { - eventCase.simulate(node); - expect(events[eventCase], isNotNull, reason: 'handler should have been called'); - expect(events[eventCase], - eventCase.isDart ? isA() : isNot(isA())); + rtu.renderIntoDocument(ForwardRefTestComponent({ + 'ref': testCase.ref, + })); + final verifyFunction = testCase.isJs ? verifyJsRefValue : verifyRefValue; + verifyFunction(testCase.getCurrent()); }); } - }); + } + }); - if (isDartComponent(factory({}))) { - group('in a way that the handlers are callable from within the Dart component:', () { - setUpAll(() { - expect(propsFromDartRender, isNotNull, - reason: 'test setup: component must pass props into props.onDartRender'); + group('forwardRef2 function passes a ref through a component to one of its children, when the ref is a:', () { + for (final name in testCaseCollection.allTestCaseNames) { + test(name, () { + final testCase = testCaseCollection.createCaseByName(name); + var ForwardRefTestComponent = forwardRef2((props, ref) { + return factory({'ref': ref}); }); - final dummyEvent = react.SyntheticMouseEvent(null, null, null, null, null, null, null, null, null, null, null, - null, null, null, null, null, null, null, null, null, null, null, null, null, null, null); - - for (var eventCase in eventCases.where((helper) => helper.isDart)) { - test(eventCase.description, () { - expect(() => propsFromDartRender[eventCase.eventPropKey](dummyEvent), returnsNormally); - }); - } + rtu.renderIntoDocument(ForwardRefTestComponent({ + 'ref': testCase.ref, + })); + final verifyFunction = testCase.isJs ? verifyJsRefValue : verifyRefValue; + verifyFunction(testCase.getCurrent()); }); } }); - group('has functional callback refs when they are typed as', () { - test('`dynamic Function(dynamic)`', () { - T fooRef; - callbackRef(dynamic ref) { - fooRef = ref; - } - - expect(() => rtu.renderIntoDocument(factory({'ref': callbackRef})), returnsNormally, - reason: 'React should not have a problem with the ref we pass it, and calling it should not throw'); - expect(fooRef, isA(), reason: 'should be the correct type, not be a NativeJavaScriptObject/etc.'); - }); - - test('`dynamic Function(ComponentClass)`', () { - T fooRef; - callbackRef(T ref) { - fooRef = ref; - } - - expect(() => rtu.renderIntoDocument(factory({'ref': callbackRef})), returnsNormally, - reason: 'React should not have a problem with the ref we pass it, and calling it should not throw'); - expect(fooRef, isA(), reason: 'should be the correct type, not be a NativeJavaScriptObject/etc.'); - }); - }); - group('chainRefList works', () { test('with all different types of values, ignoring null', () { - final testCases = RefTestCase.allChainable(); + final testCases = testCaseCollection.createAllCases(); - T refValue; + final refSpy = createRef(); rtu.renderIntoDocument(factory({ 'ref': chainRefList([ - (ref) => refValue = ref, + refSpy, null, null, ...testCases.map((t) => t.ref), ]), })); - // Test setup check: verify refValue is correct, - // which we'll use below to verify refs were updated. - verifyRefValue(refValue); for (final testCase in testCases) { - testCase.verifyRefWasUpdated(refValue); + final verifyFunction = testCase.isJs ? verifyJsRefValue : verifyRefValue; + final valueToVerify = testCase.isJs ? refSpy.jsRef.current : refSpy.current; + + // Test setup check: verify refValue is correct, + // which we'll use below to verify refs were updated. + verifyFunction(valueToVerify); + testCase.verifyRefWasUpdated(valueToVerify); } }); + group('when refs come from sources where they have been potentially converted:', () { + test('ReactElement.ref', () { + final testCases = testCaseCollection.createAllCases().map((testCase) { + return RefTestCase( + name: testCase.name, + ref: (factory({'ref': testCase.ref}) as ReactElement).ref, + verifyRefWasUpdated: testCase.verifyRefWasUpdated, + getCurrent: testCase.getCurrent, + isJs: testCase.isJs, + ); + }).toList(); + + final refSpy = createRef(); + rtu.renderIntoDocument(factory({ + 'ref': chainRefList([ + refSpy, + null, + null, + ...testCases.map((t) => t.ref), + ]), + })); + + for (final testCase in testCases) { + final verifyFunction = testCase.isJs ? verifyJsRefValue : verifyRefValue; + final valueToVerify = testCase.isJs ? refSpy.jsRef.current : refSpy.current; + + // Test setup check: verify refValue is correct, + // which we'll use below to verify refs were updated. + verifyFunction(valueToVerify); + testCase.verifyRefWasUpdated(valueToVerify); + } + }); + + group('forwardRef2 arg, and the ref is a', () { + for (final name in testCaseCollection.allTestCaseNames) { + test(name, () { + final testCase = testCaseCollection.createCaseByName(name); + + final refSpy = createRef(); + + final wrapperFactory = react.forwardRef2((props, ref) { + return factory({ + ...props, + 'ref': chainRefList([ + refSpy, + ref, + ]), + }, props['children']); + }); + + rtu.renderIntoDocument(wrapperFactory({ + 'ref': testCase.ref, + })); + + final verifyFunction = testCase.isJs ? verifyJsRefValue : verifyRefValue; + final valueToVerify = testCase.isJs ? refSpy.jsRef.current : refSpy.current; + + // Test setup check: verify refValue is correct, + // which we'll use below to verify refs were updated. + verifyFunction(valueToVerify); + testCase.verifyRefWasUpdated(valueToVerify); + }); + } + }); + }); + // Other cases tested in chainRefList's own tests }); } diff --git a/test/factory/dart_factory_test.dart b/test/factory/dart_factory_test.dart index 6c62cfc2..7cd5d536 100644 --- a/test/factory/dart_factory_test.dart +++ b/test/factory/dart_factory_test.dart @@ -18,27 +18,39 @@ main() { }); group('- common factory behavior -', () { - group('Component -', () { + group('Component', () { group('- common factory behavior -', () { commonFactoryTests(Foo, dartComponentVersion: ReactDartComponentVersion.component); }); group('- refs -', () { - refTests<_Foo>(Foo, verifyRefValue: (ref) { - expect(ref, TypeMatcher<_Foo>()); - }); + refTests<_Foo>( + Foo, + verifyRefValue: (ref) { + expect(ref, isA<_Foo>()); + }, + verifyJsRefValue: (ref) { + expect(ref, isA().having((c) => c.dartComponent, 'dartComponent', isA<_Foo>())); + }, + ); }); }); - group('Component2 -', () { + group('Component2', () { group('- common factory behavior -', () { commonFactoryTests(Foo2, dartComponentVersion: ReactDartComponentVersion.component2); }); group('- refs -', () { - refTests<_Foo2>(Foo2, verifyRefValue: (ref) { - expect(ref, TypeMatcher<_Foo2>()); - }); + refTests<_Foo2>( + Foo2, + verifyRefValue: (ref) { + expect(ref, isA<_Foo2>()); + }, + verifyJsRefValue: (ref) { + expect(ref, isA().having((c) => c.dartComponent, 'dartComponent', isA<_Foo2>())); + }, + ); }); }); }); diff --git a/test/factory/dart_function_factory_test.dart b/test/factory/dart_function_factory_test.dart index f17f70e5..f6466c78 100644 --- a/test/factory/dart_function_factory_test.dart +++ b/test/factory/dart_function_factory_test.dart @@ -20,7 +20,6 @@ main() { group('- common factory behavior -', () { commonFactoryTests( FunctionFoo, - isFunctionComponent: true, dartComponentVersion: ReactDartComponentVersion.component2, ); }); @@ -44,26 +43,6 @@ main() { }); }); }); - - group('forwardRef', () { - group('- common factory behavior -', () { - commonFactoryTests( - ForwardRefTest, - isFunctionComponent: true, - dartComponentVersion: ReactDartComponentVersion.component2, - ); - }); - }); - - group('memo', () { - group('- common factory behavior -', () { - commonFactoryTests( - MemoTest, - isFunctionComponent: true, - dartComponentVersion: ReactDartComponentVersion.component2, - ); - }); - }); } String _getJsFunctionName(Function object) => getProperty(object, 'name') ?? getProperty(object, '\$static_name'); @@ -76,13 +55,3 @@ _FunctionFoo(Map props) { props['onDartRender']?.call(props); return react.div({}); } - -final ForwardRefTest = react.forwardRef((props, ref) { - props['onDartRender']?.call(props); - return react.div({...props, 'ref': ref}); -}); - -final MemoTest = react.memo(react.registerFunctionComponent((props) { - props['onDartRender']?.call(props); - return react.div({...props}); -})); diff --git a/test/forward_ref_test.dart b/test/forward_ref_test.dart new file mode 100644 index 00000000..dcc9ad12 --- /dev/null +++ b/test/forward_ref_test.dart @@ -0,0 +1,77 @@ +@TestOn('browser') +library react.forward_ref_test; + +import 'dart:js_util'; + +import 'package:react/react.dart' as react; +import 'package:react/react_client/react_interop.dart'; +import 'package:test/test.dart'; + +import 'factory/common_factory_tests.dart'; + +main() { + group('forwardRef', () { + group('- common factory behavior -', () { + // ignore: deprecated_member_use_from_same_package + final ForwardRefTest = react.forwardRef((props, ref) { + props['onDartRender']?.call(props); + return react.div({...props, 'ref': ref}); + }); + + commonFactoryTests( + ForwardRefTest, + // ignore: invalid_use_of_protected_member + dartComponentVersion: ReactDartComponentVersion.component2, + // Dart props passed to forwardRef get converted when they shouldn't, so these tests fail. + // This is part of why forwardRef is deprecated. + skipPropValuesTest: true, + ); + }); + + // Ref behavior is tested functionally for all factory types in commonFactoryTests + + group('sets displayName on the rendered component as expected', () { + test('falling back to "Anonymous" when the displayName argument is not passed to forwardRef', () { + var ForwardRefTestComponent = forwardRef((props, ref) {}); + expect(getProperty(getProperty(ForwardRefTestComponent.type, 'render'), 'displayName'), 'Anonymous'); + }); + + test('when displayName argument is passed to forwardRef', () { + const name = 'ForwardRefTestComponent'; + var ForwardRefTestComponent = forwardRef((props, ref) {}, displayName: name); + expect(getProperty(getProperty(ForwardRefTestComponent.type, 'render'), 'displayName'), name); + }); + }); + }); + + group('forwardRef2', () { + group('- common factory behavior -', () { + final ForwardRef2Test = react.forwardRef2((props, ref) { + props['onDartRender']?.call(props); + props['onDartRenderWithRef']?.call(props, ref); + return react.div({...props, 'ref': ref}); + }); + + commonFactoryTests( + ForwardRef2Test, + // ignore: invalid_use_of_protected_member + dartComponentVersion: ReactDartComponentVersion.component2, + ); + }); + + // Ref behavior is tested functionally for all factory types in commonFactoryTests + + group('sets name on the rendered component as expected', () { + test('unless the displayName argument is not passed to forwardRef2', () { + var ForwardRefTestComponent = forwardRef2((props, ref) {}); + expect(getProperty(getProperty(ForwardRefTestComponent.type, 'render'), 'name'), anyOf('', isNull)); + }); + + test('when displayName argument is passed to forwardRef2', () { + const name = 'ForwardRefTestComponent'; + var ForwardRefTestComponent = forwardRef2((props, ref) {}, displayName: name); + expect(getProperty(getProperty(ForwardRefTestComponent.type, 'render'), 'name'), name); + }); + }); + }); +} diff --git a/test/forward_ref_test.html b/test/forward_ref_test.html new file mode 100644 index 00000000..a54f7b2b --- /dev/null +++ b/test/forward_ref_test.html @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/test/hooks_test.dart b/test/hooks_test.dart index dbaf3f13..328aa5f6 100644 --- a/test/hooks_test.dart +++ b/test/hooks_test.dart @@ -1,4 +1,3 @@ -// ignore_for_file: deprecated_member_use_from_same_package @TestOn('browser') @JS() library hooks_test; @@ -14,6 +13,8 @@ import 'package:react/react_dom.dart' as react_dom; import 'package:react/react_test_utils.dart' as react_test_utils; import 'package:test/test.dart'; +import 'factory/common_factory_tests.dart'; + main() { group('React Hooks: ', () { group('useState -', () { @@ -642,97 +643,97 @@ main() { }); group('useImperativeHandle -', () { - var mountNode = new DivElement(); - ReactDartFunctionComponentFactoryProxy UseImperativeHandleTest; - ButtonElement incrementButton; - ButtonElement reRenderButtonRef1; - ButtonElement reRenderButtonRef2; - Ref noDepsRef; - Ref emptyDepsRef; - Ref depsRef; - StateHook count; + group('updates `ref.current` to the return value of `createHandle()`', () { + var mountNode = new DivElement(); + ReactDartFunctionComponentFactoryProxy UseImperativeHandleTest; + ButtonElement incrementButton; + ButtonElement reRenderButtonRef1; + ButtonElement reRenderButtonRef2; + Ref noDepsRef; + Ref emptyDepsRef; + Ref depsRef; + StateHook count; - setUpAll(() { - var NoDepsComponent = react.forwardRef((props, ref) { - count = useState(0); + setUpAll(() { + var NoDepsComponent = react.forwardRef2((props, ref) { + count = useState(0); - useImperativeHandle( - ref, - () => {'increment': () => count.setWithUpdater((prev) => prev + 1)}, - ); + useImperativeHandle( + ref, + () => {'increment': () => count.setWithUpdater((prev) => prev + 1)}, + ); - return react.div({'ref': ref}, count.value); - }); + return react.div({'ref': ref}, count.value); + }); - var EmptyDepsComponent = react.forwardRef((props, ref) { - var count = useState(0); + var EmptyDepsComponent = react.forwardRef2((props, ref) { + var count = useState(0); - useImperativeHandle(ref, () => count.value, []); + useImperativeHandle(ref, () => count.value, []); - return react.Fragment({}, [ - react.div({'ref': ref}, count.value), - react.button({ - 'ref': (ref) => reRenderButtonRef1 = ref, - 'onClick': (_) => count.setWithUpdater((prev) => prev + 1), - }, []), - ]); - }); + return react.Fragment({}, [ + react.div({'ref': ref}, count.value), + react.button({ + 'ref': (ref) => reRenderButtonRef1 = ref, + 'onClick': (_) => count.setWithUpdater((prev) => prev + 1), + }, []), + ]); + }); - var DepsComponent = react.forwardRef((props, ref) { - var count = useState(0); + var DepsComponent = react.forwardRef2((props, ref) { + var count = useState(0); - useImperativeHandle(ref, () => count.value, [count.value]); + useImperativeHandle(ref, () => count.value, [count.value]); - return react.Fragment({}, [ - react.div({'ref': ref}, count.value), - react.button({ - 'ref': (ref) => reRenderButtonRef2 = ref, - 'onClick': (_) => count.setWithUpdater((prev) => prev + 1), - }, []), - ]); - }); + return react.Fragment({}, [ + react.div({'ref': ref}, count.value), + react.button({ + 'ref': (ref) => reRenderButtonRef2 = ref, + 'onClick': (_) => count.setWithUpdater((prev) => prev + 1), + }, []), + ]); + }); - var NullRefComponent = react.registerFunctionComponent((props) { - var count = useState(0); - Ref someRefThatIsNotSet = props['someRefThatIsNotSet']; + var NullRefComponent = react.registerFunctionComponent((props) { + var count = useState(0); + Ref someRefThatIsNotSet = props['someRefThatIsNotSet']; - useImperativeHandle(someRefThatIsNotSet, () => count.value, [count.value]); + useImperativeHandle(someRefThatIsNotSet, () => count.value, [count.value]); - return react.Fragment({}, [ - react.div({'ref': someRefThatIsNotSet}, count.value), - react.button({ - 'ref': (ref) => reRenderButtonRef2 = ref, - 'onClick': (_) => count.setWithUpdater((prev) => prev + 1), - }, []), - ]); - }); + return react.Fragment({}, [ + react.div({'ref': someRefThatIsNotSet}, count.value), + react.button({ + 'ref': (ref) => reRenderButtonRef2 = ref, + 'onClick': (_) => count.setWithUpdater((prev) => prev + 1), + }, []), + ]); + }); - expect(() => react_dom.render(NullRefComponent({}, []), mountNode), returnsNormally, - reason: 'Hook should not throw if the ref is null'); - react_dom.unmountComponentAtNode(mountNode); + expect(() => react_dom.render(NullRefComponent({}, []), mountNode), returnsNormally, + reason: 'Hook should not throw if the ref is null'); + react_dom.unmountComponentAtNode(mountNode); - UseImperativeHandleTest = react.registerFunctionComponent((Map props) { - noDepsRef = useRef(); - emptyDepsRef = useRef(); - depsRef = useRef(); + UseImperativeHandleTest = react.registerFunctionComponent((Map props) { + noDepsRef = useRef(); + emptyDepsRef = useRef(); + depsRef = useRef(); - return react.Fragment({}, [ - NoDepsComponent({'ref': noDepsRef}, []), - react.button({ - 'ref': (ref) => incrementButton = ref, - 'onClick': (_) => noDepsRef.current['increment'](), - }, [ - '+' - ]), - EmptyDepsComponent({'ref': emptyDepsRef}), - DepsComponent({'ref': depsRef}), - ]); - }); + return react.Fragment({}, [ + NoDepsComponent({'ref': noDepsRef}, []), + react.button({ + 'ref': (ref) => incrementButton = ref, + 'onClick': (_) => noDepsRef.current['increment'](), + }, [ + '+' + ]), + EmptyDepsComponent({'ref': emptyDepsRef}), + DepsComponent({'ref': depsRef}), + ]); + }); - react_dom.render(UseImperativeHandleTest({}), mountNode); - }); + react_dom.render(UseImperativeHandleTest({}), mountNode); + }); - group('updates `ref.current` to the return value of `createHandle()`', () { test('(with no dependency list)', () { expect(noDepsRef.current, isA(), reason: 'useImperativeHandle overrides the existing ref.current value'); expect(noDepsRef.current['increment'], isA()); @@ -760,6 +761,18 @@ main() { expect(depsRef.current, 1, reason: 'current value updates because count.value is in dependency list'); }); }); + + group('works with all types of consumer refs -', () { + const customRefValue = {'Am I a ref?': true}; + final factory = react.forwardRef2((props, ref) { + useImperativeHandle(ref, () => customRefValue); + return react.div({}); + }); + + refTests(factory, verifyRefValue: (refValue) { + expect(refValue, same(customRefValue)); + }); + }); }); group('useDebugValue -', () { diff --git a/test/lifecycle_test.dart b/test/lifecycle_test.dart index 615b553a..3e31c7f7 100644 --- a/test/lifecycle_test.dart +++ b/test/lifecycle_test.dart @@ -565,24 +565,6 @@ main() { expect(propTypeCheck, isA()); }); }); - - group('props are received correctly without interop interfering with the values:', () { - void testTypeValue(dynamic testValue) { - var receivedValue; - var receivedProps; - - ReactDartFunctionComponentFactoryProxy PropsTypeTest = react.registerFunctionComponent((Map props) { - receivedProps = props; - receivedValue = props['testValue']; - return null; - }); - react_dom.render(PropsTypeTest({'testValue': testValue}), mountNode); - expect(receivedProps, isA()); - expect(receivedValue, same(testValue)); - } - - sharedTypeTests(testTypeValue); - }); }); }); } diff --git a/test/react_client/chain_refs_test.dart b/test/react_client/chain_refs_test.dart index 5249579b..64c3aa9a 100644 --- a/test/react_client/chain_refs_test.dart +++ b/test/react_client/chain_refs_test.dart @@ -77,7 +77,8 @@ main() { }, tags: 'no-dart2js'); test('does not raise an assertion for valid input refs', () { - expect(() => chainRefList(RefTestCase.allChainable().map((r) => r.ref).toList()), returnsNormally); + final testCases = RefTestCaseCollection().createAllCases(); + expect(() => chainRefList(testCases.map((r) => r.ref).toList()), returnsNormally); }); }); }); diff --git a/test/react_memo_test.dart b/test/react_memo_test.dart index 45594af6..8e4d6a5d 100644 --- a/test/react_memo_test.dart +++ b/test/react_memo_test.dart @@ -5,10 +5,63 @@ import 'dart:html'; import 'package:react/react.dart' as react; import 'package:react/react_client.dart'; +import 'package:react/react_client/react_interop.dart'; import 'package:react/react_test_utils.dart' as rtu; import 'package:test/test.dart'; +import 'factory/common_factory_tests.dart'; + main() { + group('memo', () { + // ignore: deprecated_member_use_from_same_package + sharedMemoTests(react.memo); + + group('- common factory behavior -', () { + // ignore: deprecated_member_use_from_same_package + final MemoTest = react.memo(react.registerFunctionComponent((props) { + props['onDartRender']?.call(props); + return react.div({...props}); + })); + + commonFactoryTests( + MemoTest, + // ignore: invalid_use_of_protected_member + dartComponentVersion: ReactDartComponentVersion.component2, + // Dart props passed to memo get converted when they shouldn't, so these tests fail. + // This is part of why memo is deprecated. + skipPropValuesTest: true, + ); + }); + }); + + group('memo2', () { + sharedMemoTests(react.memo2); + + test('can be passed a forwardRef component (regression test)', () { + ReactComponentFactoryProxy factory; + expect(() => factory = react.memo2(react.forwardRef2((props, ref) => 'foo')), returnsNormally); + expect(() => rtu.renderIntoDocument(factory({})), returnsNormally); + }); + + group('- common factory behavior -', () { + final Memo2Test = react.memo2(react.registerFunctionComponent((props) { + props['onDartRender']?.call(props); + return react.div({...props}); + })); + + commonFactoryTests( + Memo2Test, + // ignore: invalid_use_of_protected_member + dartComponentVersion: ReactDartComponentVersion.component2, + ); + }); + }); +} + +typedef MemoFunction = react.ReactComponentFactoryProxy Function(ReactDartFunctionComponentFactoryProxy, + {bool Function(Map, Map) areEqual}); + +void sharedMemoTests(MemoFunction memoFunction) { Ref<_MemoTestWrapperComponent> memoTestWrapperComponentRef; Ref localCountDisplayRef; Ref valueMemoShouldIgnoreViaAreEqualDisplayRef; @@ -27,7 +80,7 @@ main() { return prevProps['localCount'] == nextProps['localCount']; }; - final MemoTest = react.memo(react.registerFunctionComponent((Map props) { + final MemoTest = memoFunction(react.registerFunctionComponent((Map props) { childMemoRenderCount++; return react.div( {}, @@ -58,64 +111,62 @@ main() { reason: 'test setup sanity check'); } - group('memo', () { - setUp(() { - memoTestWrapperComponentRef = react.createRef<_MemoTestWrapperComponent>(); - localCountDisplayRef = react.createRef(); - valueMemoShouldIgnoreViaAreEqualDisplayRef = react.createRef(); - childMemoRenderCount = 0; - }); - - tearDown(() { - memoTestWrapperComponentRef = null; - localCountDisplayRef = null; - valueMemoShouldIgnoreViaAreEqualDisplayRef = null; - }); - - group('renders its child component when props change', () { - test('', () { - renderMemoTest(); - - memoTestWrapperComponentRef.current.increaseLocalCount(); - expect(memoTestWrapperComponentRef.current.state['localCount'], 1, reason: 'test setup sanity check'); - expect(memoTestWrapperComponentRef.current.redrawCount, 1, reason: 'test setup sanity check'); + setUp(() { + memoTestWrapperComponentRef = react.createRef<_MemoTestWrapperComponent>(); + localCountDisplayRef = react.createRef(); + valueMemoShouldIgnoreViaAreEqualDisplayRef = react.createRef(); + childMemoRenderCount = 0; + }); - expect(childMemoRenderCount, 2); - expect(localCountDisplayRef.current.text, '1'); + tearDown(() { + memoTestWrapperComponentRef = null; + localCountDisplayRef = null; + valueMemoShouldIgnoreViaAreEqualDisplayRef = null; + }); - memoTestWrapperComponentRef.current.increaseValueMemoShouldIgnoreViaAreEqual(); - expect(memoTestWrapperComponentRef.current.state['valueMemoShouldIgnoreViaAreEqual'], 1, - reason: 'test setup sanity check'); - expect(memoTestWrapperComponentRef.current.redrawCount, 2, reason: 'test setup sanity check'); + group('renders its child component when props change', () { + test('', () { + renderMemoTest(); - expect(childMemoRenderCount, 3); - expect(valueMemoShouldIgnoreViaAreEqualDisplayRef.current.text, '1'); - }); + memoTestWrapperComponentRef.current.increaseLocalCount(); + expect(memoTestWrapperComponentRef.current.state['localCount'], 1, reason: 'test setup sanity check'); + expect(memoTestWrapperComponentRef.current.redrawCount, 1, reason: 'test setup sanity check'); - test('unless the areEqual argument is set to a function that customizes when re-renders occur', () { - renderMemoTest(testAreEqual: true); + expect(childMemoRenderCount, 2); + expect(localCountDisplayRef.current.text, '1'); - memoTestWrapperComponentRef.current.increaseValueMemoShouldIgnoreViaAreEqual(); - expect(memoTestWrapperComponentRef.current.state['valueMemoShouldIgnoreViaAreEqual'], 1, - reason: 'test setup sanity check'); - expect(memoTestWrapperComponentRef.current.redrawCount, 1, reason: 'test setup sanity check'); + memoTestWrapperComponentRef.current.increaseValueMemoShouldIgnoreViaAreEqual(); + expect(memoTestWrapperComponentRef.current.state['valueMemoShouldIgnoreViaAreEqual'], 1, + reason: 'test setup sanity check'); + expect(memoTestWrapperComponentRef.current.redrawCount, 2, reason: 'test setup sanity check'); - expect(childMemoRenderCount, 1); - expect(valueMemoShouldIgnoreViaAreEqualDisplayRef.current.text, '0'); - }); + expect(childMemoRenderCount, 3); + expect(valueMemoShouldIgnoreViaAreEqualDisplayRef.current.text, '1'); }); - test('does not re-render its child component when parent updates and props remain the same', () { - renderMemoTest(); + test('unless the areEqual argument is set to a function that customizes when re-renders occur', () { + renderMemoTest(testAreEqual: true); - memoTestWrapperComponentRef.current.increaseValueMemoShouldNotKnowAbout(); - expect(memoTestWrapperComponentRef.current.state['valueMemoShouldNotKnowAbout'], 1, + memoTestWrapperComponentRef.current.increaseValueMemoShouldIgnoreViaAreEqual(); + expect(memoTestWrapperComponentRef.current.state['valueMemoShouldIgnoreViaAreEqual'], 1, reason: 'test setup sanity check'); expect(memoTestWrapperComponentRef.current.redrawCount, 1, reason: 'test setup sanity check'); expect(childMemoRenderCount, 1); + expect(valueMemoShouldIgnoreViaAreEqualDisplayRef.current.text, '0'); }); }); + + test('does not re-render its child component when parent updates and props remain the same', () { + renderMemoTest(); + + memoTestWrapperComponentRef.current.increaseValueMemoShouldNotKnowAbout(); + expect(memoTestWrapperComponentRef.current.state['valueMemoShouldNotKnowAbout'], 1, + reason: 'test setup sanity check'); + expect(memoTestWrapperComponentRef.current.redrawCount, 1, reason: 'test setup sanity check'); + + expect(childMemoRenderCount, 1); + }); } final MemoTestWrapper = react.registerComponent(() => _MemoTestWrapperComponent()); diff --git a/test/util.dart b/test/util.dart index 4458af64..df5a90a3 100644 --- a/test/util.dart +++ b/test/util.dart @@ -62,54 +62,120 @@ bool assertsEnabled() { /// A test case that can be used for consuming a specific kind of ref and verifying /// it was updated properly when rendered. +/// +/// Test cases should not be reused within a test or across multiple tests, to avoid +/// the [ref] from being used by multiple components and its value being polluted. class RefTestCase { + /// The name of the test case. + final String name; + + /// The ref to be passed into a component. final dynamic ref; + + /// Verifies (usually via `expect`) that the ref was updated exactly once with [actualValue]. final Function(dynamic actualValue) verifyRefWasUpdated; - RefTestCase._({@required this.ref, @required this.verifyRefWasUpdated}); + /// Returns the current value of the ref. + final dynamic Function() getCurrent; + + /// Whether the ref is a non-Dart object, such as a ref originating from outside of Dart code + /// or a JS-converted Dart ref. + final bool isJs; + + RefTestCase({ + @required this.name, + @required this.ref, + @required this.verifyRefWasUpdated, + @required this.getCurrent, + this.isJs = false, + }); +} + +/// A collection of methods that create [RefTestCase]s, combined into a class so that they can easily share a +/// generic parameter [T] (the type of the Dart ref value). +class RefTestCaseCollection { + final bool includeJsCallbackRefCase; + + RefTestCaseCollection({this.includeJsCallbackRefCase = true}) { + if (T == dynamic) { + throw ArgumentError('Generic parameter T must be specified'); + } + } - static RefTestCase untypedCallbackRefCase() { + RefTestCase createUntypedCallbackRefCase() { + const name = 'untyped callback ref'; final calls = []; - return RefTestCase._( + return RefTestCase( + name: name, ref: (value) => calls.add(value), - verifyRefWasUpdated: (actualValue) => expect(calls, [same(actualValue)]), + verifyRefWasUpdated: (actualValue) => expect(calls, [same(actualValue)], reason: _reasonMessage(name)), + getCurrent: () => calls.single, ); } - static RefTestCase typedCallbackRefCase() { + RefTestCase createTypedCallbackRefCase() { + const name = 'typed callback ref'; final calls = []; - return RefTestCase._( + return RefTestCase( + name: name, ref: (T value) => calls.add(value), - verifyRefWasUpdated: (actualValue) => expect(calls, [same(actualValue)]), + verifyRefWasUpdated: (actualValue) => expect(calls, [same(actualValue)], reason: _reasonMessage(name)), + getCurrent: () => calls.single, ); } - static RefTestCase refObjectCase() { + RefTestCase createRefObjectCase() { + const name = 'ref object'; final ref = createRef(); - return RefTestCase._( + return RefTestCase( + name: name, ref: ref, - verifyRefWasUpdated: (actualValue) => expect(ref.current, same(actualValue)), + verifyRefWasUpdated: (actualValue) => expect(ref.current, same(actualValue), reason: _reasonMessage(name)), + getCurrent: () => ref.current, ); } - static RefTestCase jsRefObjectCase() { + RefTestCase createJsCallbackRefCase() { + const name = 'JS callback ref'; + final calls = []; + return RefTestCase( + name: name, + ref: allowInterop((value) => calls.add(value)), + verifyRefWasUpdated: (actualValue) => expect(calls, [same(actualValue)], reason: _reasonMessage(name)), + getCurrent: () => calls.single, + isJs: true, + ); + } + + RefTestCase createJsRefObjectCase() { + const name = 'JS ref object'; final ref = React.createRef(); - return RefTestCase._( + return RefTestCase( + name: name, ref: ref, - verifyRefWasUpdated: (actualValue) => expect(ref.current, same(actualValue)), + verifyRefWasUpdated: (actualValue) => expect(ref.current, same(actualValue), reason: _reasonMessage(name)), + getCurrent: () => ref.current, + isJs: true, ); } - /// Test cases for each of the valid, chainable ref types: + static String _reasonMessage(String name) => '$name should have been updated'; + + /// Creates test cases for all of the valid, chainable ref types: /// /// 1. callback ref with untyped argument /// 2. callback ref with typed argument /// 3. createRef (Dart wrapper) /// 4. createRef (JS object) - static List allChainable() => [ - untypedCallbackRefCase(), - typedCallbackRefCase(), - refObjectCase(), - jsRefObjectCase(), + List createAllCases() => [ + createUntypedCallbackRefCase(), + createTypedCallbackRefCase(), + createRefObjectCase(), + if (includeJsCallbackRefCase) createJsCallbackRefCase(), + createJsRefObjectCase(), ]; + + RefTestCase createCaseByName(String name) => createAllCases().singleWhere((c) => c.name == name); + + List get allTestCaseNames => createAllCases().map((c) => c.name).toList(); }