From e5f8e7810f8c2705ebbcc98a13a2eb42d2562d8f Mon Sep 17 00:00:00 2001 From: Aaron Lademann Date: Thu, 23 Jan 2020 13:10:38 -0700 Subject: [PATCH 1/4] Allow displayName to be passed to forwardRef --- lib/src/component/ref_util.dart | 22 +++++++- pubspec.yaml | 8 +++ .../component/forward_ref_test.dart | 53 +++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/lib/src/component/ref_util.dart b/lib/src/component/ref_util.dart index 1d518d21a..4a8eb0af7 100644 --- a/lib/src/component/ref_util.dart +++ b/lib/src/component/ref_util.dart @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +import 'dart:js_util'; + import 'package:react/react_client/react_interop.dart' as react_interop; import 'package:react/react_client.dart'; import 'package:over_react/component_base.dart'; @@ -91,13 +93,29 @@ Ref createRef() { /// /// Learn more: . UiFactory Function(UiFactory) forwardRef( - Function(TProps props, Ref ref) wrapperFunction) { + Function(TProps props, Ref ref) wrapperFunction, {String displayName}) { UiFactory wrapWithForwardRef(UiFactory factory) { + if (displayName == null) { + final componentFactoryType = factory().componentFactory.type; + if (componentFactoryType is String) { + // DomComponent + displayName = componentFactoryType; + } else if (componentFactoryType is Function) { + // JS component factories, Dart function components, Dart composite components + displayName = getProperty(componentFactoryType, 'displayName'); + + if (displayName == null) { + final ctor = getProperty(componentFactoryType, 'constructor'); + displayName = (ctor == null ? null : getProperty(ctor, 'name')) ?? 'Anonymous'; + } + } + } + Object wrapProps(Map props, Ref ref) { return wrapperFunction(factory(props), ref); } - ReactComponentFactoryProxy hoc = react_interop.forwardRef(wrapProps); + ReactComponentFactoryProxy hoc = react_interop.forwardRef(wrapProps, displayName: displayName); TProps forwardedFactory([Map props]) { return factory(props)..componentFactory = hoc; diff --git a/pubspec.yaml b/pubspec.yaml index 1437481a4..c40960c28 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -47,3 +47,11 @@ dev_dependencies: over_react_test: ^2.7.0 pedantic: ^1.8.0 test: ^1.9.1 + + +dependency_overrides: + react: + git: + url: https://github.com/cleandart/react-dart.git + ref: 5c19d8d43d9d3c7b115da9d1f7c0bd62a41f5a5f # from 5.4.0/improve-forwardref-examples-and-typing + diff --git a/test/over_react/component/forward_ref_test.dart b/test/over_react/component/forward_ref_test.dart index c55125163..eea43f631 100644 --- a/test/over_react/component/forward_ref_test.dart +++ b/test/over_react/component/forward_ref_test.dart @@ -15,6 +15,7 @@ library forward_ref_test; import 'dart:html'; +import 'dart:js_util'; import 'package:test/test.dart'; import 'package:over_react/over_react.dart'; @@ -47,12 +48,64 @@ main() { expect(refObject.current, TypeMatcher()); }); + + group('- sets displayName on the rendered component as expected', () { + test('when displayName argument is not passed to forwardRef', () { + UiFactory DivForwarded = forwardRef((props, ref) { + return (Dom.div() + ..ref = ref + )(); + })(Dom.div); + + final refObject = createRef(); + final vDomElement = (DivForwarded()..ref = refObject)(); + + expect(getProperty(getProperty(vDomElement.type, 'render'), 'displayName'), 'div'); + }); + + test('when displayName argument is passed to forwardRef', () { + UiFactory DivForwarded = forwardRef((props, ref) { + return (Dom.div() + ..ref = ref + )(); + }, displayName: 'SomethingCustom')(Dom.div); + + final refObject = createRef(); + final vDomElement = (DivForwarded()..ref = refObject)(); + + expect(getProperty(getProperty(vDomElement.type, 'render'), 'displayName'), 'SomethingCustom'); + }); + }); }); group('on a component with a dart component child', () { forwardRefTest(BasicChild, verifyRefValue: (ref) { expect(ref, TypeMatcher()); }); + + group('- sets displayName on the rendered component as expected', () { + test('when displayName argument is not passed to forwardRef', () { + UiFactory BasicForwarded = forwardRef((props, ref) { + return (BasicChild()..ref = ref)(); + })(Basic); + + final Ref refObject = createRef(); + final vDomElement = (BasicForwarded()..ref = refObject)(); + + expect(getProperty(getProperty(vDomElement.type, 'render'), 'displayName'), 'Basic'); + }); + + test('when displayName argument is passed to forwardRef', () { + UiFactory BasicForwarded = forwardRef((props, ref) { + return (BasicChild()..ref = ref)(); + }, displayName: 'BasicForwarded')(Basic); + + final Ref refObject = createRef(); + final vDomElement = (BasicForwarded()..ref = refObject)(); + + expect(getProperty(getProperty(vDomElement.type, 'render'), 'displayName'), 'BasicForwarded'); + }); + }); }); }); } From 9c8825ed532679fe30cb3b70bbf7755492c19348 Mon Sep 17 00:00:00 2001 From: Aaron Lademann Date: Thu, 23 Jan 2020 13:19:46 -0700 Subject: [PATCH 2/4] Improve / add forwardRef examples --- lib/src/component/ref_util.dart | 143 +++++++++++--- web/component2/index.dart | 13 ++ web/component2/index.html | 6 + web/component2/src/demos.dart | 1 + web/component2/src/demos/forward_ref.dart | 86 +++++++++ .../src/demos/forward_ref.over_react.g.dart | 181 ++++++++++++++++++ 6 files changed, 401 insertions(+), 29 deletions(-) create mode 100644 web/component2/src/demos/forward_ref.dart create mode 100644 web/component2/src/demos/forward_ref.over_react.g.dart diff --git a/lib/src/component/ref_util.dart b/lib/src/component/ref_util.dart index 4a8eb0af7..724160370 100644 --- a/lib/src/component/ref_util.dart +++ b/lib/src/component/ref_util.dart @@ -54,42 +54,127 @@ Ref createRef() { /// Automatically passes a [Ref] through a component to one of its children. /// -/// __Example__: +/// __Example 1:__ Forwarding refs to DOM components +/// +/// ```dart +/// import 'dart:html'; +/// import 'package:over_react/over_react.dart'; +/// import 'package:over_react/react_dom.dart' as react_dom; +/// +/// // ---------- Component Definition ---------- +/// +/// final FancyButton = forwardRef((props, ref) { +/// final classes = ClassNameBuilder.fromProps(props) +/// ..add('FancyButton'); +/// +/// return (Dom.button() +/// ..addProps(getPropsToForward(props, onlyCopyDomProps: true)) +/// ..className = classes.toClassName() +/// ..ref = ref +/// )('Click me!'); +/// })(Dom.button); +/// +/// // ---------- Component Consumption ---------- +/// +/// void main() { +/// setClientConfiguration(); +/// final ref = createRef(); /// -/// UiFactory DivForwarded = forwardRef((props, ref) { -/// return (Dom.div() +/// react_dom.render( +/// (FancyButton() /// ..ref = ref -/// ..className = 'special-class' -/// )( -/// props.children -/// ); -/// })(Dom.div); +/// ..onClick = (_) { +/// print(ref.current.outerHtml); +/// } +/// )(), +/// querySelector('#idOfSomeNodeInTheDom') +/// ); /// -/// ___ OR ___ +/// // You can still get a ref directly to the DOM button: +/// final buttonNode = ref.current; +/// } +/// ``` /// -/// UiFactory FooForwarded = forwardRef((props, ref) { -/// return (Foo() -/// ..forwardedRef = ref -/// )(); -/// })(Foo); +/// __Example 2:__ Forwarding refs in higher-order components /// -/// @Factory() -/// UiFactory Foo = _$Foo; +/// ```dart +/// import 'dart:html'; +/// import 'package:over_react/over_react.dart'; +/// import 'package:over_react/react_dom.dart' as react_dom; /// -/// @Props() -/// class _$FooProps extends UiProps { -/// Ref forwardedRef; -/// } +/// // ---------- Component Definitions ---------- /// -/// @Component2() -/// class FooComponent extends UiComponent2 { -/// @override -/// render() { -/// return (Dom.button() -/// ..ref = props.forwardedRef -/// )('Click this button'); -/// } -/// } +/// final FancyButton = forwardRef((props, ref) { +/// final classes = ClassNameBuilder.fromProps(props) +/// ..add('FancyButton'); +/// +/// return (Dom.button() +/// ..addProps(getPropsToForward(props, onlyCopyDomProps: true)) +/// ..className = classes.toClassName() +/// ..ref = ref +/// )('Click me!'); +/// })(Dom.button); +/// +/// final LogProps = forwardRef((props, ref) { +/// return (_LogProps() +/// ..addProps(props) +/// .._forwardedRef = ref +/// )('Click me!'); +/// })(_LogProps); +/// +/// @Factory() +/// UiFactory _LogProps = _$_LogProps; +/// +/// @Props() +/// class _$LogPropsProps extends UiProps { +/// BuilderOnlyUiFactory builder; +/// +/// // Private since we only use this to pass along the value of `ref` to +/// // the return value of forwardRef. +/// // +/// // Consumers can set this private field value using the public `ref` setter. +/// Ref _forwardedRef; +/// } +/// +/// @Component2() +/// class LogPropsComponent extends UiComponent2 { +/// @override +/// void componentDidUpdate(Map prevProps, _, [__]) { +/// print('old props: $prevProps'); +/// print('new props: $props'); +/// } +/// +/// @override +/// render() { +/// return (props.builder() +/// ..modifyProps(addUnconsumedDomProps) +/// ..ref = props._forwardedRef +/// )(props.children); +/// } +/// } +/// +/// // ---------- Component Consumption ---------- +/// +/// void main() { +/// setClientConfiguration(); +/// final ref = createRef(); +/// +/// react_dom.render( +/// (LogProps() +/// ..builder = FancyButton +/// ..className = 'btn btn-primary' +/// ..ref = ref +/// ..onClick = (_) { +/// print(ref.current.outerHtml); +/// } +/// )(), +/// querySelector('#idOfSomeNodeInTheDom') +/// ); +/// +/// // You can still get a ref directly to the DOM button: +/// final buttonNode = ref.current; +/// } +/// ``` /// /// Learn more: . UiFactory Function(UiFactory) forwardRef( diff --git a/web/component2/index.dart b/web/component2/index.dart index 686bc9e80..ffd070342 100644 --- a/web/component2/index.dart +++ b/web/component2/index.dart @@ -7,6 +7,7 @@ import 'src/demos.dart'; void main() { setClientConfiguration(); + final fancyButtonNodeRef = createRef(); react_dom.render( buttonExamplesDemo(), querySelector('$demoMountNodeSelectorPrefix--button')); @@ -26,6 +27,18 @@ void main() { react_dom.render( radioToggleButtonDemo(), querySelector('$demoMountNodeSelectorPrefix--radio-toggle')); + react_dom.render( + (LogProps() + ..builder = FancyButton + ..className = 'btn btn-primary' + ..ref = fancyButtonNodeRef + ..onClick = (_) { + print(fancyButtonNodeRef.current.outerHtml); + } + )(), + querySelector('$demoMountNodeSelectorPrefix--forwardRef'), + ); + react_dom.render( (ErrorBoundary() ..onComponentDidCatch = (error, info) { diff --git a/web/component2/index.html b/web/component2/index.html index 69cc334b3..57cd22da9 100644 --- a/web/component2/index.html +++ b/web/component2/index.html @@ -51,6 +51,12 @@

ToggleButton (Radio)

around with the component rendered below.

+
+

ForwardRef

+

Modify the source of /web/component2/src/demos/forward_ref.dart to play + around with the component rendered below.

+
+

Component That Throws A Caught Error

(Default ErrorBoundary Component)

diff --git a/web/component2/src/demos.dart b/web/component2/src/demos.dart index 4b9e16d8a..96bec89f1 100644 --- a/web/component2/src/demos.dart +++ b/web/component2/src/demos.dart @@ -9,6 +9,7 @@ export 'demos/button/button-active.dart'; export 'demos/button/button-disabled.dart'; export 'demos/custom_error_boundary.dart'; +export 'demos/forward_ref.dart'; export '../src/demo_components/prop_validation_wrap.dart'; diff --git a/web/component2/src/demos/forward_ref.dart b/web/component2/src/demos/forward_ref.dart new file mode 100644 index 000000000..99a4a2d41 --- /dev/null +++ b/web/component2/src/demos/forward_ref.dart @@ -0,0 +1,86 @@ +import 'package:over_react/over_react.dart'; + +// ignore: uri_has_not_been_generated +part 'forward_ref.over_react.g.dart'; + +final FancyButton = forwardRef((props, ref) { + final classes = ClassNameBuilder.fromProps(props) + ..add('FancyButton'); + + return (Dom.button() + ..addProps(getPropsToForward(props, onlyCopyDomProps: true)) + ..className = classes.toClassName() + ..ref = ref + )('Click me!'); +}, displayName: 'FancyButton')(Dom.button); + +final LogProps = forwardRef((props, ref) { + return (_LogProps() + ..addProps(props) + .._forwardedRef = ref + )('Click me!'); +}, displayName: 'LogProps')(_LogProps); + +@Factory() +UiFactory _LogProps = _$_LogProps; + +@Props() +class _$LogPropsProps extends UiProps { + BuilderOnlyUiFactory builder; + + // Private since we only use this to pass along the value of `ref` to + // the return value of forwardRef. + // + // Consumers can set this private field value using the public `ref` setter. + Ref _forwardedRef; +} + +@Component2() +class LogPropsComponent extends UiComponent2 { + @override + void componentDidUpdate(Map prevProps, _, [__]) { + print('old props: $prevProps'); + print('new props: $props'); + } + + @override + render() { + return (props.builder() + ..modifyProps(addUnconsumedDomProps) + ..ref = props._forwardedRef + )(props.children); + } +} + +//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})'); +// +//final LogProps = forwardRef((props, ref) { +// return (_LogProps() +// ..addProps(props) +// .._forwardedRef = ref +// )(props.children); +//})(_LogProps); diff --git a/web/component2/src/demos/forward_ref.over_react.g.dart b/web/component2/src/demos/forward_ref.over_react.g.dart new file mode 100644 index 000000000..ff2b05aa5 --- /dev/null +++ b/web/component2/src/demos/forward_ref.over_react.g.dart @@ -0,0 +1,181 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +// ignore_for_file: deprecated_member_use_from_same_package, unnecessary_null_in_if_null_operators, prefer_null_aware_operators +part of 'forward_ref.dart'; + +// ************************************************************************** +// OverReactBuilder (package:over_react/src/builder.dart) +// ************************************************************************** + +// React component factory implementation. +// +// Registers component implementation and links type meta to builder factory. +final $LogPropsComponentFactory = registerComponent2( + () => _$LogPropsComponent(), + builderFactory: _LogProps, + componentClass: LogPropsComponent, + isWrapper: false, + parentType: null, + displayName: '_LogProps', +); + +abstract class _$LogPropsPropsAccessorsMixin implements _$LogPropsProps { + @override + Map get props; + + /// + @override + BuilderOnlyUiFactory get builder => + props[_$key__builder___$LogPropsProps] ?? + null; // Add ` ?? null` to workaround DDC bug: ; + /// + @override + set builder(BuilderOnlyUiFactory value) => + props[_$key__builder___$LogPropsProps] = value; + + /// + @override + Ref get _forwardedRef => + props[_$key___forwardedRef___$LogPropsProps] ?? + null; // Add ` ?? null` to workaround DDC bug: ; + /// + @override + set _forwardedRef(Ref value) => + props[_$key___forwardedRef___$LogPropsProps] = value; + /* GENERATED CONSTANTS */ + static const PropDescriptor _$prop__builder___$LogPropsProps = + PropDescriptor(_$key__builder___$LogPropsProps); + static const PropDescriptor _$prop___forwardedRef___$LogPropsProps = + PropDescriptor(_$key___forwardedRef___$LogPropsProps); + static const String _$key__builder___$LogPropsProps = 'LogPropsProps.builder'; + static const String _$key___forwardedRef___$LogPropsProps = + 'LogPropsProps._forwardedRef'; + + static const List $props = [ + _$prop__builder___$LogPropsProps, + _$prop___forwardedRef___$LogPropsProps + ]; + static const List $propKeys = [ + _$key__builder___$LogPropsProps, + _$key___forwardedRef___$LogPropsProps + ]; +} + +const PropsMeta _$metaForLogPropsProps = PropsMeta( + fields: _$LogPropsPropsAccessorsMixin.$props, + keys: _$LogPropsPropsAccessorsMixin.$propKeys, +); + +class LogPropsProps extends _$LogPropsProps with _$LogPropsPropsAccessorsMixin { + static const PropsMeta meta = _$metaForLogPropsProps; +} + +_$$LogPropsProps _$_LogProps([Map backingProps]) => backingProps == null + ? _$$LogPropsProps$JsMap(JsBackedMap()) + : _$$LogPropsProps(backingProps); + +// Concrete props implementation. +// +// Implements constructor and backing map, and links up to generated component factory. +abstract class _$$LogPropsProps extends _$LogPropsProps + with _$LogPropsPropsAccessorsMixin + implements LogPropsProps { + _$$LogPropsProps._(); + + factory _$$LogPropsProps(Map backingMap) { + if (backingMap == null || backingMap is JsBackedMap) { + return _$$LogPropsProps$JsMap(backingMap); + } else { + return _$$LogPropsProps$PlainMap(backingMap); + } + } + + /// Let `UiProps` internals know that this class has been generated. + @override + bool get $isClassGenerated => true; + + /// The `ReactComponentFactory` associated with the component built by this class. + @override + ReactComponentFactoryProxy get componentFactory => + super.componentFactory ?? $LogPropsComponentFactory; + + /// The default namespace for the prop getters/setters generated for this class. + @override + String get propKeyNamespace => 'LogPropsProps.'; +} + +// Concrete props implementation that can be backed by any [Map]. +class _$$LogPropsProps$PlainMap extends _$$LogPropsProps { + // This initializer of `_props` to an empty map, as well as the reassignment + // of `_props` in the constructor body is necessary to work around a DDC bug: https://github.com/dart-lang/sdk/issues/36217 + _$$LogPropsProps$PlainMap(Map backingMap) + : this._props = {}, + super._() { + this._props = backingMap ?? {}; + } + + /// The backing props map proxied by this class. + @override + Map get props => _props; + Map _props; +} + +// Concrete props implementation that can only be backed by [JsMap], +// allowing dart2js to compile more optimal code for key-value pair reads/writes. +class _$$LogPropsProps$JsMap extends _$$LogPropsProps { + // This initializer of `_props` to an empty map, as well as the reassignment + // of `_props` in the constructor body is necessary to work around a DDC bug: https://github.com/dart-lang/sdk/issues/36217 + _$$LogPropsProps$JsMap(JsBackedMap backingMap) + : this._props = JsBackedMap(), + super._() { + this._props = backingMap ?? JsBackedMap(); + } + + /// The backing props map proxied by this class. + @override + JsBackedMap get props => _props; + JsBackedMap _props; +} + +// Concrete component implementation mixin. +// +// Implements typed props/state factories, defaults `consumedPropKeys` to the keys +// generated for the associated props class. +class _$LogPropsComponent extends LogPropsComponent { + _$$LogPropsProps$JsMap _cachedTypedProps; + + @override + _$$LogPropsProps$JsMap get props => _cachedTypedProps; + + @override + set props(Map value) { + assert( + getBackingMap(value) is JsBackedMap, + 'Component2.props should never be set directly in ' + 'production. If this is required for testing, the ' + 'component should be rendered within the test. If ' + 'that does not have the necessary result, the last ' + 'resort is to use typedPropsFactoryJs.'); + super.props = value; + _cachedTypedProps = typedPropsFactoryJs(getBackingMap(value)); + } + + @override + _$$LogPropsProps$JsMap typedPropsFactoryJs(JsBackedMap backingMap) => + _$$LogPropsProps$JsMap(backingMap); + + @override + _$$LogPropsProps typedPropsFactory(Map backingMap) => + _$$LogPropsProps(backingMap); + + /// Let `UiComponent` internals know that this class has been generated. + @override + bool get $isClassGenerated => true; + + /// The default consumed props, taken from _$LogPropsProps. + /// Used in `ConsumedProps` if [consumedProps] is not overridden. + @override + final List $defaultConsumedProps = const [ + _$metaForLogPropsProps + ]; +} From 6ca4f3b2fb91c1eb666885ab9696ac702ce77a7a Mon Sep 17 00:00:00 2001 From: Smai Fullerton Date: Wed, 22 Jul 2020 10:52:58 -0600 Subject: [PATCH 3/4] Consume react ^5.4.0 --- pubspec.yaml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/pubspec.yaml b/pubspec.yaml index 1f62812c4..3639061ce 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -20,7 +20,7 @@ dependencies: memoize: ^2.0.0 meta: ^1.1.6 path: ^1.5.1 - react: ^5.3.0 + react: ^5.4.0 redux: ">=3.0.0 <5.0.0" source_span: ^1.4.1 transformer_utils: ^0.2.0 @@ -46,10 +46,4 @@ dev_dependencies: over_react_test: ^2.7.0 pedantic: ^1.8.0 test: ^1.9.1 - yaml: ^2.2.1 - -dependency_overrides: - react: - git: - url: https://github.com/cleandart/react-dart.git - ref: 5dcf94dc177f19027f35398109686646073faf84 # from 5.4.0/improve-forwardref-examples-and-typing \ No newline at end of file + yaml: ^2.2.1 \ No newline at end of file From 04b7b2cf3232a240dcefd946eb53d15991b1e71c Mon Sep 17 00:00:00 2001 From: Smai Fullerton Date: Wed, 29 Jul 2020 17:37:28 -0600 Subject: [PATCH 4/4] Update built files --- web/component2/src/demos/forward_ref.over_react.g.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/component2/src/demos/forward_ref.over_react.g.dart b/web/component2/src/demos/forward_ref.over_react.g.dart index ff2b05aa5..bff7b6602 100644 --- a/web/component2/src/demos/forward_ref.over_react.g.dart +++ b/web/component2/src/demos/forward_ref.over_react.g.dart @@ -12,7 +12,7 @@ part of 'forward_ref.dart'; // Registers component implementation and links type meta to builder factory. final $LogPropsComponentFactory = registerComponent2( () => _$LogPropsComponent(), - builderFactory: _LogProps, + builderFactory: _$_LogProps, componentClass: LogPropsComponent, isWrapper: false, parentType: null, @@ -173,7 +173,7 @@ class _$LogPropsComponent extends LogPropsComponent { bool get $isClassGenerated => true; /// The default consumed props, taken from _$LogPropsProps. - /// Used in `ConsumedProps` if [consumedProps] is not overridden. + /// Used in `*ConsumedProps` methods if [consumedProps] is not overridden. @override final List $defaultConsumedProps = const [ _$metaForLogPropsProps