Merge branch 'v7_wip' into v7-removals

CHANGELOG.md

@@ -41,8 +41,39 @@
 - ReactComponentFactoryProxy no longer `implements Function`
     - This should not be a breakage, since as of Dart 2.0 inheriting from Function has had no effect
 
-#### Potential behavior breakages
-- Component and Component2 members `props`/`state`/`jsThis` are late, will now throw instead of being null if accessed before initialized (e.g., in a constructor, final class field, or static lifecycle method).
+#### Behavior breakages unlikely to cause issues
+- Component and Component2 members `props`/`state`/`jsThis` are now [late](https://dart.dev/language/variables#late-variables), and will now throw instead of being null if accessed before initialized.
+
+    It should be very uncommon for components to be affected by this change, and any affected components are likely doing something wrong to begin with. 
+
+    These fields are only uninitialized:
+     - for mounting component instances: 
+        - in component class constructors (which we don't encourage)
+        - in component class field initializers (except for lazy `late` ones)
+     - in "static" lifecycle methods like `getDerivedStateFromProps` and `defaultProps`
+
+    Examples of code affected:
+    ```dart
+    class FooComponent extends Component2 {
+      // `props` would have always been null when this is initialized, but in 7.0.0 accessing it throws.
+      final something = (props ?? {})['something'];
+
+      // We strongly discourage declaring Dart constructors in component classes;
+      // for initialization logic, use componentDidMount instead. 
+      FooComponent() {
+        // `props` would have always been null here, but in 7.0.0 accessing it throws.
+        print(props);
+      }
+
+      @override
+      getDerivedStateFromProps(nextProps, prevState)  {
+        // `props` would have always been null here, but in 7.0.0 accessing it throws.
+        print(props);
+        return {};
+      }
+    }
+    ```
+
 
 ## [6.2.1](https://github.com/Workiva/react-dart/compare/6.2.0...6.2.1)
 - [#366] Fix lints and eliminate most implicit casts 

lib/hooks.dart

@@ -6,6 +6,15 @@ import 'package:meta/meta.dart';
 import 'package:react/react.dart';
 import 'package:react/react_client/react_interop.dart';
 
+/// A setter function returned as the second item in the result of a JS useState call,
+/// which accepts either a value or an updater function.
+///
+/// If Dart had type unions, the typing would be: `void Function(T|(T Function(T)) valueOrUpdater)`
+///
+/// We could make this generic, but the generic value wouldn't be used at all since the argument
+/// can only be expressed as `dynamic`.
+typedef _JsStateHookSetter/*<T>*/ = void Function(dynamic /*T|(T Function(T))*/ valueOrUpdater);
+
 /// The return value of [useState].
 ///
 /// The current value of the state is available via [value] and
@@ -23,7 +32,7 @@ class StateHook<T> {
   final T _value;
 
   /// The second item in the pair returned by [React.useState].
-  final void Function(dynamic) _setValue;
+  final _JsStateHookSetter _setValue;
 
   StateHook._(this._value, this._setValue);
 
@@ -67,7 +76,7 @@ class StateHook<T> {
 /// Learn more: <https://reactjs.org/docs/hooks-state.html>.
 StateHook<T> useState<T>(T initialValue) {
   final result = React.useState(initialValue);
-  return StateHook._(result[0] as T, result[1] as void Function(dynamic));
+  return StateHook._(result[0] as T, result[1] as _JsStateHookSetter);
 }
 
 /// Adds local state to a [DartFunctionComponent]
@@ -93,7 +102,7 @@ StateHook<T> useState<T>(T initialValue) {
 /// Learn more: <https://reactjs.org/docs/hooks-reference.html#lazy-initial-state>.
 StateHook<T> useStateLazy<T>(T Function() init) {
   final result = React.useState(allowInterop(init));
-  return StateHook._(result[0] as T, result[1] as void Function(dynamic));
+  return StateHook._(result[0] as T, result[1] as _JsStateHookSetter);
 }
 
 /// Runs [sideEffect] after every completed render of a [DartFunctionComponent].

lib/react.dart

@@ -101,6 +101,22 @@ var Suspense = ReactJsComponentFactoryProxy(React.Suspense);
 /// See: <https://reactjs.org/docs/strict-mode.html>
 var StrictMode = ReactJsComponentFactoryProxy(React.StrictMode);
 
+// -------------------------------------------------------------------------------------------------------------------
+// [1] While these fields are always initialized upon mount immediately after the class is instantiated,
+//     since they're not passed into a Dart constructor, they can't be initialized during instantiation,
+//     forcing us to make them either `late` or nullable.
+//
+//     Since we want them to be non-nullable, we'll opt for `late`.
+//
+//     These fields only haven't been initialized:
+//      - for mounting component instances:
+//         - in component class constructors (which we don't encourage)
+//         - in component class field initializers (except for lazy `late` ones)
+//      - in "static" lifecycle methods like `getDerivedStateFromProps` and `defaultProps`
+//
+//     So, this shouldn't pose a problem for consumers.
+// -------------------------------------------------------------------------------------------------------------------
+
 /// Top-level ReactJS [Component class](https://reactjs.org/docs/react-component.html)
 /// which provides the [ReactJS Component API](https://reactjs.org/docs/react-component.html#reference)
 ///
@@ -120,7 +136,7 @@ abstract class Component {
   /// [doesn't work for overriding fields](https://github.com/dart-lang/sdk/issues/27452).
   ///
   /// TODO: Switch back to a plain field once this issue is fixed.
-  late Map _props;
+  late Map _props; // [1]
 
   /// A private field that backs [state], which is exposed via getter/setter so
   /// it can be overridden in strong mode.
@@ -138,7 +154,7 @@ abstract class Component {
   /// [doesn't work for overriding fields](https://github.com/dart-lang/sdk/issues/27452).
   ///
   /// TODO: Switch back to a plain field once this issue is fixed.
-  late RefMethod _ref;
+  late RefMethod _ref; // [1]
 
   /// The React context map of this component, passed down from its ancestors' [getChildContext] value.
   ///
@@ -190,9 +206,9 @@ abstract class Component {
       'Only supported in the deprecated Component, and not Component2. Use createRef or a ref callback instead.')
   set ref(RefMethod value) => _ref = value;
 
-  late Function _jsRedraw;
+  late void Function() _jsRedraw; // [1]
 
-  late dynamic _jsThis;
+  late dynamic _jsThis; // [1]
 
   final List<SetStateCallback> _setStateCallbacks = [];
 
@@ -210,11 +226,12 @@ abstract class Component {
 
   /// Allows the [ReactJS `displayName` property](https://reactjs.org/docs/react-component.html#displayname)
   /// to be set for debugging purposes.
+  // This return type is nullable since Component2's override will return null in certain cases.
   String? get displayName => runtimeType.toString();
 
   static dynamic _defaultRef(String _) => null;
 
-  initComponentInternal(Map props, Function _jsRedraw, [RefMethod? ref, dynamic _jsThis, Map? context]) {
+  initComponentInternal(Map props, void Function() _jsRedraw, [RefMethod? ref, dynamic _jsThis, Map? context]) {
     this._jsRedraw = _jsRedraw;
     this.ref = ref ?? _defaultRef;
     this._jsThis = _jsThis;
@@ -415,7 +432,7 @@ abstract class Component {
   @Deprecated('This legacy, unstable context API is only supported in the deprecated Component, and not Component2.'
       ' Instead, use Component2.context, Context.Consumer, or useContext.')
   // ignore: avoid_returning_null
-  bool? shouldComponentUpdateWithContext(Map nextProps, Map nextState, Map nextContext) => null;
+  bool? shouldComponentUpdateWithContext(Map nextProps, Map nextState, Map? nextContext) => null;
 
   /// ReactJS lifecycle method that is invoked immediately before rendering when [nextProps] or [nextState] are being
   /// received.
@@ -650,10 +667,10 @@ abstract class Component2 implements Component {
   dynamic context;
 
   @override
-  late Map props;
+  late Map props; // [1]
 
   @override
-  late Map state;
+  late Map state; // [1]
 
   @override
   @Deprecated('Only supported in the deprecated Component, and not in Component2. See doc comment for more info.')
@@ -665,7 +682,7 @@ abstract class Component2 implements Component {
   /// The JavaScript [`ReactComponent`](https://reactjs.org/docs/react-api.html#reactdom.render)
   /// instance of this `Component` returned by [render].
   @override
-  late ReactComponent jsThis;
+  late ReactComponent jsThis; // [1]
 
   /// Allows the [ReactJS `displayName` property](https://reactjs.org/docs/react-component.html#displayname)
   /// to be set for debugging purposes.
@@ -1276,7 +1293,7 @@ abstract class Component2 implements Component {
 
   @override
   @Deprecated('Only supported in the deprecated Component, and not in Component2.')
-  late var _jsRedraw;
+  late void Function() _jsRedraw;
 
   @override
   @Deprecated('Only supported in the deprecated Component, and not in Component2.')

lib/react_client/js_backed_map.dart

@@ -38,7 +38,7 @@ class JsBackedMap extends MapBase<dynamic, dynamic> {
   factory JsBackedMap.fromJs(JsMap jsOther) => JsBackedMap()..addAllFromJs(jsOther);
 
   // Private helpers with narrower typing than we want to expose, for use in other methods
-  List<String> get _keys => objectKeys(jsObject);
+  List<dynamic> get _keys => objectKeys(jsObject);
   // Use keys to access the value instead oof `Object.values` since MSIE 11 doesn't support it
   List<dynamic> get _values => _keys.map((key) => this[key]).toList();
 

lib/src/js_interop_util.dart

@@ -6,7 +6,8 @@ import 'dart:js_util';
 import 'package:js/js.dart';
 
 @JS('Object.keys')
-external List<String> objectKeys(Object object);
+// We can't type this as List<String> due to https://github.com/dart-lang/sdk/issues/37676
+external List<dynamic /*String*/ > objectKeys(Object object);
 
 @JS()
 @anonymous

lib/src/react_client/dart_interop_statics.dart

@@ -128,7 +128,7 @@ final ReactDartInteropStatics dartInteropStatics = (() {
         // If shouldComponentUpdateWithContext returns a valid bool (default implementation returns null),
         // then don't bother calling `shouldComponentUpdate` and have it trump.
         var shouldUpdate = component.shouldComponentUpdateWithContext(
-            component.nextProps!, component.nextState, component.nextContext!);
+            component.nextProps!, component.nextState, component.nextContext);
 
         shouldUpdate ??= component.shouldComponentUpdate(component.nextProps!, component.nextState);
 

lib/src/react_client/private_utils.dart

@@ -38,7 +38,7 @@ T validateJsApiThenReturn<T>(T Function() computeReturn) {
 Map<String, dynamic> unjsifyContext(InteropContextValue interopContext) {
   // TODO consider using `contextKeys` for this if perf of objectKeys is bad.
   return {
-    for (final key in objectKeys(interopContext))
+    for (final key in objectKeys(interopContext).cast<String>())
       key: (getProperty(interopContext, key) as ReactDartContextInternal?)?.value
   };
 }

test/lifecycle_test/component.dart

@@ -180,15 +180,15 @@ class _LifecycleTest extends react.Component with LifecycleTestHelper {
 
   @override
   componentWillReceivePropsWithContext(newProps, newContext) => lifecycleCall('componentWillReceivePropsWithContext',
-      arguments: [Map.from(newProps), Map.from(newContext as Map)]);
+      arguments: [Map.from(newProps), copyMapIfNotNull(newContext as Map?)]);
 
   @override
   componentWillUpdate(nextProps, nextState) =>
       lifecycleCall('componentWillUpdate', arguments: [Map.from(nextProps), Map.from(nextState)]);
 
   @override
   componentWillUpdateWithContext(nextProps, nextState, nextContext) => lifecycleCall('componentWillUpdateWithContext',
-      arguments: [Map.from(nextProps), Map.from(nextState), Map.from(nextContext as Map)]);
+      arguments: [Map.from(nextProps), Map.from(nextState), copyMapIfNotNull(nextContext)]);
 
   @override
   componentDidUpdate(prevProps, prevState) =>
@@ -201,7 +201,8 @@ class _LifecycleTest extends react.Component with LifecycleTestHelper {
   @override
   shouldComponentUpdateWithContext(nextProps, nextState, nextContext) =>
       lifecycleCall('shouldComponentUpdateWithContext',
-          arguments: [Map.from(nextProps), Map.from(nextState), Map.from(nextContext)], defaultReturnValue: () => true);
+          arguments: [Map.from(nextProps), Map.from(nextState), copyMapIfNotNull(nextContext)],
+          defaultReturnValue: () => true);
 
   @override
   render() => lifecycleCall('render', defaultReturnValue: () => react.div({}));
@@ -216,3 +217,5 @@ class _LifecycleTest extends react.Component with LifecycleTestHelper {
     return {'defaultProp': 'default'};
   }
 }
+
+Map<K, V>? copyMapIfNotNull<K, V>(Map<K, V>? map) => map == null ? null : {...map};

test/nullable_callback_detection/nullable_callback_detection_sound_test.dart

@@ -8,7 +8,7 @@ import 'null_safe_refs.dart' as null_safe_refs;
 import 'sound_null_safety_detection.dart';
 
 main() {
-  // This test ths the counterpart to nullable_callback_detection_unsound_test.dart,
+  // This test is the counterpart to nullable_callback_detection_unsound_test.dart,
   // to verify expected behavior under sound null safety.
   // See that file for more info.
   group('nullable callback detection, when compiled with sound null safety:', () {