Skip to content

Commit ad36450

Browse files
Merge pull request #122 from clairesarsam-wf/ddc_tests
UIP-2414: Resolve ddc test failures
2 parents 2bebfb8 + 97d11a1 commit ad36450

File tree

4 files changed

+95
-6
lines changed

4 files changed

+95
-6
lines changed

lib/react_client.dart

+6-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import "package:react/react_dom.dart";
1818
import "package:react/react_dom_server.dart";
1919
import "package:react/src/react_client/synthetic_event_wrappers.dart" as events;
2020
import 'package:react/src/typedefs.dart';
21+
import 'package:react/src/ddc_emulated_function_name_bug.dart' as ddc_emulated_function_name_bug;
2122

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

@@ -356,7 +357,11 @@ class ReactDomComponentFactoryProxy extends ReactComponentFactoryProxy {
356357

357358
ReactDomComponentFactoryProxy(name) :
358359
this.name = name,
359-
this.factory = React.createFactory(name);
360+
this.factory = React.createFactory(name) {
361+
if (ddc_emulated_function_name_bug.isBugPresent) {
362+
ddc_emulated_function_name_bug.patchName(this);
363+
}
364+
}
360365

361366
@override
362367
String get type => name;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/// Provides detection and patching of the bug described in <https://github.com/dart-lang/sdk/issues/27647>,
2+
/// in which getters/setters with the identifier `name` don't work for emulated function classes, like [UiProps].
3+
@JS()
4+
library react.ddc_emulated_function_name_bug;
5+
6+
import 'package:js/js.dart';
7+
8+
/// Create a reduced test case of the issue, using an emulated function pattern that is similar to [UiProps].
9+
///
10+
/// We can't use [UiProps] itself, since it uses [isBugPresent], and that would cause a cyclic initialization error.
11+
class _NsmEmulatedFunctionWithNameProperty implements Function {
12+
void call();
13+
14+
@override
15+
noSuchMethod(i) {}
16+
17+
String _name;
18+
19+
// ignore: unnecessary_getters_setters
20+
String get name => _name;
21+
// ignore: unnecessary_getters_setters
22+
set name(String value) => _name = value;
23+
}
24+
25+
/// Whether this bug, <https://github.com/dart-lang/sdk/issues/27647>, is present in the current runtime.
26+
///
27+
/// This performs functional detection of the bug, and will be `true`
28+
/// only in the DDC and only in versions of the DDC where this bug is present.
29+
final bool isBugPresent = (() {
30+
const testValue = 'test value';
31+
32+
var testObject = new _NsmEmulatedFunctionWithNameProperty();
33+
34+
try {
35+
// In the DDC, this throws:
36+
// TypeError: Cannot assign to read only property 'name' of function 'function call(...args) {
37+
// return call.call.apply(call, args);
38+
// }'
39+
testObject.name = testValue;
40+
} catch(_) {
41+
return true;
42+
}
43+
44+
try {
45+
// We don't expect accessing this to throw, but just in case...
46+
return testObject.name != testValue;
47+
} catch(_) {
48+
return true;
49+
}
50+
})();
51+
52+
53+
@JS()
54+
@anonymous
55+
class _PropertyDescriptor {}
56+
57+
@JS('Object.getPrototypeOf')
58+
external dynamic _getPrototypeOf(dynamic object);
59+
60+
@JS('Object.getOwnPropertyDescriptor')
61+
external _PropertyDescriptor _getOwnPropertyDescriptor(dynamic object, String propertyName);
62+
63+
@JS('Object.defineProperty')
64+
external void _defineProperty(dynamic object, String propertyName, _PropertyDescriptor descriptor);
65+
66+
/// Patches the `name` property on the given [object] to have the expected behavior
67+
/// by copying the property descriptor for `name` from the appropriate prototype.
68+
///
69+
/// This is a noop if `name` is not a property on the given object.
70+
///
71+
/// __This functionality is unstable, and should not be used when [isBugPresent] is `false`.__
72+
///
73+
/// This method also had undefined behavior on non-[UiProps] instances.
74+
void patchName(dynamic object) {
75+
var current = object;
76+
while ((current = _getPrototypeOf(current)) != null) {
77+
var nameDescriptor = _getOwnPropertyDescriptor(current, 'name');
78+
79+
if (nameDescriptor != null) {
80+
_defineProperty(object, 'name', nameDescriptor);
81+
return;
82+
}
83+
}
84+
}

test/factory/common_factory_tests.dart

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ void commonFactoryTests(Function factory) {
3333
test('multiple arguments', () {
3434
var instance = factory({}, 'one', 'two');
3535
expect(getJsChildren(instance), equals(['one', 'two']));
36-
});
36+
}, tags: 'ddcFailure'); // This test cannot be run using ddc until https://github.com/dart-lang/sdk/issues/29904 is resolved
3737

3838
test('a List', () {
3939
var instance = factory({}, ['one', 'two',]);
@@ -155,7 +155,7 @@ void _childKeyWarningTests(Function factory) {
155155
);
156156

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

160160
test('when rendering custom Dart components', () {
161161
_renderWithUniqueOwnerName(() =>

test/lifecycle_test.dart

+3-3
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ external List getUpdatingSetStateLifeCycleCalls();
427427
@JS()
428428
external List getNonUpdatingSetStateLifeCycleCalls();
429429

430-
ReactDartComponentFactoryProxy<_SetStateTest> SetStateTest = react.registerComponent(() => new _SetStateTest()) as ReactDartComponentFactoryProxy<_SetStateTest>;
430+
ReactDartComponentFactoryProxy SetStateTest = react.registerComponent(() => new _SetStateTest());
431431
class _SetStateTest extends react.Component {
432432
@override
433433
Map getDefaultProps() => {'shouldUpdate': true};
@@ -502,7 +502,7 @@ class _DefaultPropsCachingTest extends react.Component {
502502
render() => false;
503503
}
504504

505-
ReactDartComponentFactoryProxy<_DefaultPropsTest> DefaultPropsTest = react.registerComponent(() => new _DefaultPropsTest()) as ReactDartComponentFactoryProxy<_DefaultPropsTest>;
505+
ReactDartComponentFactoryProxy DefaultPropsTest = react.registerComponent(() => new _DefaultPropsTest());
506506
class _DefaultPropsTest extends react.Component {
507507
static int getDefaultPropsCallCount = 0;
508508

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

516-
ReactDartComponentFactoryProxy<_LifecycleTest> LifecycleTest = react.registerComponent(() => new _LifecycleTest()) as ReactDartComponentFactoryProxy<_LifecycleTest>;
516+
ReactDartComponentFactoryProxy LifecycleTest = react.registerComponent(() => new _LifecycleTest());
517517
class _LifecycleTest extends react.Component {
518518
List lifecycleCalls = [];
519519

0 commit comments

Comments
 (0)