diff --git a/lib/react_client/component_factory.dart b/lib/react_client/component_factory.dart index 801301eb..ab8da94d 100644 --- a/lib/react_client/component_factory.dart +++ b/lib/react_client/component_factory.dart @@ -8,6 +8,7 @@ import 'package:js/js.dart'; import 'package:react/react.dart'; import 'package:react/react_client.dart'; import 'package:react/react_client/js_backed_map.dart'; +import 'package:react/react_client/private_utils.dart'; import 'package:react/react_client/react_interop.dart'; import 'package:react/src/context.dart'; @@ -282,10 +283,16 @@ class ReactJsComponentFactoryProxy extends ReactComponentFactoryProxy { /// Default: `false` final bool alwaysReturnChildrenAsList; - ReactJsComponentFactoryProxy(ReactClass jsClass, - {this.shouldConvertDomProps: true, this.alwaysReturnChildrenAsList: false}) - : this.type = jsClass, - this.factory = React.createFactory(jsClass) { + final List _additionalRefPropKeys; + + ReactJsComponentFactoryProxy( + ReactClass jsClass, { + this.shouldConvertDomProps: true, + this.alwaysReturnChildrenAsList: false, + List additionalRefPropKeys = const [], + }) : this.type = jsClass, + this.factory = React.createFactory(jsClass), + this._additionalRefPropKeys = additionalRefPropKeys { if (jsClass == null) { throw new ArgumentError('`jsClass` must not be null. ' 'Ensure that the JS component class you\'re referencing is available and being accessed correctly.'); @@ -295,8 +302,10 @@ class ReactJsComponentFactoryProxy extends ReactComponentFactoryProxy { @override ReactElement build(Map props, [List childrenArgs]) { dynamic children = generateChildren(childrenArgs, shouldAlwaysBeList: alwaysReturnChildrenAsList); - JsMap convertedProps = - generateJsProps(props, shouldConvertEventHandlers: shouldConvertDomProps, convertCallbackRefValue: false); + JsMap convertedProps = generateJsProps(props, + shouldConvertEventHandlers: shouldConvertDomProps, + convertCallbackRefValue: false, + additionalRefPropKeys: _additionalRefPropKeys); return React.createElement(type, convertedProps, children); } } diff --git a/lib/react_client/js_interop_helpers.dart b/lib/react_client/js_interop_helpers.dart index 83d2cd5c..ea9eee6c 100644 --- a/lib/react_client/js_interop_helpers.dart +++ b/lib/react_client/js_interop_helpers.dart @@ -88,3 +88,8 @@ _convertDataTree(data) { return _convert(data); } + +/// Keeps track of functions found when converting JS props to Dart props. +/// +/// See: [forwardRef] for usage / context. +final isRawJsFunctionFromProps = Expando(); diff --git a/lib/react_client/private_utils.dart b/lib/react_client/private_utils.dart new file mode 100644 index 00000000..affa2f29 --- /dev/null +++ b/lib/react_client/private_utils.dart @@ -0,0 +1,12 @@ +@JS() +library react_client.private_utils; + +import 'dart:js_util'; + +import 'package:js/js.dart'; +import 'js_backed_map.dart'; + +@JS('Object.defineProperty') +external void defineProperty(dynamic object, String propertyName, JsMap descriptor); + +String getJsFunctionName(Function object) => getProperty(object, 'name') ?? getProperty(object, '\$static_name'); diff --git a/lib/react_client/react_interop.dart b/lib/react_client/react_interop.dart index e6884cf7..5fdf2c95 100644 --- a/lib/react_client/react_interop.dart +++ b/lib/react_client/react_interop.dart @@ -19,6 +19,8 @@ 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 ReactJsComponentFactoryProxy; +import 'package:react/react_client/js_interop_helpers.dart'; +import 'package:react/react_client/private_utils.dart'; import 'package:react/src/react_client/dart2_interop_workaround_bindings.dart'; typedef ReactElement ReactJsComponentFactory(props, children); @@ -32,6 +34,7 @@ typedef dynamic JsPropValidator( @JS() abstract class React { external static String get version; + external static ReactElement cloneElement(ReactElement element, [JsMap props, dynamic children]); external static ReactContext createContext([ dynamic defaultValue, int Function(dynamic currentValue, dynamic nextValue) calculateChangedBits, @@ -132,15 +135,107 @@ class JsRef { /// Automatically passes a [Ref] through a component to one of its children. /// +/// __Example 1:__ Forwarding refs to DOM components +/// +/// _[Analogous JS Demo](https://reactjs.org/docs/forwarding-refs.html#forwarding-refs-to-dom-components)_ +/// +/// ```dart +/// import 'dart:html'; +/// import 'package:react/react.dart' as react; +/// +/// // ---------- Component Definition ---------- +/// final FancyButton = react.forwardRef((props, ref) { +/// return react.button({'ref': ref, 'className': 'FancyButton'}, 'Click me!'); +/// }, displayName: 'FancyButton'); +/// +/// // ---------- Component Consumption ---------- +/// void main() { +/// final ref = createRef(); +/// +/// react_dom.render(FancyButton({'ref': ref})); +/// +/// // You can now get a ref directly to the DOM button: +/// final buttonNode = ref.current; +/// } +/// ``` +/// +/// __Example 2:__ Forwarding refs in higher-order components +/// +/// _[Analogous JS Demo](https://reactjs.org/docs/forwarding-refs.html#forwarding-refs-in-higher-order-components)_ +/// +/// ```dart +/// import 'dart:html'; +/// import 'package:react/react.dart' as react; +/// import 'package:react/react_client.dart' show setClientConfiguration, ReactJsComponentFactoryProxy; +/// import 'package:react/react_dom.dart' as react_dom; +/// +/// // ---------- Component Definitions ---------- +/// +/// final FancyButton = react.forwardRef((props, ref) { +/// return react.button({'ref': ref, 'className': 'FancyButton'}, 'Click me!'); +/// }, displayName: 'FancyButton'); +/// +/// class _LogProps extends react.Component2 { +/// @override +/// void componentDidUpdate(Map prevProps, _, [__]) { +/// print('old props: $prevProps'); +/// print('new props: ${this.props}'); +/// } +/// +/// @override +/// render() { +/// final propsToForward = {...props}..remove('forwardedRef'); +/// +/// // Assign the custom prop `forwardedRef` as a ref on the component passed in via `props.component` +/// return props['component']({...propsToForward, 'ref': props['forwardedRef']}, props['children']); +/// } +/// } +/// final _logPropsHoc = react.registerComponent2(() => _LogProps()); +/// +/// final LogProps = react.forwardRef((props, ref) { +/// // Note the second param "ref" provided by react.forwardRef. +/// // 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}); +/// // Optional: Make the displayName more useful for the React dev tools. +/// // See: https://reactjs.org/docs/forwarding-refs.html#displaying-a-custom-name-in-devtools +/// }, displayName: 'LogProps(${_logPropsHoc.type.displayName})'); +/// +/// // ---------- Component Consumption ---------- +/// void main() { +/// setClientConfiguration(); +/// final ref = react.createRef(); +/// +/// react_dom.render(LogProps({'component': FancyButton, 'ref': ref}), +/// querySelector('#idOfSomeNodeInTheDom')); +/// +/// // You can still get a ref directly to the DOM button: +/// final buttonNode = ref.current; +/// } +/// ``` /// See: . -ReactJsComponentFactoryProxy forwardRef(Function(Map props, Ref ref) wrapperFunction) { - var hoc = React.forwardRef(allowInterop((JsMap props, JsRef ref) { +ReactJsComponentFactoryProxy forwardRef( + Function(Map props, Ref ref) wrapperFunction, { + String displayName = 'Anonymous', +}) { + final wrappedComponent = allowInterop((JsMap props, JsRef ref) { final dartProps = JsBackedMap.backedBy(props); + for (var value in dartProps.values) { + if (value is Function) { + // Tag functions that came straight from the JS + // so that we know to pass them through as-is during prop conversion. + isRawJsFunctionFromProps[value] = true; + } + } + final dartRef = Ref.fromJs(ref); return wrapperFunction(dartProps, dartRef); - })); + }); + defineProperty(wrappedComponent, 'displayName', jsify({'value': displayName})); + + var hoc = React.forwardRef(wrappedComponent); - return new ReactJsComponentFactoryProxy(hoc, shouldConvertDomProps: false); + return ReactJsComponentFactoryProxy(hoc); } abstract class ReactDom { diff --git a/lib/src/react_client/factory_util.dart b/lib/src/react_client/factory_util.dart index 8b8a35a7..9fad7d5e 100644 --- a/lib/src/react_client/factory_util.dart +++ b/lib/src/react_client/factory_util.dart @@ -17,9 +17,6 @@ import 'package:react/src/typedefs.dart'; import 'event_prop_key_to_event_factory.dart'; -@JS('Object.defineProperty') -external void defineProperty(dynamic object, String propertyName, JsMap descriptor); - /// Converts a list of variadic children arguments to children that should be passed to ReactJS. /// /// Returns: @@ -44,14 +41,26 @@ convertEventHandlers(Map args) { args.forEach((propKey, value) { var eventFactory = eventPropKeyToEventFactory[propKey]; if (eventFactory != null && value != null) { - // Apply allowInterop here so that the function we store in `originalEventHandlers` - // is the same one we'll retrieve from the JS props. - var reactDartConvertedEventHandler = allowInterop((events.SyntheticEvent e, [_, __]) { - value(eventFactory(e)); - }); - - args[propKey] = reactDartConvertedEventHandler; - originalEventHandlers[reactDartConvertedEventHandler] = value; + // Don't attempt to convert functions that have already been converted, or functions + // that were passed in as JS props. + final handlerHasAlreadyBeenConverted = unconvertJsEventHandler(value) != null; + if (!handlerHasAlreadyBeenConverted && !(isRawJsFunctionFromProps[value] ?? false)) { + // Apply allowInterop here so that the function we store in [_originalEventHandlers] + // is the same one we'll retrieve from the JS props. + var reactDartConvertedEventHandler = allowInterop((e, [_, __]) { + // To support Dart code calling converted handlers, + // check for Dart events and pass them through directly. + // Otherwise, convert the JS events like normal. + if (e is SyntheticEvent) { + value(e); + } else { + value(eventFactory(e as events.SyntheticEvent)); + } + }); + + args[propKey] = reactDartConvertedEventHandler; + originalEventHandlers[reactDartConvertedEventHandler] = value; + } } }); } @@ -63,24 +72,34 @@ void convertRefValue(Map args) { } } -void convertRefValue2(Map args, {bool convertCallbackRefValue = true}) { - var ref = args['ref']; - - if (ref is Ref) { - args['ref'] = ref.jsRef; - // If the ref is a callback, pass ReactJS a function that will call it - // with the Dart Component instance, not the ReactComponent instance. - // - // Use CallbackRef to check arity, since parameters could be non-dynamic, and thus - // would fail the `is CallbackRef` check. - // See https://github.com/dart-lang/sdk/issues/34593 for more information on arity checks. - } else if (ref is CallbackRef && convertCallbackRefValue) { - args['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. - if (instance is ReactComponent && instance.dartComponent != null) return (ref as dynamic)(instance.dartComponent); - return (ref as dynamic)(instance); - }); +void convertRefValue2( + Map args, { + bool convertCallbackRefValue = true, + List additionalRefPropKeys = const [], +}) { + final refKeys = ['ref', ...additionalRefPropKeys]; + + for (final refKey in refKeys) { + var ref = args[refKey]; + if (ref is Ref) { + args[refKey] = ref.jsRef; + // If the ref is a callback, pass ReactJS a function that will call it + // with the Dart Component instance, not the ReactComponent instance. + // + // Use _CallbackRef to check arity, since parameters could be non-dynamic, and thus + // would fail the `is _CallbackRef` check. + // See https://github.com/dart-lang/sdk/issues/34593 for more information on arity checks. + } else if (ref is CallbackRef && convertCallbackRefValue) { + args[refKey] = 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. + if (instance is ReactComponent && instance.dartComponent != null) { + return (ref as dynamic)(instance.dartComponent); + } + + return (ref as dynamic)(instance); + }); + } } } @@ -126,11 +145,15 @@ JsMap generateJsProps(Map props, {bool shouldConvertEventHandlers = true, bool convertRefValue = true, bool convertCallbackRefValue = true, + List additionalRefPropKeys = const [], bool wrapWithJsify = true}) { final propsForJs = JsBackedMap.from(props); if (shouldConvertEventHandlers) convertEventHandlers(propsForJs); - if (convertRefValue) convertRefValue2(propsForJs, convertCallbackRefValue: convertCallbackRefValue); + if (convertRefValue) { + convertRefValue2(propsForJs, + convertCallbackRefValue: convertCallbackRefValue, additionalRefPropKeys: additionalRefPropKeys); + } return wrapWithJsify ? jsifyAndAllowInterop(propsForJs) : propsForJs.jsObject; } diff --git a/test/factory/common_factory_tests.dart b/test/factory/common_factory_tests.dart index a0d8da29..18a34c76 100644 --- a/test/factory/common_factory_tests.dart +++ b/test/factory/common_factory_tests.dart @@ -1,12 +1,17 @@ +@JS() +library react.test.common_factory_tests; + // ignore_for_file: deprecated_member_use_from_same_package import 'dart:async'; import 'dart:html'; import 'dart:js'; import 'dart:js_util'; +import 'package:js/js.dart'; import 'package:meta/meta.dart'; import 'package:react/react_client/component_factory.dart'; import 'package:react/react_client/js_backed_map.dart'; +import 'package:react/react_client/js_interop_helpers.dart'; import 'package:test/test.dart'; import 'package:react/react.dart' as react; @@ -270,9 +275,160 @@ void refTests(ReactComponentFactoryProxy factory, {void verifyRefValue(dynami 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('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'); + }); + }); + + group('forwardRef wraps event handlers properly,', () { + final isDartComponent = factory is ReactDartComponentFactoryProxy; + + 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 ? '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('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())); + }); + } + }); + + if (isDartComponent) { + 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'); + }); + + 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); + }); + } + }); + } + }); + _typedCallbackRefTests(factory); } +class EventTestCase { + /// The prop key for an arbitrary event handler that will be used to test this case. + final String eventPropKey; + + /// A description of this test case. + final String description; + + /// Whether the tested event handler is a Dart function expecting a Dart synthetic event, + /// as opposed to a JS function expecting a JS synthetic event. + final bool isDart; + + const EventTestCase.dart(this.eventPropKey, String description) + : isDart = true, + description = 'Dart handler $description'; + + const EventTestCase.js(this.eventPropKey, String description) + : isDart = false, + description = 'JS handler $description'; + + String get _camelCaseEventName { + var name = eventPropKey.replaceFirst(RegExp(r'^on'), ''); + name = name.substring(0, 1).toLowerCase() + name.substring(1); + return name; + } + + void simulate(Element node) => callMethod(_Simulate, _camelCaseEventName, [node]); + + @override + String toString() => 'EventHelper: ($eventPropKey) $description'; +} + +@JS('React.addons.TestUtils.Simulate') +external dynamic get _Simulate; + void _typedCallbackRefTests(react.ReactComponentFactoryProxy factory) { if (T == dynamic) { throw ArgumentError('Generic parameter T must be specified'); diff --git a/test/factory/dart_factory_test.dart b/test/factory/dart_factory_test.dart index 43a0dc62..76d9e279 100644 --- a/test/factory/dart_factory_test.dart +++ b/test/factory/dart_factory_test.dart @@ -1,13 +1,10 @@ // ignore_for_file: deprecated_member_use_from_same_package // ignore_for_file: invalid_use_of_protected_member @TestOn('browser') -import 'dart:js'; - import 'package:test/test.dart'; import 'package:react/react.dart' as react; import 'package:react/react_client.dart'; -import 'package:react/react_client/react_interop.dart'; import 'common_factory_tests.dart'; @@ -49,17 +46,18 @@ final Foo = react.registerComponent(() => new _Foo()) as ReactDartComponentFacto class _Foo extends react.Component { @override - render() => react.div({}); + render() { + props['onDartRender']?.call(props); + return react.div({...props, 'ref': props['forwardedRef']}); + } } final Foo2 = react.registerComponent(() => new _Foo2()) as ReactDartComponentFactoryProxy2; class _Foo2 extends react.Component2 { @override - render() => react.div({}); + render() { + props['onDartRender']?.call(props); + return react.div({...props, 'ref': props['forwardedRef']}); + } } - -final JsFoo = ReactJsComponentFactoryProxy(React.createClass(ReactClassConfig( - displayName: 'JsFoo', - render: allowInterop(() => react.div({})), -))); diff --git a/test/factory/dart_function_factory_test.dart b/test/factory/dart_function_factory_test.dart index 73711b8f..43663e73 100644 --- a/test/factory/dart_function_factory_test.dart +++ b/test/factory/dart_function_factory_test.dart @@ -47,5 +47,6 @@ final NamedFunctionFoo = react.registerFunctionComponent(_FunctionFoo, displayNa final FunctionFoo = react.registerFunctionComponent(_FunctionFoo); _FunctionFoo(Map props) { + props['onDartRender']?.call(props); return react.div({}); } diff --git a/test/factory/js_factory_test.dart b/test/factory/js_factory_test.dart index 251ae601..cda82b57 100644 --- a/test/factory/js_factory_test.dart +++ b/test/factory/js_factory_test.dart @@ -2,9 +2,13 @@ @TestOn('browser') library js_factory_test; +import 'dart:html'; +import 'dart:js_util'; + import 'package:js/js.dart'; import 'package:test/test.dart'; +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'; @@ -36,3 +40,6 @@ main() { @JS() external ReactClass get _JsFoo; final JsFoo = new ReactJsComponentFactoryProxy(_JsFoo); + +@JS() +external ReactClass get _JsFooFunction; diff --git a/test/factory/js_factory_test.js b/test/factory/js_factory_test.js index 9541a002..ab099ff0 100644 --- a/test/factory/js_factory_test.js +++ b/test/factory/js_factory_test.js @@ -3,3 +3,15 @@ window._JsFoo = class JsFooComponent extends React.Component { return React.createElement("div", this.props, this.props.children); } }; + +window._JsFooFunction = React.forwardRef((props, ref) => ( + React.createElement("div", {...props, 'ref': ref, 'onClick': function(event) { + if (props['onClickJs'] !== undefined) { + props['onClickJs'](event); + } + + if (props['onClick'] !== undefined) { + props['onClick'](event); + } + }}, React.createElement("div", {'ref': props['innerRef']}, props['children'])) +));