Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIP-2414: Resolve ddc test failures #122

Merged
merged 2 commits into from
Jun 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/react_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import "package:react/react_dom.dart";
import "package:react/react_dom_server.dart";
import "package:react/src/react_client/synthetic_event_wrappers.dart" as events;
import 'package:react/src/typedefs.dart';
import 'package:react/src/ddc_emulated_function_name_bug.dart' as ddc_emulated_function_name_bug;

export 'package:react/react_client/react_interop.dart' show ReactElement, ReactJsComponentFactory;

Expand Down Expand Up @@ -356,7 +357,11 @@ class ReactDomComponentFactoryProxy extends ReactComponentFactoryProxy {

ReactDomComponentFactoryProxy(name) :
this.name = name,
this.factory = React.createFactory(name);
this.factory = React.createFactory(name) {
if (ddc_emulated_function_name_bug.isBugPresent) {
ddc_emulated_function_name_bug.patchName(this);
}
}

@override
String get type => name;
Expand Down
84 changes: 84 additions & 0 deletions lib/src/ddc_emulated_function_name_bug.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/// Provides detection and patching of the bug described in <https://github.com/dart-lang/sdk/issues/27647>,
/// in which getters/setters with the identifier `name` don't work for emulated function classes, like [UiProps].
@JS()
library react.ddc_emulated_function_name_bug;

import 'package:js/js.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, <https://github.com/dart-lang/sdk/issues/27647>, 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;
}
}
}
4 changes: 2 additions & 2 deletions test/factory/common_factory_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ void commonFactoryTests(Function factory) {
test('multiple arguments', () {
var instance = factory({}, 'one', 'two');
expect(getJsChildren(instance), equals(['one', 'two']));
});
}, tags: 'ddcFailure'); // This test cannot be run using ddc until https://github.com/dart-lang/sdk/issues/29904 is resolved

test('a List', () {
var instance = factory({}, ['one', 'two',]);
Expand Down Expand Up @@ -155,7 +155,7 @@ void _childKeyWarningTests(Function factory) {
);

expect(consoleErrorCalled, isFalse, reason: 'should not have outputted a warning');
});
}, tags: 'ddcFailure'); // This test cannot be run using ddc until https://github.com/dart-lang/sdk/issues/29904 is resolved

test('when rendering custom Dart components', () {
_renderWithUniqueOwnerName(() =>
Expand Down
6 changes: 3 additions & 3 deletions test/lifecycle_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ external List getUpdatingSetStateLifeCycleCalls();
@JS()
external List getNonUpdatingSetStateLifeCycleCalls();

ReactDartComponentFactoryProxy<_SetStateTest> SetStateTest = react.registerComponent(() => new _SetStateTest()) as ReactDartComponentFactoryProxy<_SetStateTest>;
ReactDartComponentFactoryProxy SetStateTest = react.registerComponent(() => new _SetStateTest());
class _SetStateTest extends react.Component {
@override
Map getDefaultProps() => {'shouldUpdate': true};
Expand Down Expand Up @@ -502,7 +502,7 @@ class _DefaultPropsCachingTest extends react.Component {
render() => false;
}

ReactDartComponentFactoryProxy<_DefaultPropsTest> DefaultPropsTest = react.registerComponent(() => new _DefaultPropsTest()) as ReactDartComponentFactoryProxy<_DefaultPropsTest>;
ReactDartComponentFactoryProxy DefaultPropsTest = react.registerComponent(() => new _DefaultPropsTest());
class _DefaultPropsTest extends react.Component {
static int getDefaultPropsCallCount = 0;

Expand All @@ -513,7 +513,7 @@ class _DefaultPropsTest extends react.Component {
render() => false;
}

ReactDartComponentFactoryProxy<_LifecycleTest> LifecycleTest = react.registerComponent(() => new _LifecycleTest()) as ReactDartComponentFactoryProxy<_LifecycleTest>;
ReactDartComponentFactoryProxy LifecycleTest = react.registerComponent(() => new _LifecycleTest());
class _LifecycleTest extends react.Component {
List lifecycleCalls = [];

Expand Down