[js] Ensure dartify / jsify have inverse semantics.

CoreLibraryReviewExempt: Minor change to the internal of a web backend specific API.
Change-Id: I450e9410af7e006b853e2d8f26dc5ede1d95f4e4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/245621
Commit-Queue: Joshua Litt <joshualitt@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>

CHANGELOG.md

@@ -152,6 +152,7 @@
 
 - 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`.
 
 ### Tools
 

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

@@ -10,17 +10,12 @@ import 'dart:collection' show HashMap;
 import 'dart:async' show Completer;
 
 @patch
-dynamic jsify(Object object) {
-  if ((object is! Map) && (object is! Iterable)) {
-    throw ArgumentError("object must be a Map or Iterable");
-  }
-  return _convertDataTree(object);
-}
-
-Object _convertDataTree(Object data) {
+dynamic jsify(Object? object) {
   var _convertedObjects = HashMap.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)) {
       return _convertedObjects[o];
     }
@@ -41,7 +36,7 @@ Object _convertDataTree(Object data) {
     }
   }
 
-  return _convert(data)!;
+  return _convert(object);
 }
 
 @patch
@@ -513,11 +508,13 @@ DateTime _dateToDateTime(date) {
 @patch
 Object? dartify(Object? o) {
   var _convertedObjects = HashMap.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)) {
       return _convertedObjects[o];
     }
-    if (o == null || o is bool || o is num || o is String) return o;
 
     if (_isJavaScriptDate(o)) {
       return _dateToDateTime(o);

sdk/lib/js_util/js_util.dart

@@ -23,12 +23,13 @@ library dart.js_util;
 /// instead to use `@anonymous` `@JS()` annotated classes to create map-like
 /// objects for JS interop.
 ///
-/// The argument must be a [Map] or [Iterable], the contents of which are also
-/// deeply converted. Maps are converted into JavaScript objects. Iterables are
-/// converted into arrays. Strings, numbers, bools, and `@JS()` annotated
-/// objects are passed through unmodified. Dart objects are also passed through
-/// unmodified, but their members aren't usable from JavaScript.
-external dynamic jsify(Object object);
+/// If the argument are a [Map] or [Iterable], then they will be deeply
+/// converted.  Maps are converted into JavaScript objects. Iterables are
+/// converted into arrays. `@JS()` annotated objects are passed through
+/// unmodified. Dart objects are also passed through unmodified, but their
+/// members aren't usable from JavaScript.  The conversion logic for
+/// primitives(numbers, bools, and Strings) is backend specific.
+external dynamic jsify(Object? object);
 
 external Object get globalThis;
 

tests/lib/js/js_util/dartify_test.dart

@@ -17,7 +17,7 @@ external void eval(String code);
 
 main() {
   eval(r"""
-    globalThis.arrayData = [1, 2, false, 4, 'hello', 6, [1, 2], {'foo': 'bar'}];
+    globalThis.arrayData = [1, 2, false, 4, 'hello', 6, [1, 2], {'foo': 'bar'}, null];
     globalThis.recArrayData = [];
     globalThis.recArrayData = [globalThis.recArrayData];
     globalThis.objectData = {
@@ -26,6 +26,7 @@ main() {
       'c': {
         'a': true,
         'b': 'foo',
+        'c': null,
       },
     };
     globalThis.recObjectData = {};
@@ -52,7 +53,8 @@ main() {
       'hello',
       6,
       [1, 2],
-      {'foo': 'bar'}
+      {'foo': 'bar'},
+      null
     ];
     Expect.deepEquals(expectedValues, dartArray);
   });
@@ -73,6 +75,7 @@ main() {
       'c': {
         'a': true,
         'b': 'foo',
+        'c': null,
       },
     };
     Expect.deepEquals(expectedValues, dartObject);

tests/lib/js/js_util/jsify_test.dart

@@ -43,6 +43,15 @@ main() {
     }
     """);
 
+  test('convert primitives', () {
+    final cases = [null, true, 0.5, 'foo'];
+    for (int i = 0; i < cases.length; i++) {
+      final test = cases[i];
+      final jsTest = js_util.jsify(test);
+      expect(identical(test, jsTest), isTrue);
+    }
+  });
+
   test('convert a List', () {
     final list = [1, 2, 3, 4, 5, 6, 7, 8];
     var array = js_util.jsify(list);
@@ -97,8 +106,4 @@ main() {
 
     expect(js_util.getProperty(jsObject, 'e'), isNull);
   });
-
-  test('throws if object is not a Map or Iterable', () {
-    expect(() => js_util.jsify('a'), throwsArgumentError);
-  });
 }

tests/lib_2/js/js_util/dartify_test.dart

@@ -19,7 +19,7 @@ external void eval(String code);
 
 main() {
   eval(r"""
-    globalThis.arrayData = [1, 2, false, 4, 'hello', 6, [1, 2], {'foo': 'bar'}];
+    globalThis.arrayData = [1, 2, false, 4, 'hello', 6, [1, 2], {'foo': 'bar'}, null];
     globalThis.recArrayData = [];
     globalThis.recArrayData = [globalThis.recArrayData];
     globalThis.objectData = {
@@ -28,6 +28,7 @@ main() {
       'c': {
         'a': true,
         'b': 'foo',
+        'c': null,
       },
     };
     globalThis.recObjectData = {};
@@ -54,7 +55,8 @@ main() {
       'hello',
       6,
       [1, 2],
-      {'foo': 'bar'}
+      {'foo': 'bar'},
+      null
     ];
     Expect.deepEquals(expectedValues, dartArray);
   });
@@ -72,10 +74,7 @@ main() {
     Map<Object, Object> expectedValues = {
       'a': 1,
       'b': [1, 2, 3],
-      'c': {
-        'a': true,
-        'b': 'foo',
-      },
+      'c': {'a': true, 'b': 'foo', 'c': null},
     };
     Expect.deepEquals(expectedValues, dartObject);
   });

tests/lib_2/js/js_util/jsify_test.dart

@@ -45,6 +45,15 @@ main() {
     }
     """);
 
+  test('convert primitives', () {
+    final cases = [null, true, 0.5, 'foo'];
+    for (int i = 0; i < cases.length; i++) {
+      final test = cases[i];
+      final jsTest = js_util.jsify(test);
+      expect(identical(test, jsTest), isTrue);
+    }
+  });
+
   test('convert a List', () {
     final list = [1, 2, 3, 4, 5, 6, 7, 8];
     var array = js_util.jsify(list);
@@ -99,8 +108,4 @@ main() {
 
     expect(js_util.getProperty(jsObject, 'e'), isNull);
   });
-
-  test('throws if object is not a Map or Iterable', () {
-    expect(() => js_util.jsify('a'), throwsArgumentError);
-  });
 }