Add forwardRef2/memo2 which fixes jsification of Dart props

[greglittlefield-wf]

Motivation

Dart forwardRef components and memo-wrapped components were being passed JSified props due to the use of JS component factories, when they expected to be getting non-converted Dart props.

Solution

tl;dr: I added forwardRef2/memo2, added a bunch of tests around refs, and fixed a bunch of issues surfaced by test failures.

• Add forwardRef2 and memo2, which return Dart component factories so they don't convert props

  • I had to make new versions because fixing the issues required return type changes that would be breaking. We can consume these new versions under the hood in over_react's memo/uiForwardRef without any breakages.

  • Remove prop de-converting logic from forwardRef2 This was only a partial solution to wrapping JS components, only applying to callbacks

    Also, now that we're using Dart component factories, this logic would prevent Dart event handlers from being converted.

    So, since we'll need a more holistic solution to the problem of wrapping JS components, and it wouldn't be easy to retain this logic without breaking other behavior, it was removed.

• Update common factory tests:

  • Refactor simple ref tests use ref test case objects for consistency and to DRY up setup
  • Add tests for all Dart factories (including forwardRef2/memo2) that Dart props are passed in without being converted These verify that the main issue that motivated this PR (converted JS props being passed to Dart components) is fixed
  • Add tests that verify that all ref types work with forwardRef
  • Add tests that chainRef works with refs that come from forwardRef2 callback args (as well as ReactElement, since it experienced a similar problem)
    • This required some code changes in different places to get these passing
  • Add test cases JS ref objects are populated with the correct values

• Update event handler tests to not test de-conversion of props via forwardRef

QA Instructions

• Verify unit tests pass (CI passes)
• Verify the function components in WSD that originally surfaced these issues still work (@aaronlademann-wf)
• Smoke test examples
• Perform QA instructions in over_react PR consuming these in uiForwardRef and memo: https://github.com/Workiva/over_react/pull/620

[semveraudit-wf]

Public API Changes

Recommendation: :bangbang: Major version bump (fyi @Workiva/semver-audit-group )

@@ line 37: package:react/react_client/react_interop.dart @@
abstract class React
-     static ReactClass memo(dynamic Function(JsMap, [JsMap]) wrapperFunction, [bool Function(JsMap, JsMap) areEqual])
+     static ReactClass memo(dynamic wrapperFunction, [bool Function(JsMap, JsMap) areEqual])
//    `type` of `wrapperFunction` has changed.
//    Changing a parameter signature is a major change.

@@ line 38: package:react/react.dart @@
+  typedef DartForwardRefFunctionComponent = dynamic Function(JsBackedMap props, dynamic ref)
// Adding a typedef is a minor change.

@@ line 232: package:react/react_client/react_interop.dart @@
+  ReactComponentFactoryProxy forwardRef2(dynamic Function(Map<dynamic, dynamic>, dynamic) wrapperFunction, {String displayName})
// Adding a top-level function is a minor change.

@@ line 232: package:react/react_client/react_interop.dart @@
+  ReactComponentFactoryProxy forwardRef2(dynamic Function(Map<dynamic, dynamic>, dynamic) wrapperFunction, {String displayName})
// Adding a top-level function is a minor change.

@@ line 303: package:react/react_client/react_interop.dart @@
+  ReactComponentFactoryProxy memo2(ReactComponentFactoryProxy factory, {bool Function(Map<dynamic, dynamic>, Map<dynamic, dynamic>) areEqual})
// Adding a top-level function is a minor change.

Click to see 2 more API Changes

--- ```diff @@ line 303: package:react/react_client/react_interop.dart @@ + ReactComponentFactoryProxy memo2(ReactComponentFactoryProxy factory, {bool Function(Map, Map) areEqual}) // Adding a top-level function is a minor change. ``` ```diff @@ line 364: package:react/react_client/component_factory.dart @@ + class ReactDartWrappedComponentFactoryProxy extends ReactComponentFactoryProxy with JsBackedMapComponentFactoryMixin // Adding a class is a minor change. + ReactClass get type // Adding a field is a minor change. + ReactElement build(Map props, [List childrenArgs = const []]) // Adding a method is a minor change. + ReactDartWrappedComponentFactoryProxy ReactDartWrappedComponentFactoryProxy.forwardRef(dynamic Function(JsBackedMap, dynamic) dartFunctionComponent, {String displayName}) // Adding a constructor is a minor change. + ReactDartWrappedComponentFactoryProxy ReactDartWrappedComponentFactoryProxy(ReactClass type) // Adding a constructor is a minor change. From package:react/react.dart + dynamic call(Map props, [dynamic c1 = _notSpecified, dynamic c2 = _notSpecified, dynamic c3 = _notSpecified, dynamic c4 = _notSpecified, dynamic c5 = _notSpecified, dynamic c6 = _notSpecified, dynamic c7 = _notSpecified, dynamic c8 = _notSpecified, dynamic c9 = _notSpecified, dynamic c10 = _notSpecified, dynamic c11 = _notSpecified, dynamic c12 = _notSpecified, dynamic c13 = _notSpecified, dynamic c14 = _notSpecified, dynamic c15 = _notSpecified, dynamic c16 = _notSpecified, dynamic c17 = _notSpecified, dynamic c18 = _notSpecified, dynamic c19 = _notSpecified, dynamic c20 = _notSpecified, dynamic c21 = _notSpecified, dynamic c22 = _notSpecified, dynamic c23 = _notSpecified, dynamic c24 = _notSpecified, dynamic c25 = _notSpecified, dynamic c26 = _notSpecified, dynamic c27 = _notSpecified, dynamic c28 = _notSpecified, dynamic c29 = _notSpecified, dynamic c30 = _notSpecified, dynamic c31 = _notSpecified, dynamic c32 = _notSpecified, dynamic c33 = _notSpecified, dynamic c34 = _notSpecified, dynamic c35 = _notSpecified, dynamic c36 = _notSpecified, dynamic c37 = _notSpecified, dynamic c38 = _notSpecified, dynamic c39 = _notSpecified, dynamic c40 = _notSpecified]) // Adding a method is a minor change. ``` ---

Showing results for e200d485c3e763085c924ce2763f507d27be1044

Powered by semver-audit-service. Please report any problems by filing an issue. Reported by the dart semver audit client 2.2.0 Browse public API.

Last edited UTC Aug 20 at 22:06:52

[greglittlefield-wf]

@aaronlademann-wf @smaifullerton-wk All feedback has been addressed! (including adding a test case for wrapping forwardRef2 with memo)

[sydneyjodon-wk]

QAing now... I think this just needs to be formatted?

[sydneyjodon-wk]

• [x] Verify unit tests pass (CI passes)
• [x] Smoke test examples
• [x] Perform QA instructions in over_react PR

QA+1 for every step except:

• [x] Verify the function components in WSD that originally surfaced these issues still work (@aaronlademann-wf )

[aaronlademann-wf]

QA +1

• [x] Verify the function components in WSD that originally surfaced these issues still work