Reland "[dart:js_interop] Remove Object.toJS and JSNumber.toDart"

This is a reland of commit 16fcfe7eae

Original change's description:
> [dart:js_interop] Remove Object.toJS and JSNumber.toDart
>
> Modifies JSBoxedDartObject reified type on JS backends and also
> modifies JSBoxedDartObject.toDart now that a proper box is introduced.
> Also uses a JS symbol in JSBoxedDartObject.toDart for a property
> check so that different Dart apps don't accidentally share Dart
> objects. It's now an error to call this function on non-boxed objects.
>
> Fixes a type issue where creating a new object literal with the JS
> foreign function was resulting in dart2js thinking toJSBox would
> always throw. Changing the typeDescription to PlainJavaScriptObject
> instead of =Object fixes that issue.
>
> CoreLibraryReviewExempt: Backend-specific library.
> Change-Id: I5cfb1f32ff4328fafdf9831b0d8da806c39391d9
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/309082
> Reviewed-by: Joshua Litt <joshualitt@google.com>
> Commit-Queue: Srujan Gaddam <srujzs@google.com>

CoreLibraryReviewExempt: Reland.
Change-Id: If6b190f12bdf840b0259c5739f50d9bdcd27fd47
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/313600
Reviewed-by: Joshua Litt <joshualitt@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>

CHANGELOG.md

@@ -1,3 +1,22 @@
+## 3.2.0
+
+### Language
+
+### Libraries
+
+#### `dart:js_interop`
+
+- **JSNumber.toDart and Object.toJS**:
+  `JSNumber.toDart` is removed in favor of `toDartDouble` and `toDartInt` to
+  make the type explicit. `Object.toJS` is also removed in favor of
+  `Object.toJSBox`. Previously, this function would allow Dart objects to flow
+  into JS unwrapped on the JS backends. Now, there's an explicit wrapper that is
+  added and unwrapped via `JSBoxedDartObject.toDart`. Similarly,
+  `JSExportedDartObject` is renamed to `JSBoxedDartObject` and the extensions
+  `ObjectToJSExportedDartObject` and `JSExportedDartObjectToObject` are renamed
+  to `ObjectToJSBoxedDartObject` and `JSBoxedDartObjectToObject` in order to
+  avoid confusion with `@JSExport`.
+
 ## 3.1.0
 
 ### Language

pkg/_js_interop_checks/lib/src/transformations/static_interop_class_eraser.dart

@@ -46,9 +46,8 @@ InterfaceType eraseStaticInteropTypesForJSCompilers(
         typeArguments = [coreTypes.objectNullableRawType];
         break;
       case 'JSBoxedDartObject':
-        // TODO(srujzs): This should be JavaScriptObject once we migrate Flutter
+        erasedClass =
-        // off of using `Object.toJS`.
+            coreTypes.index.getClass('dart:_interceptors', 'JSObject');
-        erasedClass = coreTypes.objectClass;
         break;
       case 'JSArrayBuffer':
         erasedClass = coreTypes.index.getClass('dart:typed_data', 'ByteBuffer');

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

@@ -72,7 +72,13 @@ extension FunctionToJSExportedDartFunction on Function {
       js_util.allowInterop(this) as JSExportedDartFunction;
 }
 
-const _jsBoxedDartObjectProperty = "'_\$jsBoxedDartObject'";
+/// Embedded global property for wrapped Dart objects passed via JS interop.
+///
+/// This is a Symbol so that different Dart applications don't share Dart
+/// objects from different Dart runtimes. We expect all [JSBoxedDartObject]s to
+/// have this Symbol.
+final Object _jsBoxedDartObjectProperty =
+    foreign_helper.JS('', 'Symbol("jsBoxedDartObjectProperty")');
 
 /// [JSBoxedDartObject] <-> [Object]
 @patch
@@ -80,37 +86,25 @@ extension JSBoxedDartObjectToObject on JSBoxedDartObject {
   @patch
   @pragma('dart2js:prefer-inline')
   Object get toDart {
-    if (this is JavaScriptObject) {
+    final val = js_util.getProperty(this, _jsBoxedDartObjectProperty);
-      final val = foreign_helper.JS(
+    if (val == null) {
-          'Object|Null', '#[$_jsBoxedDartObjectProperty]', this);
+      throw 'Expected a wrapped Dart object, but got a JS object or a wrapped '
-      if (val == null) {
+          'Dart object from a separate runtime instead.';
-        throw 'Expected a wrapped Dart object, but got a JS object instead.';
-      }
-      return val as Object;
     }
-    // TODO(srujzs): Currently we have to still support Dart objects being
+    return val as Object;
-    // returned from JS until `Object.toJS` is removed. Once that is removed,
-    // and the runtime type of this type is changed, we can get rid of this and
-    // the type check above.
-    return this;
   }
 }
 
 @patch
 extension ObjectToJSBoxedDartObject on Object {
-  // TODO(srujzs): Remove.
-  @patch
-  @pragma('dart2js:prefer-inline')
-  JSBoxedDartObject get toJS => this as JSBoxedDartObject;
-
   @patch
   @pragma('dart2js:prefer-inline')
   JSBoxedDartObject get toJSBox {
     if (this is JavaScriptObject) {
       throw 'Attempting to box non-Dart object.';
     }
-    final box =
+    final box = js_util.newObject();
-        foreign_helper.JS('=Object', '{$_jsBoxedDartObjectProperty: #}', this);
+    js_util.setProperty(box, _jsBoxedDartObjectProperty, this);
     return box as JSBoxedDartObject;
   }
 }
@@ -306,11 +300,6 @@ extension ListToJSArray on List<JSAny?> {
 /// [JSNumber] -> [double] or [int].
 @patch
 extension JSNumberToNumber on JSNumber {
-  // TODO(srujzs): Remove.
-  @patch
-  @pragma('dart2js:prefer-inline')
-  double get toDart => this as double;
-
   @patch
   @pragma('dart2js:prefer-inline')
   double get toDartDouble => this as double;

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

@@ -67,7 +67,7 @@ dynamic jsify(Object? object) {
 Object get globalThis => JS('', 'globalThis');
 
 @patch
-T newObject<T>() => JS('=Object', '{}');
+T newObject<T>() => JS('PlainJavaScriptObject', '{}');
 
 @patch
 bool hasProperty(Object o, Object name) => JS('bool', '# in #', name, o);

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

@@ -112,7 +112,7 @@ class JSArrayIteratorAdapter<T> implements Iterator<T> {
   @override
   bool moveNext() {
     index++;
-    int length = array.length.toDart.toInt();
+    int length = array.length.toDartInt;
     if (index > length) {
       throw 'Iterator out of bounds';
     }
@@ -134,7 +134,7 @@ class JSArrayIterableAdapter<T> extends EfficientLengthIterable<T> {
   Iterator<T> get iterator => JSArrayIteratorAdapter<T>(array);
 
   @override
-  int get length => array.length.toDart.toInt();
+  int get length => array.length.toDartInt;
 }
 
 // Convert to double to avoid converting to [BigInt] in the case of int64.

sdk/lib/_internal/wasm/lib/js_interop_patch.dart

@@ -88,11 +88,6 @@ extension JSBoxedDartObjectToObject on JSBoxedDartObject {
 
 @patch
 extension ObjectToJSBoxedDartObject on Object {
-  // TODO(srujzs): Remove.
-  @patch
-  JSBoxedDartObject get toJS =>
-      _box<JSBoxedDartObject>(jsObjectFromDartObject(this));
-
   @patch
   JSBoxedDartObject get toJSBox {
     if (this is JSValue) {
@@ -348,10 +343,6 @@ extension ListToJSArray on List<JSAny?> {
 /// [JSNumber] -> [double] or [int].
 @patch
 extension JSNumberToNumber on JSNumber {
-  // TODO(srujzs): Remove.
-  @patch
-  double get toDart => toDartDouble;
-
   @patch
   double get toDartDouble => toDartNumber(toExternRef);
 

sdk/lib/_internal/wasm/lib/regexp_helper.dart

@@ -176,12 +176,12 @@ class _MatchImplementation implements RegExpMatch {
 
   String get input => _match.input.toDart;
 
-  int get start => _match.index.toDart.toInt();
+  int get start => _match.index.toDartInt;
 
   int get end => (start + (_match[0.toJS].toString()).length);
 
   String? group(int index) {
-    if (index < 0 || index >= _match.length.toDart.toInt()) {
+    if (index < 0 || index >= _match.length.toDartInt) {
       throw RangeError("Index $index is out of range ${_match.length}");
     }
     return _match[index.toJS]?.toString();
@@ -189,7 +189,7 @@ class _MatchImplementation implements RegExpMatch {
 
   String? operator [](int index) => group(index);
 
-  int get groupCount => _match.length.toDart.toInt() - 1;
+  int get groupCount => _match.length.toDartInt - 1;
 
   List<String?> groups(List<int> groups) {
     List<String?> out = [];
@@ -311,7 +311,7 @@ int regExpCaptureCount(JSSyntaxRegExp regexp) {
   final match = nativeAnchoredRegExp.exec(''.toJS)!;
   // The native-anchored regexp always have one capture more than the original,
   // and always matches the empty string.
-  return match.length.toDart.toInt() - 2;
+  return match.length.toDartInt - 2;
 }
 
 /// Find the first match of [regExp] in [string] at or after [start].

sdk/lib/js_interop/js_interop.dart

@@ -197,9 +197,6 @@ extension JSBoxedDartObjectToObject on JSBoxedDartObject {
 }
 
 extension ObjectToJSBoxedDartObject on Object {
-  // TODO(srujzs): Remove. Prefer toJSBox.
-  external JSBoxedDartObject get toJS;
-
   external JSBoxedDartObject get toJSBox;
 }
 
@@ -321,9 +318,6 @@ extension ListToJSArray on List<JSAny?> {
 
 /// [JSNumber] -> [double] or [int].
 extension JSNumberToNumber on JSNumber {
-  // TODO(srujzs): Remove. Prefer toDartDouble or toDartInt.
-  external double get toDart;
-
   /// Returns a Dart [double] for the given [JSNumber].
   external double get toDartDouble;
 

tests/lib/js/static_interop_test/js_array_impl_test.dart

@@ -41,7 +41,7 @@ String jsString(String s) => s.toJS.toDart;
 
 extension ListJSAnyExtension on List<JSAny?> {
   List<double?> get toListDouble =>
-      this.map((a) => (a as JSNumber?)?.toDart).toList();
+      this.map((a) => (a as JSNumber?)?.toDartDouble).toList();
 }
 
 extension ListNumExtension on List<num?> {
@@ -55,11 +55,11 @@ extension ListStringExtension on List<String?> {
 }
 
 extension NullableJSAnyExtension on JSAny? {
-  double? get toDouble => (this as JSNumber?)?.toDart;
+  double? get toDouble => (this as JSNumber?)?.toDartDouble;
 }
 
 extension JSAnyExtension on JSAny {
-  double get toDouble => (this as JSNumber).toDart;
+  double get toDouble => (this as JSNumber).toDartDouble;
 }
 
 void modedTests(TestMode mode) {
@@ -345,7 +345,7 @@ void nonModedTests() {
       (list.reduce((a, b) =>
                   ((a as JSNumber).toDouble + (b as JSNumber).toDouble).toJS)
               as JSNumber)
-          .toDart);
+          .toDartDouble);
 
   // fold
   Expect.equals(
@@ -353,32 +353,34 @@ void nonModedTests() {
 
   // firstWhere
   Expect.equals(
-      1, list.firstWhere((a) => (a as JSNumber).toDart == 1).toDouble);
+      1, list.firstWhere((a) => (a as JSNumber).toDartDouble == 1).toDouble);
   Expect.equals(
       45,
       list
-          .firstWhere((a) => (a as JSNumber).toDart == 4, orElse: () => 45.toJS)
+          .firstWhere((a) => (a as JSNumber).toDartDouble == 4,
+              orElse: () => 45.toJS)
           .toDouble);
 
   // lastWhere
   list = [1, 2, 3, 4].toJSListJSAny;
   Expect.equals(
-      4, list.lastWhere((a) => (a as JSNumber).toDart % 2 == 0).toDouble);
+      4, list.lastWhere((a) => (a as JSNumber).toDartDouble % 2 == 0).toDouble);
   Expect.equals(
       45,
       list
-          .lastWhere((a) => (a as JSNumber).toDart == 5, orElse: () => 45.toJS)
+          .lastWhere((a) => (a as JSNumber).toDartDouble == 5,
+              orElse: () => 45.toJS)
           .toDouble);
 
   // singleWhere
   Expect.equals(
-      1, list.singleWhere((a) => (a as JSNumber).toDart == 1).toDouble);
+      1, list.singleWhere((a) => (a as JSNumber).toDartDouble == 1).toDouble);
   Expect.throwsStateError(
-      () => list.singleWhere((a) => (a as JSNumber).toDart % 2 == 0));
+      () => list.singleWhere((a) => (a as JSNumber).toDartDouble % 2 == 0));
   Expect.equals(
       45,
       list
-          .singleWhere((a) => (a as JSNumber).toDart == 5,
+          .singleWhere((a) => (a as JSNumber).toDartDouble == 5,
               orElse: () => 45.toJS)
           .toDouble);
 

tests/lib/js/static_interop_test/js_types_test.dart

@@ -138,6 +138,8 @@ void syncTests() {
   edo = DartObject().toJSBox;
   expect(edo is JSBoxedDartObject, true);
   expect(((edo as JSBoxedDartObject).toDart as DartObject).foo, 'bar');
+  // Should not box a non Dart-object.
+  Expect.throws(() => edo.toJSBox);
 
   // [JSArray] <-> [List<JSAny?>]
   arr = [1.0.toJS, 'foo'.toJS].toJS;