diff --git a/lib/react_client/js_interop_helpers.dart b/lib/react_client/js_interop_helpers.dart index 6bce2009..c76d4412 100644 --- a/lib/react_client/js_interop_helpers.dart +++ b/lib/react_client/js_interop_helpers.dart @@ -10,19 +10,32 @@ import "package:js/js.dart"; import 'js_backed_map.dart'; -/// Like [identityHashCode], but uses a different hash for JS objects to work around an issue where -/// [identityHashCode] adds an unwanted `$identityHash` property on JS objects (https://github.com/dart-lang/sdk/issues/47595). -int _jsObjectFriendlyIdentityHashCode(Object object) => - object is JsMap ? _jsObjectHashCode(object) : identityHashCode(object); +/// Like [identityHashCode], but uses a different hash for JS objects to work around: +/// - an issue where [identityHashCode] adds an unwanted `$identityHash` property on JS objects: https://github.com/dart-lang/sdk/issues/47595 +/// - an issue where [identityHashCode] throws for frozen objects: https://github.com/dart-lang/sdk/issues/36354 +int _jsObjectFriendlyIdentityHashCode(Object object) { + // Try detecting JS objects so we don't add properties to them. + // Workaround for https://github.com/dart-lang/sdk/issues/47595 + if (object is JsMap) return _anonymousJsObjectOrFrozenObjectHashCode(object); -/// A hashCode implementation for JS objects. + // If this fails, then most likely the object is a frozen JS object, such as props object or a variadic JSX children Array. + // Note that props objects are typically handled by the above is JsMap case, though. + // Fall back to a safe implementation. + try { + return identityHashCode(object); + } catch (_) { + return _anonymousJsObjectOrFrozenObjectHashCode(object); + } +} + +/// A hashCode implementation for anonymous JS objects or frozen objects. /// /// Even though the current implementation of returning the same hash code for all values is low-quality /// since all JS objects will collide, it is valid since it always returns the same value for the same object. /// -/// We also don't expect many JS objects to be passed into [jsifyAndAllowInterop], so the quality of this hash code -/// is not of much concern. -int _jsObjectHashCode(JsMap jsObject) => 0; +/// We also don't expect many JS objects or frozen objects to be passed into [jsifyAndAllowInterop], +/// so the quality of this hash code is not of much concern. +int _anonymousJsObjectOrFrozenObjectHashCode(Object _) => 0; // The following code is adapted from `package:js` in the dart-lang/sdk repo: // https://github.com/dart-lang/sdk/blob/2.2.0/sdk/lib/js_util/dart2js/js_util_dart2js.dart#L27 diff --git a/test/react_client/js_interop_helpers_test.dart b/test/react_client/js_interop_helpers_test.dart index 6f9eb24c..dafe4248 100644 --- a/test/react_client/js_interop_helpers_test.dart +++ b/test/react_client/js_interop_helpers_test.dart @@ -50,6 +50,9 @@ class Foo { external num bar(); } +@JS() +external dynamic createArray(); + main() { group('jsifyAndAllowInterop', () { test('converts a List', () { @@ -131,5 +134,82 @@ main() { expect(objectKeys(convertedNestedJsObject), expectedProperties, reason: 'JS object should not have any additional properties'); }); + + group('works as expected when the map contains frozen objects:', () { + test('anonymous object', () { + final frozenAnonymousObject = jsify({'foo': 'bar'}); + expect(_getPrototypeOf(frozenAnonymousObject), _objectPrototype, + reason: 'test setup check; should not extend from another JS type'); + + _freeze(frozenAnonymousObject); + expect(_isFrozen(frozenAnonymousObject), isTrue, reason: 'test setup check; should have frozen'); + + dynamic jsObject; + expect(() { + jsObject = jsifyAndAllowInterop({ + 'frozenObject': frozenAnonymousObject, + }); + }, returnsNormally, reason: 'should not throw when it encounters a frozen object'); + final convertedNestedFrozenObject = getProperty(jsObject, 'frozenObject'); + expect(convertedNestedFrozenObject, same(frozenAnonymousObject), + reason: 'JS object should have just gotten passed through'); + }); + + test('non-anonymous object', () { + final frozenNonAnynymousObject = Foo(21); + expect(_getPrototypeOf(frozenNonAnynymousObject), isNot(_objectPrototype), + reason: 'test setup check; should extend from another JS type'); + + _freeze(frozenNonAnynymousObject); + expect(_isFrozen(frozenNonAnynymousObject), isTrue, reason: 'test setup check; should have frozen'); + + dynamic jsObject; + expect(() { + jsObject = jsifyAndAllowInterop({ + 'frozenObject': frozenNonAnynymousObject, + }); + }, returnsNormally, reason: 'should not throw when it encounters a frozen object'); + final convertedNestedFrozenObject = getProperty(jsObject, 'frozenObject'); + expect(convertedNestedFrozenObject, same(frozenNonAnynymousObject), + reason: 'JS object should have just gotten passed through'); + }); + + test('array object constructed in JS', () { + // Create this on the JS side so that we're not starting out with Dart's list implementation + // and potentially any wrapper classes. + final frozenArray = createArray(); + expect(_getPrototypeOf(frozenArray), _arrayPrototype, reason: 'test setup check; should be an array'); + + _freeze(frozenArray); + expect(_isFrozen(frozenArray), isTrue, reason: 'test setup check; should have frozen'); + + dynamic jsObject; + expect(() { + jsObject = jsifyAndAllowInterop({ + 'frozenArray': frozenArray, + }); + }, returnsNormally, reason: 'should not throw when it encounters a frozen object'); + final convertedNestedFrozenObject = getProperty(jsObject, 'frozenArray'); + // Note that we don't expect it to be `same(frozenArray)` since the array passes the `is Iterable` check, + // and thus gets processed in _convertDataTree. + expect(convertedNestedFrozenObject, equals(frozenArray), + reason: 'JS object should have been converted properly passed through'); + }); + }); }); } + +@JS('Object.prototype') +external dynamic get _objectPrototype; + +@JS('Array.prototype') +external dynamic get _arrayPrototype; + +@JS('Object.freeze') +external void _freeze(Object object); + +@JS('Object.isFrozen') +external bool _isFrozen(Object object); + +@JS('Object.getPrototypeOf') +external dynamic _getPrototypeOf(Object object); diff --git a/test/react_client/js_interop_helpers_test.html b/test/react_client/js_interop_helpers_test.html index 36043862..97281d94 100644 --- a/test/react_client/js_interop_helpers_test.html +++ b/test/react_client/js_interop_helpers_test.html @@ -20,6 +20,10 @@ else return false; } + + function createArray() { + return [1, 2, 3]; + }