From 11fbf7d89af4c5c173ac54a54f6b38125e12c537 Mon Sep 17 00:00:00 2001 From: Greg Littlefield Date: Fri, 27 Sep 2024 15:11:29 -0700 Subject: [PATCH 1/3] Fix dummyEvent setup that never should have worked --- test/factory/common_factory_tests.dart | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/test/factory/common_factory_tests.dart b/test/factory/common_factory_tests.dart index 5ac6729e..91767d53 100644 --- a/test/factory/common_factory_tests.dart +++ b/test/factory/common_factory_tests.dart @@ -277,15 +277,22 @@ void domEventHandlerWrappingTests(ReactComponentFactoryProxy factory) { reason: 'test setup: component must pass props into props.onDartRender'); }); - late react.SyntheticMouseEvent event; - final divRef = react.createRef(); - render(react.div({ - 'ref': divRef, - 'onClick': (react.SyntheticMouseEvent e) => event = e, - })); - rtu.Simulate.click(divRef); + late react.SyntheticMouseEvent dummyEvent; + setUpAll(() { + final mountNode = DivElement(); + document.body!.append(mountNode); + addTearDown(() { + react_dom.unmountComponentAtNode(mountNode); + mountNode.remove(); + }); - final dummyEvent = event; + final divRef = react.createRef(); + react_dom.render(react.div({ + 'ref': divRef, + 'onClick': (react.SyntheticMouseEvent e) => dummyEvent = e, + }), mountNode); + divRef.current!.click(); + }); for (final eventCase in eventCases.where((helper) => helper.isDart)) { test(eventCase.description, () { From c80f88fb6dee7cb2004b2dca94b0736cb10a4e3e Mon Sep 17 00:00:00 2001 From: Greg Littlefield Date: Fri, 27 Sep 2024 15:24:29 -0700 Subject: [PATCH 2/3] Add lazy test coverage, add wrapper in react-dart --- lib/react_client/react_interop.dart | 17 ++++++- test/factory/common_factory_tests.dart | 11 ++-- test/react_lazy_test.dart | 69 +++++++++++++++++++++++--- test/react_lazy_test.html | 1 + 4 files changed, 84 insertions(+), 14 deletions(-) diff --git a/lib/react_client/react_interop.dart b/lib/react_client/react_interop.dart index 661618b6..764f8db2 100644 --- a/lib/react_client/react_interop.dart +++ b/lib/react_client/react_interop.dart @@ -312,14 +312,27 @@ ReactComponentFactoryProxy lazy(Future Function() lo () => futureToPromise( (() async { final factory = await load(); - return jsify({'default': factory.type}); + // By using a wrapper uiForwardRef it ensures that we have a matching factory proxy type given to react-dart's lazy, + // a `ReactDartWrappedComponentFactoryProxy`. This is necessary to have consistent prop conversions since we don't + // have access to the original factory proxy outside of this async block. + final wrapper = forwardRef2((props, ref) { + final children = props['children']; + return factory.build( + {...props, 'ref': ref}, + [ + if (children != null && !(children is List && children.isEmpty)) children, + ], + ); + }); + return jsify({'default': wrapper.type}); })(), ), ), ); + // Setting this version and wrapping with ReactDartWrappedComponentFactoryProxy + // is only okay because it matches the version and factory proxy of the wrapperFactory above. setProperty(hoc, 'dartComponentVersion', ReactDartComponentVersion.component2); - return ReactDartWrappedComponentFactoryProxy(hoc); } diff --git a/test/factory/common_factory_tests.dart b/test/factory/common_factory_tests.dart index 91767d53..c2fc2edf 100644 --- a/test/factory/common_factory_tests.dart +++ b/test/factory/common_factory_tests.dart @@ -28,6 +28,7 @@ import '../util.dart'; void commonFactoryTests(ReactComponentFactoryProxy factory, {String? dartComponentVersion, bool skipPropValuesTest = false, + bool isNonDartComponentWithDartWrapper = false, ReactElement Function(dynamic children)? renderWrapper}) { _childKeyWarningTests( factory, @@ -115,7 +116,7 @@ void commonFactoryTests(ReactComponentFactoryProxy factory, shouldAlwaysBeList: isDartComponent2(factory({}))); }); - if (isDartComponent(factory({}))) { + if (isDartComponent(factory({})) && !isNonDartComponentWithDartWrapper) { group('passes children to the Dart component when specified as', () { final notCalledSentinelValue = Object(); dynamic childrenFromLastRender; @@ -173,7 +174,7 @@ void commonFactoryTests(ReactComponentFactoryProxy factory, } } - if (isDartComponent2(factory({}))) { + if (isDartComponent2(factory({})) && !isNonDartComponentWithDartWrapper) { test('executes Dart render code in the component zone', () { final oldComponentZone = componentZone; addTearDown(() => componentZone = oldComponentZone); @@ -193,7 +194,9 @@ void commonFactoryTests(ReactComponentFactoryProxy factory, } } -void domEventHandlerWrappingTests(ReactComponentFactoryProxy factory) { +void domEventHandlerWrappingTests(ReactComponentFactoryProxy factory, { + bool isNonDartComponentWithDartWrapper = false, +}) { Element renderAndGetRootNode(ReactElement content) { final mountNode = Element.div(); react_dom.render(content, mountNode); @@ -270,7 +273,7 @@ void domEventHandlerWrappingTests(ReactComponentFactoryProxy factory) { } }); - if (isDartComponent(factory({}))) { + if (isDartComponent(factory({})) && !isNonDartComponentWithDartWrapper) { group('in a way that the handlers are callable from within the Dart component:', () { setUpAll(() { expect(propsFromDartRender, isNotNull, diff --git a/test/react_lazy_test.dart b/test/react_lazy_test.dart index bffeffdb..2d87e3dd 100644 --- a/test/react_lazy_test.dart +++ b/test/react_lazy_test.dart @@ -1,7 +1,13 @@ @TestOn('browser') +@JS() library react.react_lazy_test; +import 'dart:js_util'; + +import 'package:js/js.dart'; +import 'package:react/hooks.dart'; import 'package:react/react.dart' as react; +import 'package:react/react_client/component_factory.dart'; import 'package:react/react_client/react_interop.dart'; import 'package:test/test.dart'; @@ -9,18 +15,65 @@ import 'factory/common_factory_tests.dart'; main() { group('lazy', () { - group('- common factory behavior -', () { - final LazyTest = react.lazy(() async => react.registerFunctionComponent((props) { + group('Dart component', () { + final LazyTest = react.lazy(() async => react.forwardRef2((props, ref) { + useImperativeHandle(ref, () => TestImperativeHandle()); props['onDartRender']?.call(props); return react.div({...props}); })); - commonFactoryTests( - LazyTest, - // ignore: invalid_use_of_protected_member - dartComponentVersion: ReactDartComponentVersion.component2, - renderWrapper: (child) => react.Suspense({'fallback': 'Loading...'}, child), - ); + group('- common factory behavior -', () { + commonFactoryTests( + LazyTest, + // ignore: invalid_use_of_protected_member + dartComponentVersion: ReactDartComponentVersion.component2, + renderWrapper: (child) => react.Suspense({'fallback': 'Loading...'}, child), + ); + }); + + group('- dom event handler wrapping -', () { + domEventHandlerWrappingTests(LazyTest); + }); + + group('- refs -', () { + refTests(LazyTest, verifyRefValue: (ref) { + expect(ref, isA()); + }); + }); + }); + + group('JS component', () { + final LazyJsTest = react.lazy(() async => ReactJsComponentFactoryProxy(_JsFoo)); + + group('- common factory behavior -', () { + commonFactoryTests( + LazyJsTest, + // ignore: invalid_use_of_protected_member + dartComponentVersion: ReactDartComponentVersion.component2, + // This isn't a Dart component, but it's detected as one by tests due to the factory's dartComponentVersion + isNonDartComponentWithDartWrapper: true, + renderWrapper: (child) => react.Suspense({'fallback': 'Loading...'}, child), + ); + }); + + group('- dom event handler wrapping -', () { + domEventHandlerWrappingTests( + LazyJsTest, + // This isn't a Dart component, but it's detected as one by tests due to the factory's dartComponentVersion + isNonDartComponentWithDartWrapper: true, + ); + }); + + group('- refs -', () { + refTests(LazyJsTest, verifyRefValue: (ref) { + expect(getProperty(ref as Object, 'constructor'), same(_JsFoo)); + }); + }); }); }); } + +class TestImperativeHandle {} + +@JS() +external ReactClass get _JsFoo; diff --git a/test/react_lazy_test.html b/test/react_lazy_test.html index 73faf0d6..2f98b084 100644 --- a/test/react_lazy_test.html +++ b/test/react_lazy_test.html @@ -7,6 +7,7 @@ + From 94aa9f3a372cc5594bfb3759afaf2fa67b63c07d Mon Sep 17 00:00:00 2001 From: Greg Littlefield Date: Fri, 27 Sep 2024 16:10:37 -0700 Subject: [PATCH 3/3] Format --- test/factory/common_factory_tests.dart | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/factory/common_factory_tests.dart b/test/factory/common_factory_tests.dart index c2fc2edf..d0d800ba 100644 --- a/test/factory/common_factory_tests.dart +++ b/test/factory/common_factory_tests.dart @@ -194,7 +194,8 @@ void commonFactoryTests(ReactComponentFactoryProxy factory, } } -void domEventHandlerWrappingTests(ReactComponentFactoryProxy factory, { +void domEventHandlerWrappingTests( + ReactComponentFactoryProxy factory, { bool isNonDartComponentWithDartWrapper = false, }) { Element renderAndGetRootNode(ReactElement content) { @@ -290,10 +291,12 @@ void domEventHandlerWrappingTests(ReactComponentFactoryProxy factory, { }); final divRef = react.createRef(); - react_dom.render(react.div({ - 'ref': divRef, - 'onClick': (react.SyntheticMouseEvent e) => dummyEvent = e, - }), mountNode); + react_dom.render( + react.div({ + 'ref': divRef, + 'onClick': (react.SyntheticMouseEvent e) => dummyEvent = e, + }), + mountNode); divRef.current!.click(); });