diff --git a/dart_test.yaml b/dart_test.yaml new file mode 100644 index 000000000..022702d61 --- /dev/null +++ b/dart_test.yaml @@ -0,0 +1,9 @@ +tags: + # DDC-related tags declared here, since you can't target the DDC using platform selectors: + # https://github.com/dart-lang/test/issues/652 + + # Tests that should only be run in the dev compiler + ddc: + skip: "skipping DDC-specific tests until UIP-2410 is implemented" + # Tests that should NOT be run in the dev compiler + no-ddc: diff --git a/lib/src/component/dom_components.dart b/lib/src/component/dom_components.dart index 9950a4f03..7b9bda8b5 100644 --- a/lib/src/component/dom_components.dart +++ b/lib/src/component/dom_components.dart @@ -35,7 +35,7 @@ DomProps domProps([Map backingMap]) => new DomProps(null, backingMap); typedef DomProps DomPropsFactory(); -class DomProps extends component_base.UiProps with DomPropsMixin, ReactPropsMixin { +class DomProps extends component_base.UiProps with DomPropsMixin { // Wrap Map literal in parens to work around https://github.com/dart-lang/sdk/issues/24410 DomProps(this.componentFactory, [Map props]) : this.props = props ?? ({}); @@ -45,7 +45,7 @@ class DomProps extends component_base.UiProps with DomPropsMixin, ReactPropsMixi final Map props; } -class SvgProps extends component_base.UiProps with DomPropsMixin, ReactPropsMixin, SvgPropsMixin implements DomProps { +class SvgProps extends component_base.UiProps with DomPropsMixin, SvgPropsMixin implements DomProps { // Wrap Map literal in parens to work around https://github.com/dart-lang/sdk/issues/24410 SvgProps(this.componentFactory, [Map props]) : this.props = props ?? ({}); diff --git a/lib/src/component_declaration/component_base.dart b/lib/src/component_declaration/component_base.dart index 295f09afe..cb3160e12 100644 --- a/lib/src/component_declaration/component_base.dart +++ b/lib/src/component_declaration/component_base.dart @@ -28,6 +28,7 @@ import 'package:over_react/over_react.dart' show PropError; import 'package:over_react/src/component_declaration/component_type_checking.dart'; +import 'package:over_react/src/util/ddc_emulated_function_name_bug.dart' as ddc_emulated_function_name_bug; import 'package:react/react.dart' as react; import 'package:react/react_client.dart'; @@ -257,7 +258,31 @@ abstract class UiStatefulComponent this.state; + @override String toString() => '$runtimeType: ${prettyPrintMap(_map)}'; + + // Manually implement members from `MapViewMixin`, + // since mixing that class in doesn't play well with the DDC. + // TODO find out root cause and reduced test case. + @override operator[](Object key) => _map[key]; + @override void operator[]=(key, value) { _map[key] = value; } + @override void addAll(other) { _map.addAll(other); } + @override void clear() { _map.clear(); } + @override putIfAbsent(key, ifAbsent()) => _map.putIfAbsent(key, ifAbsent); + @override bool containsKey(Object key) => _map.containsKey(key); + @override bool containsValue(Object value) => _map.containsValue(value); + @override void forEach(void action(key, value)) { _map.forEach(action); } + @override bool get isEmpty => _map.isEmpty; + @override bool get isNotEmpty => _map.isNotEmpty; + @override int get length => _map.length; + @override Iterable get keys => _map.keys; + @override remove(Object key) => _map.remove(key); + @override Iterable get values => _map.values; +} /// The string used by default for the key of the attribute added by [UiProps.addTestId]. const defaultTestIdKey = 'data-test-id'; @@ -275,9 +300,41 @@ typedef PropsModifier(Map props); /// /// Note: Implements MapViewMixin instead of extending it so that the abstract [Props] declarations /// don't need a constructor. The generated implementations can mix that functionality in. -abstract class UiProps - extends Object with MapViewMixin, PropsMapViewMixin, ReactPropsMixin, UbiquitousDomPropsMixin, CssClassPropsMixin - implements Map { +abstract class UiProps extends Object + with ReactPropsMixin, UbiquitousDomPropsMixin, CssClassPropsMixin + implements PropsMapViewMixin, MapViewMixin, Map { + + UiProps() { + // Work around https://github.com/dart-lang/sdk/issues/27647 for all UiProps instances + if (ddc_emulated_function_name_bug.isBugPresent) { + ddc_emulated_function_name_bug.patchName(this); + } + } + + // Manually implement members from `MapViewMixin`, + // since mixing that class in doesn't play well with the DDC. + // TODO find out root cause and reduced test case. + @override operator[](Object key) => _map[key]; + @override void operator[]=(key, value) { _map[key] = value; } + @override void addAll(other) { _map.addAll(other); } + @override void clear() { _map.clear(); } + @override putIfAbsent(key, ifAbsent()) => _map.putIfAbsent(key, ifAbsent); + @override bool containsKey(Object key) => _map.containsKey(key); + @override bool containsValue(Object value) => _map.containsValue(value); + @override void forEach(void action(key, value)) { _map.forEach(action); } + @override bool get isEmpty => _map.isEmpty; + @override bool get isNotEmpty => _map.isNotEmpty; + @override int get length => _map.length; + @override Iterable get keys => _map.keys; + @override remove(Object key) => _map.remove(key); + @override Iterable get values => _map.values; + + // Manually implement members from `StateMapViewMixin`, + // since mixing that class in doesn't play well with the DDC. + // TODO find out root cause and reduced test case. + @override Map get _map => this.props; + @override String toString() => '$runtimeType: ${prettyPrintMap(_map)}'; + /// Adds an arbitrary prop key-value pair if [shouldAdd] is true, otherwise, does nothing. void addProp(propKey, value, [bool shouldAdd = true]) { if (!shouldAdd) return; @@ -362,10 +419,6 @@ abstract class UiProps @override dynamic noSuchMethod(Invocation invocation) { if (invocation.memberName == #call && invocation.isMethod) { - var parameters = [] - ..add(props) - ..addAll(invocation.positionalArguments); - assert(() { // These checks are within the assert so they are not done in production. var children = invocation.positionalArguments; @@ -377,7 +430,18 @@ abstract class UiProps return _validateChildren(children); }); - return Function.apply(componentFactory, parameters); + 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, invocation.positionalArguments); + } else { + var parameters = [] + ..add(props) + ..addAll(invocation.positionalArguments); + return Function.apply(factory, parameters); + } } return super.noSuchMethod(invocation); diff --git a/lib/src/transformer/README.md b/lib/src/transformer/README.md index 73bbb3e1e..63d441541 100644 --- a/lib/src/transformer/README.md +++ b/lib/src/transformer/README.md @@ -16,7 +16,23 @@ much cleaner and more _grokkable_.     - +## Transformer options: +The following configuration options are available for the `over_react` transformer. + +All values shown are the defaults + +```yaml +transformers: +- over_react: + # Whether to apply a workaround in transformed props/state classes for a DDC bug + # in which abstract accessors clobber inherited concrete implementations: + # https://github.com/dart-lang/sdk/issues/29914. + # + # Fixes the issue by generating corresponding abstract getters/setters to + # complete the pair, limited to problematic accessors within transformed + # props/state classes that have the `@override` annotation. + fixDdcAbstractAccessors: false +``` ## Wiring it all up diff --git a/lib/src/transformer/impl_generation.dart b/lib/src/transformer/impl_generation.dart index 7c0fa81ec..01f98c8bb 100644 --- a/lib/src/transformer/impl_generation.dart +++ b/lib/src/transformer/impl_generation.dart @@ -53,6 +53,7 @@ class ImplGenerator { final TransformLogger logger; final TransformedSourceFile transformedFile; + bool shouldFixDdcAbstractAccessors = false; SourceFile get sourceFile => transformedFile.sourceFile; @@ -392,6 +393,10 @@ class ImplGenerator { AccessorType type, NodeWithMeta typedMap ) { + if (shouldFixDdcAbstractAccessors) { + fixDdcAbstractAccessors(type, typedMap); + } + String keyNamespace = getAccessorKeyNamespace(typedMap); final bool isProps = type == AccessorType.props; @@ -588,6 +593,66 @@ class ImplGenerator { staticVariablesImpl ); } + + /// Apply a workaround for an issue where, in the DDC, abstract getter or setter overrides declared in a class clobber + /// the inherited concrete implementations. + /// + /// Fixes the issue by generating corresponding abstract getters/setters to complete the pair + /// for accessors with the `@override` annotation. + void fixDdcAbstractAccessors( + AccessorType accessorType, + NodeWithMeta typedMap, + ) { + var candidateAccessors = new List.from( + typedMap.node.members.where((member) => + member is MethodDeclaration && + (member.isGetter || member.isSetter) && + !member.isSynthetic && + !member.isStatic && + member.metadata.any((meta) => meta.name.name == 'override') + ) + ); + + for (var accessor in candidateAccessors) { + // Non-abstract accessors don't exhibit this issue. + if (!accessor.isAbstract) return; + + var name = accessor.name.name; + + // Don't generate for `Map get props;`/`Map get state;` in mixins + if (accessorType == AccessorType.props && name == 'props') continue; + if (accessorType == AccessorType.state && name == 'state') continue; + + if (candidateAccessors.any((other) => other != accessor && other.name.name == name)) { + // Don't generate when both the getter and the setter are declared. + continue; + } + + /// Warning: tests rely on this comment as a means of determining whether this fix was applied. + /// + /// DO NOT modify or remove without updating tests + const String generatedComment = ' /* fixDdcAbstractAccessors workaround: */ '; + + if (accessor.isGetter) { + var type = accessor.returnType?.toSource(); + var typeString = type == null ? '' : '$type '; + + transformedFile.insert(sourceFile.location(accessor.end), + // use `covariant` here to be extra safe in this typing + '${generatedComment}set $name(covariant ${typeString}value);'); + } else { + var parameter = accessor.parameters.parameters.single; + var type = parameter is SimpleFormalParameter + ? parameter.type?.toSource() + // This `null` case is mainly for [FunctionTypedFormalParameter]. + : null; + var typeString = type == null ? '' : '$type '; + + transformedFile.insert(sourceFile.location(accessor.end), + '$generatedComment${typeString}get $name;'); + } + } + } } enum AccessorType {props, state} diff --git a/lib/src/util/ddc_emulated_function_name_bug.dart b/lib/src/util/ddc_emulated_function_name_bug.dart new file mode 100644 index 000000000..3fcadae89 --- /dev/null +++ b/lib/src/util/ddc_emulated_function_name_bug.dart @@ -0,0 +1,85 @@ +/// Provides detection and patching of the bug described in , +/// in which getters/setters with the identifier `name` don't work for emulated function classes, like [UiProps]. +@JS() +library over_react.src.util.ddc_emulated_function_name_bug; + +import 'package:js/js.dart'; +import 'package:over_react/over_react.dart'; + +/// Create a reduced test case of the issue, using an emulated function pattern that is similar to [UiProps]. +/// +/// We can't use [UiProps] itself, since it uses [isBugPresent], and that would cause a cyclic initialization error. +class _NsmEmulatedFunctionWithNameProperty implements Function { + void call(); + + @override + noSuchMethod(i) {} + + String _name; + + // ignore: unnecessary_getters_setters + String get name => _name; + // ignore: unnecessary_getters_setters + set name(String value) => _name = value; +} + +/// Whether this bug, , is present in the current runtime. +/// +/// This performs functional detection of the bug, and will be `true` +/// only in the DDC and only in versions of the DDC where this bug is present. +final bool isBugPresent = (() { + const testValue = 'test value'; + + var testObject = new _NsmEmulatedFunctionWithNameProperty(); + + try { + // In the DDC, this throws: + // TypeError: Cannot assign to read only property 'name' of function 'function call(...args) { + // return call.call.apply(call, args); + // }' + testObject.name = testValue; + } catch(_) { + return true; + } + + try { + // We don't expect accessing this to throw, but just in case... + return testObject.name != testValue; + } catch(_) { + return true; + } +})(); + + +@JS() +@anonymous +class _PropertyDescriptor {} + +@JS('Object.getPrototypeOf') +external dynamic _getPrototypeOf(dynamic object); + +@JS('Object.getOwnPropertyDescriptor') +external _PropertyDescriptor _getOwnPropertyDescriptor(dynamic object, String propertyName); + +@JS('Object.defineProperty') +external void _defineProperty(dynamic object, String propertyName, _PropertyDescriptor descriptor); + +/// Patches the `name` property on the given [object] to have the expected behavior +/// by copying the property descriptor for `name` from the appropriate prototype. +/// +/// This is a noop if `name` is not a property on the given object. +/// +/// __This functionality is unstable, and should not be used when [isBugPresent] is `false`.__ +/// +/// This method also had undefined behavior on non-[UiProps] instances. +void patchName(dynamic object) { + var current = object; + while ((current = _getPrototypeOf(current)) != null) { + var nameDescriptor = _getOwnPropertyDescriptor(current, 'name'); + + if (nameDescriptor != null) { + _defineProperty(object, 'name', nameDescriptor); + return; + } + } +} diff --git a/lib/src/util/react_wrappers.dart b/lib/src/util/react_wrappers.dart index bb81b3669..3618fdd5e 100644 --- a/lib/src/util/react_wrappers.dart +++ b/lib/src/util/react_wrappers.dart @@ -101,7 +101,32 @@ Map getJsProps(/* ReactElement|ReactComponent */ instance) { return props; } -Expando _elementPropsCache = new Expando('_elementPropsCache'); +/// Whether [Expando]s can be used on [ReactElement]s. +/// +/// At the time this was written, this should return: +/// +/// - `true` for dart2js and Dart VM +/// - `false` for DDC +final bool _canUseExpandoOnReactElement = (() { + var expando = new Expando('_canUseExpandoOnReactElement test'); + var reactElement = react.div({}); + + try { + expando[reactElement] = true; + } catch(_) { + return false; + } + + return true; +})(); + +/// A cache of props for a given [ReactElement]. +/// +/// If caching isn't possible due to [_canUseExpandoOnReactElement] being false, +/// then this will be initialized to `null`, and caching will be disabled. +final Expando _elementPropsCache = _canUseExpandoOnReactElement + ? new Expando('_elementPropsCache') + : null; /// Returns an unmodifiable Map view of props for a [ReactElement] or composite [ReactComponent] [instance]. /// @@ -140,7 +165,7 @@ Map getProps(/* ReactElement|ReactComponent */ instance, {bool traverseWrappers: } } - if (!isCompositeComponent) { + if (_elementPropsCache != null && !isCompositeComponent) { var cachedView = _elementPropsCache[instance]; if (cachedView != null) return cachedView; } @@ -148,7 +173,9 @@ Map getProps(/* ReactElement|ReactComponent */ instance, {bool traverseWrappers: var propsMap = isDartComponent(instance) ? _getExtendedProps(instance) : getJsProps(instance); var view = new UnmodifiableMapView(propsMap); - if (!isCompositeComponent) _elementPropsCache[instance] = view; + if (_elementPropsCache != null && !isCompositeComponent) { + _elementPropsCache[instance] = view; + } return view; } diff --git a/lib/transformer.dart b/lib/transformer.dart index 24750671b..d48e4d4af 100644 --- a/lib/transformer.dart +++ b/lib/transformer.dart @@ -33,10 +33,35 @@ import 'package:transformer_utils/transformer_utils.dart'; /// * Generates implementations for stubbed props/state/component classes. /// * Creates component factories, registers them with react-dart, and wires them up to /// their associated props/component implementations. +/// +/// +/// The following configuration options are available for the `over_react` transformer. +/// All values shown are the defaults. +/// +/// transformers: +/// - over_react: +/// # Whether to apply a workaround in transformed props/state classes for a DDC bug +/// # in which abstract accessors clobber inherited concrete implementations: +/// # https://github.com/dart-lang/sdk/issues/29914. +/// # +/// # Fixes the issue by generating corresponding abstract getters/setters to +/// # complete the pair, limited to problematic accessors within transformed +/// # props/state classes that have the `@override` annotation. +/// fixDdcAbstractAccessors: false + class WebSkinDartTransformer extends Transformer implements LazyTransformer { final BarbackSettings _settings; + final bool _shouldFixDdcAbstractAccessors; - WebSkinDartTransformer.asPlugin(this._settings); + WebSkinDartTransformer.asPlugin(this._settings) : + _shouldFixDdcAbstractAccessors = _loadBoolConfig(_settings, 'fixDdcAbstractAccessors'); + + static bool _loadBoolConfig(BarbackSettings _settings, String configKey, {bool defaultValue: false}) { + var value = _settings.configuration[configKey] ?? defaultValue; + if (value is bool) return value; + + throw new ArgumentError.value(value, configKey, 'must be a bool'); + } /// Declare the assets this transformer uses. Only dart assets will be transformed. @override @@ -89,7 +114,8 @@ class WebSkinDartTransformer extends Transformer implements LazyTransformer { // If there are no errors, generate the component. if (!declarations.hasErrors) { new ImplGenerator(logger, transformedFile) - .generate(declarations); + ..shouldFixDdcAbstractAccessors = _shouldFixDdcAbstractAccessors + ..generate(declarations); } } diff --git a/pubspec.yaml b/pubspec.yaml index f7b001ae6..f22600879 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -13,7 +13,7 @@ dependencies: logging: ">=0.11.3+1 <1.0.0" meta: "^1.0.4" path: "^1.4.1" - react: "^3.2.2" + react: "^3.4.0" source_span: "^1.2.0" transformer_utils: "^0.1.1" w_flux: "^2.7.1" diff --git a/test/over_react/component_declaration/component_base_test.dart b/test/over_react/component_declaration/component_base_test.dart index 1f49c9cef..14da990d3 100644 --- a/test/over_react/component_declaration/component_base_test.dart +++ b/test/over_react/component_declaration/component_base_test.dart @@ -247,6 +247,36 @@ main() { }); }); + test('invokes a non-ReactComponentFactoryProxy componentFactory function properly when invoked', () { + final ReactElement expectedReturnValue = Dom.div()(); + const expectedProps = const {'testProp': 'testValue'}; + + var calls = []; + + ReactElement customFactory([Map props, a = 0, b = 0, c = 0, d = 0]) { + calls.add([props, a, b, c, d]); + return expectedReturnValue; + } + + var builder = new TestUiPropsWithCustomComponentFactory() + ..componentFactory = customFactory + ..['testProp'] = 'testValue'; + + expect(builder(), expectedReturnValue); + expect(builder(1), expectedReturnValue); + expect(builder(1, 2), expectedReturnValue); + expect(builder(1, 2, 3), expectedReturnValue); + expect(builder(1, 2, 3, 4), expectedReturnValue); + + expect(calls, [ + [expectedProps, 0, 0, 0, 0], + [expectedProps, 1, 0, 0, 0], + [expectedProps, 1, 2, 0, 0], + [expectedProps, 1, 2, 3, 0], + [expectedProps, 1, 2, 3, 4], + ]); + }); + group('provides Map functionality:', () { test('is a Map', () { expect(new TestComponentProps(), const isInstanceOf()); @@ -427,6 +457,24 @@ main() { }); }); + // These tests are here to cover the PropsMapViewMixin, which used to be covered when + // testing UiProps, but isn't anymore since UiProps uses it as an interface instead + // to work around DDC issues. + // + // If these test classes cause trouble when running in the DDC, just disable these tests in the DDC. + group('PropsMapViewMixin provides Map functionality:', () { + mapProxyTests((Map backingMap) => new TestPropsMapViewMixin(backingMap)); + }); + + // These tests are here to cover the StateMapViewMixin, which used to be covered when + // testing UiState, but isn't anymore since UiState uses it as an interface instead + // to work around DDC issues. + // + // If these test classes cause trouble when running in the DDC, just disable these tests in the DDC. + group('StateMapViewMixin provides Map functionality:', () { + mapProxyTests((Map backingMap) => new TestStateMapViewMixin(backingMap)); + }); + group('UiComponent', () { TestComponentComponent component; @@ -781,3 +829,25 @@ class TestStatefulComponentComponent extends UiStatefulComponent testObject.name = testValue, returnsNormally); + expect(testObject.name, testValue); + }, tags: 'ddc', + // Tests run in `ddev coverage` don't respect tags and show up as the 'vm' platform + // so we can use this to disable certain browser tests during coverage. + // Workaround for https://github.com/Workiva/dart_dev/issues/200 + testOn: '!vm'); + }); +} + +class ProblematicClass implements Function { + void call(); + + @override + noSuchMethod(i) {} + + String _name; + + // ignore: unnecessary_getters_setters + String get name => _name; + // ignore: unnecessary_getters_setters + set name(String value) => _name = value; +} diff --git a/test/over_react/util/react_wrappers_test.dart b/test/over_react/util/react_wrappers_test.dart index 8dfd9ea6a..df01b595d 100644 --- a/test/over_react/util/react_wrappers_test.dart +++ b/test/over_react/util/react_wrappers_test.dart @@ -923,6 +923,38 @@ main() { expect(() => getProps(renderedInstance)['style'] = testStyle, throwsUnsupportedError); }); + group('caches the returned unmodifiable map for ReactElements', () { + test('in dart2js and the Dart VM', () { + ReactElement element = TestComponentFactory({ + 'dartProp': 'dart' + }); + + var result1 = getProps(element); + var result2 = getProps(element); + + expect(result1, containsPair('dartProp', 'dart'), reason: 'test setup sanity check'); + expect(result2, same(result1), reason: 'should have returned the same object'); + }, tags: 'no-ddc'); + + test('unless the runtime is the DDC', () { + ReactElement element = TestComponentFactory({ + 'dartProp': 'dart' + }); + + var result1 = getProps(element); + var result2 = getProps(element); + + expect(result1, containsPair('dartProp', 'dart'), reason: 'test setup sanity check'); + expect(result2, isNot(same(result1)), + reason: 'if this test fails, then it\'s possible that the bug was fixed in' + ' a newer version of the Dart SDK, and this test can be removed!'); + }, tags: 'ddc', + // Tests run in `ddev coverage` don't respect tags and show up as the 'vm' platform + // so we can use this to disable certain browser tests during coverage. + // Workaround for https://github.com/Workiva/dart_dev/issues/200 + testOn: '!vm'); + }); + group('throws when passed', () { test('a JS ReactComponent and traverseWrappers is true', () { var renderedInstance = render(testJsComponentFactory({})); diff --git a/test/over_react_util_test.dart b/test/over_react_util_test.dart index 1e7f313b3..cf0886a3d 100644 --- a/test/over_react_util_test.dart +++ b/test/over_react_util_test.dart @@ -26,6 +26,7 @@ import 'package:test/test.dart'; import 'over_react/util/class_names_test.dart' as class_names_test; import 'over_react/util/constants_base_test.dart' as constants_base_test; import 'over_react/util/css_value_util_test.dart' as css_value_util_test; +import 'over_react/util/ddc_emulated_function_name_bug_test.dart' as ddc_emulated_function_name_bug_test; import 'over_react/util/dom_util_test.dart' as dom_util_test; import 'over_react/util/event_helpers_test.dart' as event_helpers_test; import 'over_react/util/guid_util_test.dart' as guid_util_test; @@ -47,6 +48,7 @@ void main() { class_names_test.main(); constants_base_test.main(); css_value_util_test.main(); + ddc_emulated_function_name_bug_test.main(); dom_util_test.main(); event_helpers_test.main(); guid_util_test.main(); diff --git a/test/vm_tests/transformer/impl_generation_test.dart b/test/vm_tests/transformer/impl_generation_test.dart index e72d7b44b..28e9ddda8 100644 --- a/test/vm_tests/transformer/impl_generation_test.dart +++ b/test/vm_tests/transformer/impl_generation_test.dart @@ -47,10 +47,11 @@ main() { implGenerator = new ImplGenerator(logger, transformedFile); } - void setUpAndGenerate(String source) { + void setUpAndGenerate(String source, {bool shouldFixDdcAbstractAccessors: false}) { setUpAndParse(source); - implGenerator = new ImplGenerator(logger, transformedFile); + implGenerator = new ImplGenerator(logger, transformedFile) + ..shouldFixDdcAbstractAccessors = shouldFixDdcAbstractAccessors; implGenerator.generate(declarations); } @@ -398,6 +399,164 @@ main() { expect(transformedLines[5].trimLeft(), startsWith('/* line 5 start */')); }); }); + + group('shouldFixDdcAbstractAccessors', () { + /// The comment always inserted by this patching logic. + const String abstractAccessorsPatchComment = '/* fixDdcAbstractAccessors workaround: */'; + + group('does not patch abstract getters corresponding to the backing map for', () { + test('props classes', () { + setUpAndGenerate(''' + @PropsMixin() class FooPropsMixin { + // override isn't typically used here, but it's necessary to trigger the patching logic + @override + Map get props; + + var bar; + var baz; + } + ''', shouldFixDdcAbstractAccessors: true); + + expect(transformedFile.getTransformedText(), isNot(contains(abstractAccessorsPatchComment))); + }); + + test('state classes', () { + setUpAndGenerate(''' + @StateMixin() class FooStateMixin { + // override isn't typically used here, but it's necessary to trigger the patching logic + @override + Map get state; + } + ''', shouldFixDdcAbstractAccessors: true); + + expect(transformedFile.getTransformedText(), isNot(contains(abstractAccessorsPatchComment))); + }); + }); + + group('patches abstract accessors', () { + group('getters', () { + test('with types', () { + setUpAndGenerate(''' + @PropsMixin() abstract class FooPropsMixin { + Map get props; + + @override + Iterable get bar; + } + ''', shouldFixDdcAbstractAccessors: true); + + expect(transformedFile.getTransformedText(), contains( + 'Iterable get bar; $abstractAccessorsPatchComment set bar(covariant Iterable value);' + )); + }); + + test('without types', () { + setUpAndGenerate(''' + @PropsMixin() abstract class FooPropsMixin { + Map get props; + + @override + get bar; + } + ''', shouldFixDdcAbstractAccessors: true); + + expect(transformedFile.getTransformedText(), contains( + 'get bar; $abstractAccessorsPatchComment set bar(covariant value);' + )); + }); + }); + + group('setters', () { + test('with types', () { + setUpAndGenerate(''' + @PropsMixin() abstract class FooPropsMixin { + Map get props; + + @override + set bar(Iterable value); + } + ''', shouldFixDdcAbstractAccessors: true); + + expect(transformedFile.getTransformedText(), contains( + 'set bar(Iterable value); $abstractAccessorsPatchComment Iterable get bar;' + )); + }); + + test('without types', () { + setUpAndGenerate(''' + @PropsMixin() abstract class FooPropsMixin { + Map get props; + + @override + set bar(value); + } + ''', shouldFixDdcAbstractAccessors: true); + + expect(transformedFile.getTransformedText(), contains( + 'set bar(value); $abstractAccessorsPatchComment get bar;' + )); + }); + }); + + group('in all generated accessor classes', () { + const types = const { + 'props mixin': ''' + @PropsMixin() abstract class FooPropsMixin { + Map get props; + @override get bar; + } + ''', + 'state mixin': ''' + @StateMixin() abstract class FooStateMixin { + Map get state; + @override get bar; + } + ''', + 'props class': ''' + @Factory() UiFactory Foo; + @Props() class FooProps { + @override get bar; + } + @Component() class FooComponent {} + ''', + 'state class': ''' + @Factory() UiFactory Foo; + @Props() class FooProps {} + @State() class FooState { + @override get bar; + } + @Component() class FooComponent {} + ''', + 'abstract props class': ''' + @AbstractProps() abstract class AbstractFooProps { + @override get bar; + } + ''', + 'abstract state class': ''' + @AbstractState() abstract class AbstractFooState { + @override get bar; + } + ''', + }; + + types.forEach((String name, String source) { + test(name, () { + setUpAndGenerate(source, shouldFixDdcAbstractAccessors: true); + expect(transformedFile.getTransformedText(), contains(abstractAccessorsPatchComment)); + }); + }); + + group('unless shouldFixDdcAbstractAccessors is false', () { + types.forEach((String name, String source) { + test(name, () { + setUpAndGenerate(source, shouldFixDdcAbstractAccessors: false); + expect(transformedFile.getTransformedText(), isNot(contains(abstractAccessorsPatchComment))); + }); + }); + }); + }); + }); + }); }); }); diff --git a/test/vm_tests/transformer/transformer_test.dart b/test/vm_tests/transformer/transformer_test.dart new file mode 100644 index 000000000..58e09c6bf --- /dev/null +++ b/test/vm_tests/transformer/transformer_test.dart @@ -0,0 +1,49 @@ +// Copyright 2017 Workiva Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +@TestOn('vm') +library transformer_test; + +import 'package:barback/barback.dart'; +import 'package:test/test.dart'; + +import 'package:over_react/transformer.dart'; + +main() { + group('over_react transformer', () { + group('loads config value:', () { + WebSkinDartTransformer initWithConfig(Map config) => + new WebSkinDartTransformer.asPlugin(new BarbackSettings(config, BarbackMode.DEBUG)); + + group('"fixDdcAbstractAccessors"', () { + const String configKey = 'fixDdcAbstractAccessors'; + + test('when specified as a boolean', () { + expect(() => initWithConfig({configKey: true}), returnsNormally); + expect(() => initWithConfig({configKey: false}), returnsNormally); + }); + + test('when not specified', () { + expect(() => initWithConfig({}), returnsNormally); + expect(() => initWithConfig({configKey: null}), returnsNormally); + }); + + test('and throws when specified with an invalid value', () { + expect(() => initWithConfig({configKey: 'foo'}), throwsArgumentError); + expect(() => initWithConfig({configKey: []}), throwsArgumentError); + }); + }); + }); + }); +}