[js_interop] Make dartify / jsify a bit more efficient.

CoreLibraryReviewExempt: Minor refactor of the implementation of helpers in `js_util`.
Change-Id: I584bd4efbc8f4aff81f3bb62da9029ffdac3eb5f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/287669
Reviewed-by: Srujan Gaddam <[email protected]>
Commit-Queue: Joshua Litt <[email protected]>

Author: joshualitt

CHANGELOG.md

@@ -161,6 +161,8 @@
 - Added several helper functions to access more JavaScript operator, like
   `delete` and the `typeof` functionality.
 - `jsify` is now permissive and has inverse semantics to `dartify`.
+- `jsify` and `dartify` both handle types they understand natively more
+  efficiently.
 
 ### Tools
 

sdk/lib/_internal/js_shared/lib/js_interop_patch.dart

@@ -6,7 +6,6 @@ import 'dart:_foreign_helper' show JS;
 import 'dart:_internal' show patch;
 import 'dart:_js_types';
 import 'dart:js_util';
-import 'dart:js_util';
 import 'dart:typed_data';
 
 /// Helper for working with the [JSAny?] top type in a backend agnostic way.

sdk/lib/_internal/js_shared/lib/js_util_patch.dart

@@ -8,26 +8,49 @@ import 'dart:_js_helper'
     show convertDartClosureToJS, assertInterop, assertInteropArgs;
 import 'dart:collection' show HashMap;
 import 'dart:async' show Completer;
+import 'dart:typed_data';
+
+bool _noJsifyRequired(Object? o) =>
+    o == null ||
+    o is bool ||
+    o is num ||
+    o is String ||
+    o is Int8List ||
+    o is Uint8List ||
+    o is Uint8ClampedList ||
+    o is Int16List ||
+    o is Uint16List ||
+    o is Int32List ||
+    o is Uint32List ||
+    o is Float32List ||
+    o is Float64List ||
+    o is ByteBuffer ||
+    o is ByteData;
 
 @patch
 dynamic jsify(Object? object) {
-  var _convertedObjects = HashMap.identity();
+  if (_noJsifyRequired(object)) {
+    return object;
+  }
+  final _convertedObjects = HashMap<Object?, Object?>.identity();
 
   Object? _convert(Object? o) {
     // Fast path for primitives.
-    if (o == null || o is bool || o is num || o is String) return o;
+    if (_noJsifyRequired(o)) {
+      return o;
+    }
     if (_convertedObjects.containsKey(o)) {
       return _convertedObjects[o];
     }
-    if (o is Map) {
+    if (o is Map<Object?, Object?>) {
       final convertedMap = JS('=Object', '{}');
       _convertedObjects[o] = convertedMap;
       for (var key in o.keys) {
         JS('=Object', '#[#]=#', convertedMap, key, _convert(o[key]));
       }
       return convertedMap;
-    } else if (o is Iterable) {
-      var convertedList = [];
+    } else if (o is Iterable<Object?>) {
+      final convertedList = [];
       _convertedObjects[o] = convertedList;
       convertedList.addAll(o.map(_convert));
       return convertedList;
@@ -505,14 +528,54 @@ DateTime _dateToDateTime(date) {
   return new DateTime.fromMillisecondsSinceEpoch(millisSinceEpoch, isUtc: true);
 }
 
+bool _noDartifyRequired(Object? o) =>
+    o == null ||
+    JS(
+        'bool',
+        '''typeof # === 'boolean' ||
+                      typeof # === 'number' ||
+                      typeof # === 'string' ||
+                      # instanceof Int8Array ||
+                      # instanceof Uint8Array ||
+                      # instanceof Uint8ClampedArray ||
+                      # instanceof Int16Array ||
+                      # instanceof Uint16Array ||
+                      # instanceof Int32Array ||
+                      # instanceof Uint32Array ||
+                      # instanceof Float32Array ||
+                      # instanceof Float64Array ||
+                      # instanceof ArrayBuffer ||
+                      # instanceof DataView''',
+        o,
+        o,
+        o,
+        o,
+        o,
+        o,
+        o,
+        o,
+        o,
+        o,
+        o,
+        o,
+        o,
+        o);
+
 @patch
 Object? dartify(Object? o) {
-  var _convertedObjects = HashMap.identity();
+  if (_noDartifyRequired(o)) {
+    return o;
+  }
+
+  final _convertedObjects = HashMap<Object?, Object?>.identity();
 
   Object? convert(Object? o) {
     // Fast path for primitives.
-    if (o == null || o is bool || o is num || o is String) return o;
-    if (_convertedObjects.containsKey(o)) {
+    if (_noDartifyRequired(o)) {
+      return o;
+    }
+
+    if (_convertedObjects.containsKey(o!)) {
       return _convertedObjects[o];
     }
 
@@ -527,32 +590,32 @@ Object? dartify(Object? o) {
     }
 
     if (_isJavaScriptPromise(o)) {
-      return promiseToFuture(o);
+      return promiseToFuture<Object?>(o);
     }
 
     if (isJavaScriptSimpleObject(o)) {
-      Map<Object?, Object?> dartObject = {};
+      final dartObject = <Object?, Object?>{};
       _convertedObjects[o] = dartObject;
-      List<Object?> originalKeys = objectKeys(o);
-      List<Object?> dartKeys = [];
-      for (Object? key in originalKeys) {
+      final originalKeys = objectKeys(o);
+      final dartKeys = <Object?>[];
+      for (final key in originalKeys) {
         dartKeys.add(dartify(key));
       }
       for (int i = 0; i < originalKeys.length; i++) {
-        Object? jsKey = originalKeys[i];
-        Object? dartKey = dartKeys[i];
+        final jsKey = originalKeys[i];
+        final dartKey = dartKeys[i];
         if (jsKey != null) {
-          dartObject[dartKey] = convert(getProperty(o, jsKey));
+          dartObject[dartKey] = convert(getProperty<Object?>(o, jsKey));
         }
       }
       return dartObject;
     }
 
     if (isJavaScriptArray(o)) {
-      var l = JS<List>('returns:List;creates:;', '#', o);
-      List<Object?> dartObject = [];
+      final l = JS<List<Object?>>('returns:List;creates:;', '#', o);
+      final dartObject = <Object?>[];
       _convertedObjects[o] = dartObject;
-      int length = getProperty(o, 'length');
+      final length = getProperty<int>(o, 'length');
       for (int i = 0; i < length; i++) {
         dartObject.add(convert(l[i]));
       }

sdk/lib/_internal/wasm/lib/js_helper.dart

@@ -29,8 +29,8 @@ class JSValue {
   JSValue(this._ref);
 
   // This is currently only used in js_util.
-  // TODO(joshualitt): Investigate migrating `js_util` to js types. It should be
-  // non-breaking for js backends, and a tractable migration for wasm backends.
+  // TODO(joshualitt): Remove [box] and [unbox] once `JSNull` is boxed and users
+  // have been migrated over to the helpers in `dart:js_interop`.
   static JSValue? box(WasmExternRef? ref) =>
       isDartNull(ref) ? null : JSValue(ref);
 

sdk/lib/_internal/wasm/lib/js_util_patch.dart

@@ -15,8 +15,7 @@ import "dart:wasm";
 
 @patch
 dynamic jsify(Object? object) {
-  HashMap<Object?, Object?> convertedObjects =
-      HashMap<Object?, Object?>.identity();
+  final convertedObjects = HashMap<Object?, Object?>.identity();
   Object? convert(Object? o) {
     if (convertedObjects.containsKey(o)) {
       return convertedObjects[o];
@@ -25,7 +24,6 @@ dynamic jsify(Object? object) {
     if (o == null ||
         o is num ||
         o is bool ||
-        o is Function ||
         o is JSValue ||
         o is String ||
         o is Int8List ||
@@ -42,19 +40,19 @@ dynamic jsify(Object? object) {
       return JSValue(jsifyRaw(o));
     }
 
-    if (o is Map) {
-      JSValue convertedMap = newObject<JSValue>();
+    if (o is Map<Object?, Object?>) {
+      final convertedMap = newObject<JSValue>();
       convertedObjects[o] = convertedMap;
       for (final key in o.keys) {
-        JSValue? convertedKey = convert(key) as JSValue?;
+        final convertedKey = convert(key) as JSValue?;
         setPropertyRaw(convertedMap.toExternRef(), convertedKey?.toExternRef(),
             (convert(o[key]) as JSValue?)?.toExternRef());
       }
       return convertedMap;
-    } else if (o is Iterable) {
-      JSValue convertedIterable = _newArray();
+    } else if (o is Iterable<Object?>) {
+      final convertedIterable = _newArray();
       convertedObjects[o] = convertedIterable;
-      for (Object? item in o) {
+      for (final item in o) {
         callMethod(convertedIterable, 'push', [convert(item)]);
       }
       return convertedIterable;
@@ -188,8 +186,7 @@ List<Object?> objectKeys(Object? o) =>
 
 @patch
 Object? dartify(Object? object) {
-  HashMap<Object?, Object?> convertedObjects =
-      HashMap<Object?, Object?>.identity();
+  final convertedObjects = HashMap<Object?, Object?>.identity();
   Object? convert(Object? o) {
     if (convertedObjects.containsKey(o)) {
       return convertedObjects[o];
@@ -198,8 +195,8 @@ Object? dartify(Object? object) {
     // Because [List] needs to be shallowly converted across the interop
     // boundary, we have to double check for the case where a shallowly
     // converted [List] is passed back into [dartify].
-    if (o is List) {
-      List<Object?> converted = [];
+    if (o is List<Object?>) {
+      final converted = <Object?>[];
       for (final item in o) {
         converted.add(convert(item));
       }
@@ -236,22 +233,22 @@ Object? dartify(Object? object) {
     // TODO(joshualitt) handle Date and Promise.
 
     if (isJSSimpleObject(ref)) {
-      Map<Object?, Object?> dartMap = {};
+      final dartMap = <Object?, Object?>{};
       convertedObjects[o] = dartMap;
       // Keys will be a list of Dart [String]s.
-      List<Object?> keys = objectKeys(o);
+      final keys = objectKeys(o);
       for (int i = 0; i < keys.length; i++) {
-        Object? key = keys[i];
+        final key = keys[i];
         if (key != null) {
           dartMap[key] = convert(
               JSValue.box(getPropertyRaw(ref, (key as String).toExternRef())));
         }
       }
       return dartMap;
     } else if (isJSArray(ref)) {
-      List<Object?> dartList = [];
+      final dartList = <Object?>[];
       convertedObjects[o] = dartList;
-      int length = getProperty<double>(o, 'length').toInt();
+      final length = getProperty<double>(o, 'length').toInt();
       for (int i = 0; i < length; i++) {
         dartList.add(convert(JSValue.box(objectReadIndex(ref, i.toDouble()))));
       }

sdk/lib/js_interop/js_interop.dart

@@ -265,6 +265,11 @@ extension DoubleToJSNumber on double {
   external JSNumber get toJS;
 }
 
+/// [num] -> [JSNumber].
+extension NumToJSExtension on num {
+  JSNumber get toJS => DoubleToJSNumber(toDouble()).toJS;
+}
+
 /// [JSBoolean] <-> [bool]
 extension JSBooleanToBool on JSBoolean {
   external bool get toDart;