Merge pull request #974 from Workiva/fix-fragment-type-check-assert

FED-3585 Fix fragment type check assert

Author: btr-rmconsole-4[bot]

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].

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({