From d0a3cb7505c324de4cc34cab11be6b443029dc27 Mon Sep 17 00:00:00 2001 From: Corwin Sheahan Date: Fri, 7 Dec 2018 09:57:49 -0700 Subject: [PATCH 1/3] Add workaround for NSM --- lib/src/component/dom_components.dart | 4 +- .../component_declaration/component_base.dart | 66 ++++--- lib/src/transformer/impl_generation.dart | 2 +- lib/src/util/react_util.dart | 4 +- .../component_base_test.dart | 173 +++++++++--------- .../transformer/impl_generation_test.dart | 5 +- 6 files changed, 144 insertions(+), 110 deletions(-) diff --git a/lib/src/component/dom_components.dart b/lib/src/component/dom_components.dart index e20335b7a..c30a36407 100644 --- a/lib/src/component/dom_components.dart +++ b/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. diff --git a/lib/src/component_declaration/component_base.dart b/lib/src/component_declaration/component_base.dart index 30d5a04ed..0cf526400 100644 --- a/lib/src/component_declaration/component_base.dart +++ b/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 /// 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(); +} diff --git a/lib/src/transformer/impl_generation.dart b/lib/src/transformer/impl_generation.dart index d4f6fb930..62a96f5b1 100644 --- a/lib/src/transformer/impl_generation.dart +++ b/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(); diff --git a/lib/src/util/react_util.dart b/lib/src/util/react_util.dart index bd0065b2b..66bacec23 100644 --- a/lib/src/util/react_util.dart +++ b/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'); } diff --git a/test/over_react/component_declaration/component_base_test.dart b/test/over_react/component_declaration/component_base_test.dart index 53154c594..81652202a 100644 --- a/test/over_react/component_declaration/component_base_test.dart +++ b/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 = []..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(), 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)); diff --git a/test/vm_tests/transformer/impl_generation_test.dart b/test/vm_tests/transformer/impl_generation_test.dart index 6881fcd33..8f009651e 100644 --- a/test/vm_tests/transformer/impl_generation_test.dart +++ b/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'); From eeb947c648c22d8c97d98638aa7e6bb0dedb1e2c Mon Sep 17 00:00:00 2001 From: Corwin Sheahan Date: Mon, 10 Dec 2018 11:56:42 -0700 Subject: [PATCH 2/3] Remove NSM --- lib/src/component_declaration/component_base.dart | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/src/component_declaration/component_base.dart b/lib/src/component_declaration/component_base.dart index 0cf526400..bb07f7471 100644 --- a/lib/src/component_declaration/component_base.dart +++ b/lib/src/component_declaration/component_base.dart @@ -677,11 +677,6 @@ abstract class UiProps extends MapBase } } - @override - dynamic noSuchMethod(Invocation invocation) { - return super.noSuchMethod(invocation); - } - /// Validates that no [children] are instances of [UiProps], and prints a helpful message for a better debugging /// experience. bool _validateChildren(dynamic children) { From 2e1cb13ef017fa6909eb51e83eaf0291fe62c624 Mon Sep 17 00:00:00 2001 From: Corwin Sheahan Date: Thu, 13 Dec 2018 09:56:55 -0700 Subject: [PATCH 3/3] Address review comments * Naming adjustment * actually test paths for call() on UiProps * Remove unneeded call() instances --- lib/src/component/dom_components.dart | 6 --- lib/src/transformer/impl_generation.dart | 5 --- .../component_base_test.dart | 22 +++++------ .../transformer/impl_generation_test.dart | 37 ------------------- 4 files changed, 11 insertions(+), 59 deletions(-) diff --git a/lib/src/component/dom_components.dart b/lib/src/component/dom_components.dart index c30a36407..2b784eae8 100644 --- a/lib/src/component/dom_components.dart +++ b/lib/src/component/dom_components.dart @@ -52,9 +52,6 @@ class DomProps extends component_base.UiProps @override String get propKeyNamespace => ''; - - @override - 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 @@ -73,9 +70,6 @@ class SvgProps extends component_base.UiProps @override String get propKeyNamespace => ''; - - @override - 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. diff --git a/lib/src/transformer/impl_generation.dart b/lib/src/transformer/impl_generation.dart index 62a96f5b1..60550af57 100644 --- a/lib/src/transformer/impl_generation.dart +++ b/lib/src/transformer/impl_generation.dart @@ -193,11 +193,6 @@ class ImplGenerator { ..writeln(' /// The default namespace for the prop getters/setters generated for this class.') ..writeln(' @override') ..writeln(' String get propKeyNamespace => ${stringLiteral(propKeyNamespace)};') - ..writeln() - ..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([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(); diff --git a/test/over_react/component_declaration/component_base_test.dart b/test/over_react/component_declaration/component_base_test.dart index 81652202a..413d004ba 100644 --- a/test/over_react/component_declaration/component_base_test.dart +++ b/test/over_react/component_declaration/component_base_test.dart @@ -30,7 +30,7 @@ import '../../test_util/test_util.dart'; import '../shared/map_proxy_tests.dart'; main() { - void _commonNonInvokedBuilderTests(UiProps factory) { + void _commonNonInvokedBuilderTests(UiProps builder) { bool warningsWereEnabled; setUp(() { warningsWereEnabled = ValidationUtil.WARNINGS_ENABLED; @@ -44,14 +44,14 @@ main() { }); test('when a single non-invoked builder child is passed in', () { - expect(() => factory(Dom.div()), throwsArgumentError); + expect(() => builder(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(() => - factory([ + builder([ Dom.div(), Dom.p()(), Dom.div() @@ -67,13 +67,13 @@ main() { yield Dom.p()(); yield Dom.div(); })(); - expect(() => factory(children), returnsNormally); + expect(() => builder(children), returnsNormally); rejectValidationWarning(anything); }); test('when non-invoked builder children are passed in variadically', () { expect(() => - factory( + builder( Dom.div(), Dom.p()(), Dom.div() @@ -83,17 +83,17 @@ main() { }); } - void _commonVariadicChildrenTests(UiProps factory) { + void _commonVariadicChildrenTests(UiProps builder) { // 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(); + final instance = builder(); expect(getJsChildren(instance), isNull); }); test('1', () { - final instance = factory(1); + final instance = builder(1); expect(getJsChildren(instance), equals(1)); }); @@ -103,14 +103,14 @@ main() { final childrenCount = i; test('$childrenCount', () { final expectedChildren = new List.generate(childrenCount, (i) => i + 1); - final arguments = []..add(expectedChildren); - final instance = Function.apply(factory, arguments); + final arguments = []..addAll(expectedChildren); + final instance = Function.apply(builder, 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); + final instance = builder(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)); diff --git a/test/vm_tests/transformer/impl_generation_test.dart b/test/vm_tests/transformer/impl_generation_test.dart index 8f009651e..fb4827e3d 100644 --- a/test/vm_tests/transformer/impl_generation_test.dart +++ b/test/vm_tests/transformer/impl_generation_test.dart @@ -985,43 +985,6 @@ main() { uiPropsCall = uiPropsClass.members .singleWhere((entity) => entity is MethodDeclaration && entity.name?.name == 'call'); }); - - test('generates `call` override on the _\$*PropsImpl class correctly, as a workaround for dart-lang/sdk#16030', () { - setUpAndGenerate(''' - @Factory() - UiFactory Foo; - - @Props() - class FooProps {} - - @Component() - class FooComponent { - render() => null; - } - '''); - - var transformedSource = transformedFile.getTransformedText(); - var parsedTransformedSource = parseCompilationUnit(transformedSource); - - ClassDeclaration propsImplClass = parsedTransformedSource.declarations - .singleWhere((entity) => entity is ClassDeclaration && entity.name?.name == r'_$FooPropsImpl'); - - MethodDeclaration propsImplCall = propsImplClass.members - .singleWhere((entity) => entity is MethodDeclaration && entity.name?.name == 'call'); - - 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'); - expect(propsImplCall.returnType, null, - reason: 'should not be typed, since ReactElement may not be imported'); - expect(propsImplCall.isAbstract, isTrue, - reason: 'should be abstract; the declaration is only for dart2js bug workaround purposes, ' - 'and the inherited implementation should be used'); - }); }); test('getComponentFactoryName() throws an error when its argument is null', () {