Merge branch 'master' of github.com:Workiva/over_react into fedx_3476

Author: bender-wk

CHANGELOG.md

@@ -1,5 +1,12 @@
 # OverReact Changelog
 
+## 5.4.3
+- [#966] Revert #964 (an attempt to fix `manageAndReturnTypedDisposable` typing)
+- [#969] Use a `dart_dependency_validator.yaml` config file
+- [#973] React 18 Prep
+  - Bumps the minimum Dart SDK from `2.17.0` to `2.19.0`
+- [#974] Fix fragment type check assert
+
 ## 5.4.2
 - [#964] Fix bad override of `manageAndReturnTypedDisposable` caused by w_common 3.3.0
 

lib/src/component_declaration/component_type_checking.dart

@@ -172,9 +172,6 @@ bool isTypeAlias(dynamic type) {
 /// Returns the [ComponentTypeMeta] associated with the component type [type] in [setComponentTypeMeta],
 /// or `const ComponentTypeMeta.none()` if there is no associated meta.
 ComponentTypeMeta getComponentTypeMeta(Object type) {
-  assert(isPotentiallyValidComponentType(type),
-      '`type` should be a valid component type (and not null or a type alias).');
-
   if (type is! String) {
     return getProperty(type, _componentTypeMetaKey) as ComponentTypeMeta? ??
         const ComponentTypeMeta.none();
@@ -307,7 +304,16 @@ Object? getComponentTypeFromAlias(Object? typeAlias) {
 /// * [Function] factory (Dart components)
 /// * [ReactClass] component type (JS composite component classes, JS function component functions, Dart component JS classes)
 bool isPotentiallyValidComponentType(dynamic type) {
-  return type is Function || type is ReactClass || type is String;
+  return type is Function ||
+      // In DDC, `type is ReactClass` is true for JS symbols (such as for Fragment ReactElements),
+      // but in dart2js it is false.
+      // Since isPotentiallyValidComponentType is only used in aliasing code now, this isn't a big deal since
+      // people aren't likely aliasing Fragment, but we should make this behavior consistent.
+      // TODO handle that case in dart2js once our Dart SDK constraint is >=3.0.0, with the following code:
+      //    typeofEquals(type, 'symbol') ||
+      // TODO also add a test (see TODO in component_type_checking_test.dart)
+      type is ReactClass ||
+      type is String;
 }
 
 /// Returns an [Iterable] of all component types that are ancestors of [type].

pubspec.yaml

@@ -1,5 +1,5 @@
 name: over_react
-version: 5.4.2
+version: 5.4.3
 description: A library for building statically-typed React UI components using Dart.
 homepage: https://github.com/Workiva/over_react/
 environment:

test/over_react/component/context_test.dart

@@ -206,7 +206,7 @@ void main() {
         });
       });
 
-      group('experimental calculateChangeBits argument functions correctly', () {
+      group('experimental calculateChangeBits argument does not throw when used (has no effect in React 18)', () {
         late Ref<ContextProviderWrapperComponent?> providerRef;
         int? consumerEvenValue;
         int? consumerOddValue;
@@ -243,15 +243,14 @@ void main() {
         });
 
         test('on value updates', () {
+          // Test common behavior between React 17 (calculateChangedBits working)
+          // and React 18 (it having no effect).
           providerRef.current!.increment();
           expect(consumerEvenValue, 2);
-          expect(consumerOddValue, 1);
           providerRef.current!.increment();
-          expect(consumerEvenValue, 2);
           expect(consumerOddValue, 3);
           providerRef.current!.increment();
           expect(consumerEvenValue, 4);
-          expect(consumerOddValue, 3);
         });
       });
     });

test/over_react/component/error_boundary/shared_stack_tests.dart

@@ -12,9 +12,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import 'dart:html';
+
 import 'package:meta/meta.dart';
 import 'package:over_react/over_react.dart' hide ErrorBoundary;
-import 'package:react_testing_library/react_testing_library.dart' as rtl;
+import 'package:over_react/react_dom.dart' as react_dom;
 import 'package:over_react/components.dart';
 import 'package:test/test.dart';
 
@@ -27,17 +29,15 @@ void sharedErrorBoundaryStackTests() {
     void expectRenderErrorWithComponentName(ReactElement element,
         {required String expectedComponentName}) {
       final capturedInfos = <ReactErrorInfo>[];
-      expect(() {
-        rtl.render((ErrorBoundary()
-          ..shouldLogErrors = false
-          ..onComponentDidCatch = (error, info) {
-            capturedInfos.add(info);
-          })(element));
-        // Use prints as an easy way to swallow `print` calls and
-        // prevent RTL from forwarding console errors to the test output,
-        // since React error boundary logging is pretty noisy.
-        // TODO instead, disable logging in this rtl.render call once that option is available: FED-1641
-      }, prints(anything));
+      final mountNode = DivElement();
+      // Use react_dom.render instead of RTL to avoid errors on React 18 about
+      // `act` being used in prod builds.
+      react_dom.render((ErrorBoundary()
+        ..shouldLogErrors = false
+        ..onComponentDidCatch = (error, info) {
+          capturedInfos.add(info);
+        })(element), mountNode);
+      addTearDown(() => react_dom.unmountComponentAtNode(mountNode));
 
       expect(capturedInfos, hasLength(1),
           reason: 'test setup check; should have captured a single component error');

test/over_react/component_declaration/component_type_checking_test.dart

@@ -318,6 +318,53 @@ main() {
       }, tags: 'ddc');
     });
   });
+
+  // Note: due to current behavioral differences of isPotentiallyValidComponentType between dart2js and DDC,
+  // these regression tests only fail in dart2js with --enable-asserts, which we don't want to run our tests with.
+  group('type checking and related utilities work as expected, without throwing, for', () {
+    group('types of built-in and DOM components', () {
+      final componentsByDescription = <String, ReactElement Function()>{
+        // Edge-case: the `type` of Fragment is a JS Symbol
+        'Fragment': () => Fragment()(''),
+        'StrictMode': () => StrictMode()(),
+        'div': () => Dom.div()(),
+      };
+
+      componentsByDescription.forEach((description, factory) {
+        group('- $description -', () {
+          late ReactElement element;
+          late Object type;
+
+          setUp(() {
+            element = factory();
+            final maybeType = element.type as Object?;
+            expect(maybeType, isNotNull, reason: 'test setup check; element should have a type');
+            type = maybeType!;
+          });
+
+          // TODO uncomment once isPotentiallyValidComponentType is fixed for JS Symbols (see TODO in isPotentiallyValidComponentType), and update "Note:" comment above
+          // test('isPotentiallyValidComponentType', () {
+          //   expect(isPotentiallyValidComponentType(type), isTrue);
+          // });
+
+          test('getComponentTypeMeta', () {
+            expect(getComponentTypeMeta(type), isA<ComponentTypeMeta>()
+                  .having((m) => m.isWrapper, 'isWrapper', false)
+                  .having((m) => m.isHoc, 'isHoc', false)
+                  .having((m) => m.parentType, 'parentType', isNull));
+          });
+
+          test('getProps()', () {
+            expect(getProps(element), isA<Map>());
+          });
+
+          test('getProps(traverseWrappers: true)', () {
+            expect(getProps(element, traverseWrappers: true), isA<Map>());
+          });
+        });
+      });
+    });
+  });
 }
 
 testComponentTypeChecking({

tools/analyzer_plugin/dart_dependency_validator.yaml

@@ -0,0 +1,5 @@
+ignore:
+  # Ignore the over_react pin
+  - over_react
+exclude:
+  - playground/**

tools/analyzer_plugin/lib/src/diagnostic/exhaustive_deps.dart

@@ -1605,12 +1605,6 @@ Iterable<Union<Identifier, NamedType>> resolvedReferencesWithin(AstNode node) sy
   }
 }
 
-extension on NamedType {
-  // NamedType.element is added in analyzer 5.11.0; we can't resolve to that version in Dart 2.18,
-  // so we'll add it here so we don't have to go back through and change it later.
-  Element? get element => name.staticElement;
-}
-
 enum HookTypeWithStableMethods { stateHook, reducerHook, transitionHook }
 
 abstract class HookConstants {

tools/analyzer_plugin/lib/src/diagnostic/missing_required_prop.dart

@@ -1,13 +1,11 @@
 import 'package:analyzer/dart/analysis/results.dart';
-import 'package:analyzer/dart/ast/ast.dart';
 import 'package:analyzer/dart/element/element.dart';
 import 'package:over_react_analyzer_plugin/src/diagnostic/analyzer_debug_helper.dart';
 import 'package:over_react_analyzer_plugin/src/diagnostic_contributor.dart';
 import 'package:over_react_analyzer_plugin/src/util/ast_util.dart';
 import 'package:over_react_analyzer_plugin/src/util/pretty_print.dart';
 import 'package:over_react_analyzer_plugin/src/util/prop_declarations/props_set_by_factory.dart';
 import 'package:over_react_analyzer_plugin/src/util/prop_forwarding/forwarded_props.dart';
-import 'package:over_react_analyzer_plugin/src/util/util.dart';
 import 'package:over_react_analyzer_plugin/src/util/weak_map.dart';
 
 import '../fluent_interface_util.dart';

tools/analyzer_plugin/pubspec.yaml

@@ -4,16 +4,16 @@ publish_to: none
 description: Dart analyzer plugin for OverReact
 repository: https://github.com/Workiva/over_react/tree/master/tools/analyzer_plugin
 environment:
-  sdk: '>=2.17.0 <3.0.0'
+  sdk: '>=2.19.0 <3.0.0'
 dependencies:
-  analyzer: ^5.1.0
+  analyzer: ^5.11.0
   analyzer_plugin: ^0.11.0
   collection: ^1.15.0-nullsafety.4
   meta: ^1.16.0
   # Upon release, this should be pinned to the over_react version from ../../pubspec.yaml
   # so that it always resolves to the same version of over_react that the user has pulled in,
   # and thus has the same boilerplate parsing code that's running in the builder.
-  over_react: 5.4.2
+  over_react: 5.4.3
   path: ^1.5.1
   source_span: ^1.7.0
   yaml: ^3.0.0
@@ -34,18 +34,9 @@ dev_dependencies:
   test_reflective_loader: ^0.2.0
   uuid: '>3.0.0 <5.0.0'
   workiva_analysis_options: ^1.1.0
-
-dependency_validator:
-  ignore:
-    # Ignore the over_react pin
-    - over_react
-  exclude:
-    - playground/**
-
 workiva:
   core_checks:
     react_boilerplate: disabled
-
 # If you're developing on changes in over_react that the analyzer plugin needs to consume,
 # point to the local copy of over_react. This must be an absolute path.
 #