AF-392: Add workaround to NSM for Dart 2

lib/src/component/dom_components.dart

@@ -54,7 +54,7 @@ class DomProps extends component_base.UiProps
   String get propKeyNamespace => '';
 
   @override
-  ReactElement call([children, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, c31, c32, c33, c34, c35, c36, c37, c38, c39, c40]);
+  ReactElement call([c1, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, c31, c32, c33, c34, c35, c36, c37, c38, c39, c40]);
 }
 
 // Include pieces from transformer_helpers so that consumers can type these instances
@@ -75,7 +75,7 @@ class SvgProps extends component_base.UiProps
   String get propKeyNamespace => '';
 
   @override
-  ReactElement call([children, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, c31, c32, c33, c34, c35, c36, c37, c38, c39, c40]);
+  ReactElement call([c1, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, c31, c32, c33, c34, c35, c36, c37, c38, c39, c40]);
 }
 
 /// A class that provides namespacing for static DOM component factory methods, much like `React.DOM` in React JS.

lib/src/component_declaration/component_base.dart

@@ -636,30 +636,49 @@ abstract class UiProps extends MapBase
   ///
   /// Restricted statically to 40 arguments until the dart2js fix in
   /// <https://github.com/dart-lang/sdk/pull/26032> is released.
-  ReactElement call([children, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, c31, c32, c33, c34, c35, c36, c37, c38, c39, c40]);
+  ///
+  ReactElement call([c1 = _notSpecified, c2 = _notSpecified, c3 = _notSpecified, c4 = _notSpecified, c5 = _notSpecified, c6 = _notSpecified, c7 = _notSpecified, c8 = _notSpecified, c9 = _notSpecified, c10 = _notSpecified, c11 = _notSpecified, c12 = _notSpecified, c13 = _notSpecified, c14 = _notSpecified, c15 = _notSpecified, c16 = _notSpecified, c17 = _notSpecified, c18 = _notSpecified, c19 = _notSpecified, c20 = _notSpecified, c21 = _notSpecified, c22 = _notSpecified, c23 = _notSpecified, c24 = _notSpecified, c25 = _notSpecified, c26 = _notSpecified, c27 = _notSpecified, c28 = _notSpecified, c29 = _notSpecified, c30 = _notSpecified, c31 = _notSpecified, c32 = _notSpecified, c33 = _notSpecified, c34 = _notSpecified, c35 = _notSpecified, c36 = _notSpecified, c37 = _notSpecified, c38 = _notSpecified, c39 = _notSpecified, c40 = _notSpecified]) {
+    List childArguments;
+    // Use `identical` since it compiles down to `===` in dart2js instead of calling equality helper functions,
+    // and we don't want to allow any object overriding `operator==` to claim it's equal to `_notSpecified`.
+    if (identical(c1, _notSpecified)) {
+      childArguments = [];
+    } else if (identical(c2, _notSpecified)) {
+      childArguments = [c1];
+    } else if (identical(c3, _notSpecified)) {
+      childArguments = [c1, c2];
+    } else if (identical(c4, _notSpecified)) {
+      childArguments = [c1, c2, c3];
+    } else if (identical(c5, _notSpecified)) {
+      childArguments = [c1, c2, c3, c4];
+    } else if (identical(c6, _notSpecified)) {
+      childArguments = [c1, c2, c3, c4, c5];
+    } else if (identical(c7, _notSpecified)) {
+      childArguments = [c1, c2, c3, c4, c5, c6];
+    } else {
+      childArguments = [c1, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, c31, c32, c33, c34, c35, c36, c37, c38, c39, c40]
+        .takeWhile((child) => !identical(child, _notSpecified))
+        .toList();
+    }
 
-  /// Supports variadic children of the form `call([child1, child2, child3...])`.
-  @override
-  dynamic noSuchMethod(Invocation invocation) {
-    if (invocation.memberName == #call && invocation.isMethod) {
-      final positionalArguments = invocation.positionalArguments;
-      assert(_validateChildren(positionalArguments.length == 1 ? positionalArguments.single : positionalArguments));
-
-      final factory = componentFactory;
-      if (factory is ReactComponentFactoryProxy) {
-        // Use `build` instead of using emulated function behavior to work around DDC issue
-        // https://github.com/dart-lang/sdk/issues/29904
-        // Should have the benefit of better performance; TODO optimize type check?
-        // ignore: avoid_as
-        return factory.build(props, invocation.positionalArguments);
-      } else {
-        var parameters = []
-          ..add(props)
-          ..addAll(invocation.positionalArguments);
-        return Function.apply(factory, parameters);
-      }
+    assert(_validateChildren(childArguments.length == 1 ? childArguments.single : childArguments));
+
+    final factory = componentFactory;
+    if (factory is ReactComponentFactoryProxy) {
+      // Use `build` instead of using emulated function behavior to work around DDC issue
+      // https://github.com/dart-lang/sdk/issues/29904
+      // Should have the benefit of better performance; TODO optimize type check?
+      return factory.build(props, childArguments);
+    } else {
+      var parameters = []
+        ..add(props)
+        ..addAll(childArguments);
+      return Function.apply(factory, parameters);
     }
+  }
 
+  @override
+  dynamic noSuchMethod(Invocation invocation) {
     return super.noSuchMethod(invocation);
   }
 
@@ -809,3 +828,8 @@ class ConsumedProps {
 
   const ConsumedProps(this.props, this.keys);
 }
+
+const _notSpecified = const NotSpecified();
+class NotSpecified {
+  const NotSpecified();
+}

lib/src/transformer/impl_generation.dart

@@ -197,7 +197,7 @@ class ImplGenerator {
         ..writeln('  // Work around https://github.com/dart-lang/sdk/issues/16030 by making')
         ..writeln('  // the original props class abstract and redeclaring `call` in the impl class.')
         ..writeln('  @override')
-        ..writeln('  call([children, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, c31, c32, c33, c34, c35, c36, c37, c38, c39, c40]);')
+        ..writeln('  call([c1, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, c31, c32, c33, c34, c35, c36, c37, c38, c39, c40]);')
         ..writeln('}')
         ..writeln();
 

lib/src/util/react_util.dart

@@ -1,7 +1,7 @@
 import 'dart:collection';
 
-import 'package:over_react/over_react.dart';
 import 'package:over_react/component_base.dart' as component_base show UiProps;
+import 'package:over_react/over_react.dart';
 
 /// A `MapView` helper that stubs in unimplemented pieces of [UiProps].
 ///
@@ -58,5 +58,5 @@ class UiPropsMapView extends MapView with ReactPropsMixin, UbiquitousDomPropsMix
       throw new UnimplementedError('@PropsMixin instances do not implement componentFactory');
 
   @override
-  ReactElement call([children, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, c31, c32, c33, c34, c35, c36, c37, c38, c39, c40]) => throw new UnimplementedError('@PropsMixin instances do not implement call');
+  ReactElement call([c1, c2, c3, c4, c5, c6, c7, c8, c9, c10, c11, c12, c13, c14, c15, c16, c17, c18, c19, c20, c21, c22, c23, c24, c25, c26, c27, c28, c29, c30, c31, c32, c33, c34, c35, c36, c37, c38, c39, c40]) => throw new UnimplementedError('@PropsMixin instances do not implement call');
 }

test/over_react/component_declaration/component_base_test.dart

@@ -30,56 +30,103 @@ import '../../test_util/test_util.dart';
 import '../shared/map_proxy_tests.dart';
 
 main() {
-  group('component base:', () {
-    group('UiProps', () {
-      group('warns and throws when rendering a DOM component', () {
-        bool warningsWereEnabled;
-        setUp(() {
-          warningsWereEnabled = ValidationUtil.WARNINGS_ENABLED;
-          ValidationUtil.WARNINGS_ENABLED = false;
-          startRecordingValidationWarnings();
-        });
+  void _commonNonInvokedBuilderTests(UiProps factory) {
+    bool warningsWereEnabled;
+    setUp(() {
+      warningsWereEnabled = ValidationUtil.WARNINGS_ENABLED;
+      ValidationUtil.WARNINGS_ENABLED = false;
+      startRecordingValidationWarnings();
+    });
 
-        tearDown(() {
-          ValidationUtil.WARNINGS_ENABLED = warningsWereEnabled;
-          stopRecordingValidationWarnings();
-        });
+    tearDown(() {
+      ValidationUtil.WARNINGS_ENABLED = warningsWereEnabled;
+      stopRecordingValidationWarnings();
+    });
 
-        test('when a single non-invoked builder child is passed in', () {
-          expect(() => Dom.div()(Dom.div()), throwsArgumentError);
-          verifyValidationWarning(contains('It looks like you are trying to use a non-invoked builder as a child.'));
-        });
+    test('when a single non-invoked builder child is passed in', () {
+      expect(() => factory(Dom.div()), throwsArgumentError);
+      verifyValidationWarning(contains(
+          'It looks like you are trying to use a non-invoked builder as a child.'));
+    });
 
-        test('when a list with a non-invoked builder child passed in', () {
-          expect(() => Dom.div()([
+    test('when a list with a non-invoked builder child passed in', () {
+      expect(() =>
+          factory([
             Dom.div(),
             Dom.p()(),
             Dom.div()
           ]), throwsArgumentError);
-          verifyValidationWarning(contains('It looks like you are trying to use a non-invoked builder as a child.'));
-        });
+      verifyValidationWarning(contains(
+          'It looks like you are trying to use a non-invoked builder as a child.'));
+    });
 
-        test('except when an iterable with a non-invoked builder child passed in', () {
-          var children = (() sync* {
-            yield Dom.div();
-            yield Dom.p()();
-            yield Dom.div();
-          })();
-          expect(() => Dom.div()(children), returnsNormally);
-          rejectValidationWarning(anything);
-        });
+    test(
+        'except when an iterable with a non-invoked builder child passed in', () {
+      var children = (() sync* {
+        yield Dom.div();
+        yield Dom.p()();
+        yield Dom.div();
+      })();
+      expect(() => factory(children), returnsNormally);
+      rejectValidationWarning(anything);
+    });
 
-        test('when non-invoked builder children are passed in variadically via noSuchMethod', () {
-          expect(() => Dom.div()(
-            Dom.div(),
-            Dom.p()(),
-            Dom.div()
+    test('when non-invoked builder children are passed in variadically', () {
+      expect(() =>
+          factory(
+              Dom.div(),
+              Dom.p()(),
+              Dom.div()
           ), throwsArgumentError);
-          verifyValidationWarning(contains('It looks like you are trying to use a non-invoked builder as a child.'));
-        });
+      verifyValidationWarning(contains(
+          'It looks like you are trying to use a non-invoked builder as a child.'));
+    });
+  }
+
+  void _commonVariadicChildrenTests(UiProps factory) {
+    // There are different code paths for 0, 1, 2, 3, 4, 5, 6, and 6+ arguments.
+    // Test all of them.
+    group('a number of variadic children:', () {
+      test('0', () {
+        final instance = factory();
+        expect(getJsChildren(instance), isNull);
+      });
+
+      test('1', () {
+        final instance = factory(1);
+        expect(getJsChildren(instance), equals(1));
+      });
+
+      const firstGeneralCaseVariadicChildCount = 2;
+      const maxSupportedVariadicChildCount = 40;
+      for (var i = firstGeneralCaseVariadicChildCount; i < maxSupportedVariadicChildCount; i++) {
+        final childrenCount = i;
+        test('$childrenCount', () {
+          final expectedChildren = new List.generate(childrenCount, (i) => i + 1);
+          final arguments = <dynamic>[]..add(expectedChildren);
+          final instance = Function.apply(factory, arguments);
+          expect(getJsChildren(instance), expectedChildren);
+        });
+      }
+
+      test('$maxSupportedVariadicChildCount (and passes static analysis)', () {
+        final instance = factory(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40);
+        // Generate these instead of hard coding them to ensure the arguments passed into this test match maxSupportedVariadicChildCount
+        final expectedChildren = new List.generate(maxSupportedVariadicChildCount, (i) => i + 1);
+        expect(getJsChildren(instance), equals(expectedChildren));
+      });
+    });
+  }
+
+  group('component base:', () {
+    group('UiProps', () {
+      group('warns and throws when rendering a DOM component', () {
+        _commonNonInvokedBuilderTests(Dom.div());
       }, testOn: '!js');
 
       group('renders a DOM component with the correct children when', () {
+        _commonVariadicChildrenTests(Dom.div());
+
         test('no children are passed in', () {
           var renderedNode = renderAndGetDom(Dom.div()());
 
@@ -124,7 +171,7 @@ main() {
           expect(childNodes[1].text, equals('Second Child'));
         });
 
-        test('children are set variadically via noSuchMethod', () {
+        test('children are set variadically', () {
           var firstChild = 'First Child';
           var secondChild = 'Second Child';
           var renderedNode = renderAndGetDom(Dom.div()(firstChild, secondChild));
@@ -137,54 +184,14 @@ main() {
       });
 
       group('warns and throws when rendering a Dart composite component', () {
-        bool warningsWereEnabled;
-        setUp(() {
-          warningsWereEnabled = ValidationUtil.WARNINGS_ENABLED;
-          ValidationUtil.WARNINGS_ENABLED = false;
-          startRecordingValidationWarnings();
-        });
-
-        tearDown(() {
-          ValidationUtil.WARNINGS_ENABLED = warningsWereEnabled;
-          stopRecordingValidationWarnings();
-        });
-
-        test('when a single non-invoked builder child is passed in', () {
-          expect(() => TestComponent()(Dom.div()), throwsArgumentError);
-          verifyValidationWarning(contains('It looks like you are trying to use a non-invoked builder as a child.'));
-        });
-
-        test('when a list with a non-invoked builder child passed in', () {
-          expect(() => TestComponent()([
-            Dom.div(),
-            Dom.p()(),
-            Dom.div()
-          ]), throwsArgumentError);
-          verifyValidationWarning(contains('It looks like you are trying to use a non-invoked builder as a child.'));
-        });
-
-        test('except when an iterable with a non-invoked builder passed in', () {
-          var children = (() sync* {
-            yield Dom.div();
-            yield Dom.p()();
-            yield Dom.div();
-          })();
-          expect(() => TestComponent()(children), returnsNormally);
-          rejectValidationWarning(anything);
-        });
-
-        test('when non-invoked builder children are passed in variadically via noSuchMethod', () {
-          expect(() => TestComponent()(
-            Dom.div(),
-            Dom.p()(),
-            Dom.div()
-          ), throwsArgumentError);
-          verifyValidationWarning(contains('It looks like you are trying to use a non-invoked builder as a child.'));
-        });
+        _commonNonInvokedBuilderTests(Dom.div());
       }, testOn: '!js');
 
       group('renders a composite Dart component with the correct children when', () {
+        _commonVariadicChildrenTests(TestComponent());
+
         test('no children are passed in', () {
+
           var renderedInstance = render(TestComponent()());
 
           expect(getDartChildren(renderedInstance), new isInstanceOf<List>(), reason: 'Should be a list because lists will be JSified');
@@ -237,7 +244,7 @@ main() {
           expect(getJsChildren(renderedInstance), orderedEquals(children));
         });
 
-        test('children are set variadically via noSuchMethod', () {
+        test('children are set variadically', () {
           var firstChild = 'First Child';
           var secondChild = 'Second Child';
           var renderedInstance = render(TestComponent()(firstChild, secondChild));

test/vm_tests/transformer/impl_generation_test.dart

@@ -1009,7 +1009,10 @@ main() {
         MethodDeclaration propsImplCall = propsImplClass.members
             .singleWhere((entity) => entity is MethodDeclaration && entity.name?.name == 'call');
 
-        expect(propsImplCall.parameters.toSource(), uiPropsCall.parameters.toSource(),
+        var generatedParameterList = uiPropsCall.parameters.parameters.expand((param) => [param.identifier.name]);
+        var expectedParameterList = uiPropsCall.parameters.parameters.expand((param) => [param.identifier.name]);
+
+        expect(generatedParameterList.toString(), expectedParameterList.toString(),
             reason: 'should have the correct number of arguments');
         expect(propsImplCall.metadata, contains(predicate((meta) => meta.name?.name == 'override')),
             reason: 'should have @override');