[dart:js_interop] Add toJSProxyOrRef
Adds a conversion function on List<JSAny?> so that users can
modify the original list and have those changes carry forward
to the array. Creates a proxy object for dart2wasm when the
list is not a JSArrayImpl. This proxy uses handler methods and
the fact that Array methods are generic to provide an Array
interface. Also fixes a small issue in the exporting so that
toJS'd function types have a valid return type.
CoreLibraryReviewExempt: Backend-specific library.
Change-Id: I00f453aa82dd19f2913820579eb2675b799288d3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/323446
Commit-Queue: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
diff --git a/pkg/_js_interop_checks/lib/src/transformations/export_creator.dart b/pkg/_js_interop_checks/lib/src/transformations/export_creator.dart
index 6ebba4e..05adf8e 100644
--- a/pkg/_js_interop_checks/lib/src/transformations/export_creator.dart
+++ b/pkg/_js_interop_checks/lib/src/transformations/export_creator.dart
@@ -304,20 +304,19 @@
var getter = getSet.getter;
var setter = getSet.setter;
if (getter != null) {
+ final resultType = _staticInteropMockValidator.typeParameterResolver
+ .resolve(getter.getterType);
block.add(setProperty(
VariableGet(getSetMap),
'get',
StaticInvocation(
_functionToJS,
Arguments([
- FunctionExpression(FunctionNode(ReturnStatement(InstanceGet(
- InstanceAccessKind.Instance,
- VariableGet(dartInstance),
- getter.name,
- interfaceTarget: getter,
- resultType: _staticInteropMockValidator
- .typeParameterResolver
- .resolve(getter.getterType)))))
+ FunctionExpression(FunctionNode(
+ ReturnStatement(InstanceGet(InstanceAccessKind.Instance,
+ VariableGet(dartInstance), getter.name,
+ interfaceTarget: getter, resultType: resultType)),
+ returnType: resultType))
]))));
}
if (setter != null) {
@@ -340,7 +339,8 @@
setter.name,
VariableGet(setterParameter),
interfaceTarget: setter)),
- positionalParameters: [setterParameter]))
+ positionalParameters: [setterParameter],
+ returnType: const VoidType()))
]))));
}
// Call `Object.defineProperty` to define the export name with the
diff --git a/sdk/lib/_internal/js_shared/lib/js_interop_patch.dart b/sdk/lib/_internal/js_shared/lib/js_interop_patch.dart
index a2afcad..b5f6883 100644
--- a/sdk/lib/_internal/js_shared/lib/js_interop_patch.dart
+++ b/sdk/lib/_internal/js_shared/lib/js_interop_patch.dart
@@ -298,6 +298,18 @@
@patch
@pragma('dart2js:prefer-inline')
JSArray get toJS => this as JSArray;
+
+ // TODO(srujzs): Should we do a check to make sure this List is a JSArray
+ // under the hood and then potentially proxy? This applies for user lists. For
+ // now, don't do the check to avoid the cost of the check in the general case,
+ // and user lists will likely crash. Note that on dart2js, we do an
+ // `Array.isArray` check instead of `instanceof Array` when we cast to a
+ // `List`, which is what `JSArray` is. This won't work for proxy objects as
+ // they're not actually Arrays, so the cast will fail unless we change that
+ // check.
+ @patch
+ @pragma('dart2js:prefer-inline')
+ JSArray get toJSProxyOrRef => this as JSArray;
}
/// [JSNumber] -> [double] or [int].
diff --git a/sdk/lib/_internal/wasm/lib/js_interop_patch.dart b/sdk/lib/_internal/wasm/lib/js_interop_patch.dart
index 044ee37..be9cf75 100644
--- a/sdk/lib/_internal/wasm/lib/js_interop_patch.dart
+++ b/sdk/lib/_internal/wasm/lib/js_interop_patch.dart
@@ -9,6 +9,7 @@
import 'dart:_wasm';
import 'dart:async' show Completer;
import 'dart:js_interop';
+import 'dart:js_interop_unsafe' as unsafe;
import 'dart:js_util' as js_util;
import 'dart:typed_data';
@@ -330,13 +331,18 @@
@patch
extension ListToJSArray on List<JSAny?> {
- @patch
- JSArray get toJS {
+ JSArray? get _underlyingArray {
final t = this;
return t is js_types.JSArrayImpl
? JSValue.boxT<JSArray>(t.toExternRef)
- : toJSArray(this);
+ : null;
}
+
+ @patch
+ JSArray get toJS => _underlyingArray ?? toJSArray(this);
+
+ @patch
+ JSArray get toJSProxyOrRef => _underlyingArray ?? _createJSProxyOfList(this);
}
/// [JSNumber] -> [double] or [int].
@@ -392,3 +398,153 @@
t is js_types.JSStringImpl ? t.toExternRef : jsStringFromDartString(t));
}
}
+
+@JS('Array')
+@staticInterop
+class _Array {
+ external static JSObject get prototype;
+}
+
+@JS('Symbol')
+@staticInterop
+class _Symbol {
+ external static JSSymbol get isConcatSpreadable;
+}
+
+// Used only so we can use `createStaticInteropMock`'s prototype-setting.
+@JS()
+@staticInterop
+class __ListBackedJSArray {}
+
+/// Implementation of indexing, `length`, and core handler methods.
+///
+/// JavaScript's `Array` methods are similar to Dart's `ListMixin`, because they
+/// only rely on the implementation of `length` and indexing methods (and
+/// support for any JS operators like `in` or `delete`).
+/// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array#generic_array_methods
+class _ListBackedJSArray {
+ final List<JSAny?> _list;
+ // The proxy that wraps this list.
+ late final JSArray proxy;
+
+ _ListBackedJSArray(this._list);
+
+ @JSExport()
+ int get length => _list.length;
+
+ // TODO(srujzs): Resizing the list populates the list with `null`. Should we
+ // instead populate it with `undefined` as JS does?
+ @JSExport()
+ void set length(int val) => _list.length = val;
+
+ // []
+ @JSExport()
+ JSAny? _getIndex(int index) => _list[index];
+
+ // []=
+ @JSExport()
+ void _setIndex(int index, JSAny? value) {
+ // Need to resize the array if out of bounds.
+ if (index >= length) length = index + 1;
+ _list[index] = value;
+ }
+
+ // in
+ @JSExport()
+ bool _hasIndex(int index) => index >= 0 && index < length;
+
+ // delete
+ @JSExport()
+ bool _deleteIndex(int index) {
+ if (_hasIndex(index)) {
+ _list.removeAt(index);
+ return true;
+ }
+ return false;
+ }
+}
+
+JSArray _createJSProxyOfList(List<JSAny?> list) {
+ final wrapper = _ListBackedJSArray(list);
+ final jsExportWrapper =
+ js_util.createStaticInteropMock<__ListBackedJSArray, _ListBackedJSArray>(
+ wrapper, _Array.prototype) as JSObject;
+
+ // Needed for `concat` to spread the contents of the current array instead of
+ // prepending.
+ // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/isConcatSpreadable
+ unsafe.JSObjectUtilExtension(jsExportWrapper)[_Symbol.isConcatSpreadable] =
+ true.toJS;
+
+ final getIndex =
+ (unsafe.JSObjectUtilExtension(jsExportWrapper)['_getIndex'.toJS]
+ as JSFunction)
+ .toExternRef;
+ final setIndex =
+ (unsafe.JSObjectUtilExtension(jsExportWrapper)['_setIndex'.toJS]
+ as JSFunction)
+ .toExternRef;
+ final hasIndex =
+ (unsafe.JSObjectUtilExtension(jsExportWrapper)['_hasIndex'.toJS]
+ as JSFunction)
+ .toExternRef;
+ final deleteIndex =
+ (unsafe.JSObjectUtilExtension(jsExportWrapper)['_deleteIndex'.toJS]
+ as JSFunction)
+ .toExternRef;
+
+ final proxy = _box<JSArray>(js_helper.JS<WasmExternRef?>('''
+ (wrapper, getIndex, setIndex, hasIndex, deleteIndex) => new Proxy(wrapper, {
+ 'get': function (target, prop, receiver) {
+ if (typeof prop == 'string') {
+ const numProp = Number(prop);
+ if (Number.isInteger(numProp)) {
+ const args = new Array();
+ args.push(numProp);
+ return Reflect.apply(getIndex, wrapper, args);
+ }
+ }
+ return Reflect.get(target, prop, receiver);
+ },
+ 'set': function (target, prop, value, receiver) {
+ if (typeof prop == 'string') {
+ const numProp = Number(prop);
+ if (Number.isInteger(numProp)) {
+ const args = new Array();
+ args.push(numProp, value);
+ Reflect.apply(setIndex, wrapper, args);
+ return true;
+ }
+ }
+ // Note that handler set is required to return a bool (whether it
+ // succeeded or not), so `[]=` won't return the value set.
+ return Reflect.set(target, prop, value, receiver);
+ },
+ 'has': function (target, prop) {
+ if (typeof prop == 'string') {
+ const numProp = Number(prop);
+ if (Number.isInteger(numProp)) {
+ const args = new Array();
+ args.push(numProp);
+ // Array-like objects are assumed to have indices as properties.
+ return Reflect.apply(hasIndex, wrapper, args);
+ }
+ }
+ return Reflect.has(target, prop);
+ },
+ 'deleteProperty': function (target, prop) {
+ if (typeof prop == 'string') {
+ const numProp = Number(prop);
+ if (Number.isInteger(numProp)) {
+ const args = new Array();
+ args.push(numProp);
+ return Reflect.apply(deleteIndex, wrapper, args);
+ }
+ }
+ return Reflect.deleteProperty(target, prop);
+ }
+ })''', jsExportWrapper.toExternRef, getIndex, setIndex, hasIndex,
+ deleteIndex));
+ wrapper.proxy = proxy;
+ return proxy;
+}
diff --git a/sdk/lib/js_interop/js_interop.dart b/sdk/lib/js_interop/js_interop.dart
index 9413d88..1d959ab 100644
--- a/sdk/lib/js_interop/js_interop.dart
+++ b/sdk/lib/js_interop/js_interop.dart
@@ -316,11 +316,27 @@
/// [JSArray] <-> [List]
extension JSArrayToList on JSArray {
+ /// Returns a list wrapper of the JS array.
external List<JSAny?> get toDart;
}
extension ListToJSArray on List<JSAny?> {
+ /// Compiler-specific conversion from list to JS array.
+ ///
+ /// This is either a pass-by-reference, unwrap, or copy depending on the
+ /// implementation of the given list, and users shouldn't rely on
+ /// modifications to the list to affect the array or vice versa.
external JSArray get toJS;
+
+ /// Either passes by reference, unwraps, or creates a heavyweight proxy that
+ /// wraps the list.
+ ///
+ /// Only use this member if you want modifications to the list to also affect
+ /// the JS array and vice versa. In practice, dart2js and DDC will pass lists
+ /// by reference and dart2wasm will add a proxy or unwrap for most lists.
+ ///
+ /// **WARNING**: Do not rely on this to be performant.
+ external JSArray get toJSProxyOrRef;
}
/// [JSNumber] -> [double] or [int].
diff --git a/tests/lib/js/static_interop_test/js_array_impl_test.dart b/tests/lib/js/static_interop_test/js_array_impl_test.dart
index 0d2ac27..73f2ce3 100644
--- a/tests/lib/js/static_interop_test/js_array_impl_test.dart
+++ b/tests/lib/js/static_interop_test/js_array_impl_test.dart
@@ -35,7 +35,15 @@
(mode == TestMode.jsArgument ||
mode == TestMode.jsReceiverAndArguments));
-List<JSAny?> jsList(List<JSAny?> l) => l.toJS.toDart;
+late bool testProxiedArray;
+
+// We test two types of round-trips from Dart to JS to Dart:
+// - A copy that `toJS` creates that then gets wrapped by JSArrayImpl
+// - A proxy that `toJSProxyOrRef` creates that then gets wrapped by JSArrayImpl
+List<JSAny?> jsList(List<JSAny?> l) {
+ final arr = testProxiedArray ? l.toJS : l.toJSProxyOrRef;
+ return arr.toDart;
+}
String jsString(String s) => s.toJS.toDart;
@@ -427,7 +435,7 @@
Expect.isTrue(list.contains(4.toJS));
}
-void main() {
+void runAllTests() {
for (final mode in [
TestMode.jsReceiver,
TestMode.jsArgument,
@@ -438,6 +446,13 @@
nonModedTests();
}
+void main() {
+ testProxiedArray = false;
+ runAllTests();
+ testProxiedArray = true;
+ runAllTests();
+}
+
// Will swap the first and last items in a list, while leaving the the middle
// elements in place because they will be swapped and then swapped back as
// shuffle iterates through the list.
diff --git a/tests/lib/js/static_interop_test/js_array_proxy_or_ref_test.dart b/tests/lib/js/static_interop_test/js_array_proxy_or_ref_test.dart
new file mode 100644
index 0000000..31ac275
--- /dev/null
+++ b/tests/lib/js/static_interop_test/js_array_proxy_or_ref_test.dart
@@ -0,0 +1,161 @@
+// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// SharedOptions=--enable-experiment=inline-class
+
+// Test the JS array proxy or reference that's created for lists for interop.
+
+import 'dart:js_interop';
+import 'package:expect/minitest.dart';
+
+@JS()
+external void eval(String code);
+
+@JS()
+external int iterateAndValidateArray(JSArray array);
+
+@JS()
+external Counter get counter;
+
+extension type Counter(JSObject _) implements JSObject {
+ external int get numElements;
+ external JSFunction get forEachBound;
+}
+
+extension on JSArray {
+ external int get length;
+ external set length(int val);
+ external int operator [](int i);
+ external void operator []=(int i, int value);
+ external int at(int i);
+ external JSArray concat([JSAny? arg1, JSAny? arg2, JSAny? arg3, JSAny? arg4]);
+ external void forEach(JSFunction callback, [JSObject thisObj]);
+ external int indexOf(JSAny? element, [int? fromIndex]);
+ external bool includes(JSAny? element, [int? fromIndex]);
+ external String join([String? delimiter]);
+ external int pop();
+ external int push([JSAny? arg1, JSAny? arg2, JSAny? arg3]);
+ external JSArray slice([int? start, int? end]);
+ external JSArray splice(
+ [int? start,
+ int? deleteCount,
+ JSAny? arg1,
+ JSAny? arg2,
+ JSAny? arg3,
+ JSAny? arg4]);
+}
+
+void main() {
+ final numList = <int?>[4, 3, 2, 1, 0].map((e) => e?.toJS).toList();
+ final numProxy = numList.toJSProxyOrRef;
+ final roundTripProxy = numProxy.toDart.toJSProxyOrRef;
+ expect(numProxy, roundTripProxy);
+
+ // `length`/`[]`/`at`/`[]=`
+ numProxy.length = 6;
+ expect(numList.length, 6);
+ expect(numProxy.length, numList.length);
+ numList.length = 5;
+ expect(numProxy.length, numList.length);
+ for (var i = 0; i < numProxy.length; i++) {
+ expect(numProxy[i], numProxy.length - i - 1);
+ expect(numProxy.at(i), numProxy.length - i - 1);
+
+ numProxy[i] = i;
+
+ expect(numProxy[i], i);
+ expect(numProxy.at(i), i);
+ // Test negative indexing.
+ expect(numProxy.at(i - numProxy.length), i);
+ expect(numList[i]!.toDartInt, i);
+ }
+
+ // `includes`/`indexOf`
+ expect(numProxy.includes(0.toJS), true);
+ expect(numProxy.includes(0.toJS, 1), false);
+ expect(numProxy.includes(5.toJS), false);
+ expect(numProxy.indexOf(0.toJS), 0);
+ expect(numProxy.indexOf(0.toJS, 1), -1);
+ expect(numProxy.indexOf(5.toJS), -1);
+
+ // `pop`/`push`
+ expect(numProxy.push(5.toJS), numList.length);
+ expect(numProxy.length, numList.length);
+ expect(numList.length, 6);
+ expect(numList[5]!.toDartInt, 5);
+
+ expect(numProxy.pop(), 5);
+ expect(numProxy.length, numList.length);
+ expect(numList.length, 5);
+
+ expect(numProxy.push(5.toJS, 6.toJS, 7.toJS), 8);
+ expect(numProxy.push(8.toJS, null, null), 11);
+ expect(numList.length, 11);
+ numList.length = 9; // Remove the `null`s we just added.
+
+ // iteration/for loop
+ eval('''
+ globalThis.iterateAndValidateArray = function(array) {
+ var counter = 0;
+ for (const i of array) {
+ if (counter != i) break;
+ counter++;
+ }
+ return counter;
+ };
+ ''');
+ expect(iterateAndValidateArray(numProxy), numList.length);
+
+ // `forEach`
+ var numElements = 0;
+ numProxy.forEach((int element, int index, JSArray array) {
+ expect(numElements, element);
+ expect(numElements, index);
+ expect(numProxy, array);
+ numElements++;
+ }.toJS);
+ expect(numElements, numList.length);
+
+ // `forEach` with a bound this.
+ eval('''
+ const counter = {
+ numElements: 0,
+ forEachBound: function (element, index, array) {
+ this.numElements++;
+ },
+ };
+ globalThis.counter = counter;
+ ''');
+ numProxy.forEach(counter.forEachBound, counter);
+ expect(counter.numElements, numList.length);
+
+ // `slice`, `splice`, and `concat`
+ void testEquals(JSArray arr, List<int> list) {
+ final len = list.length;
+ for (var i = 0; i < len; i++) {
+ expect(arr[i], list[i]);
+ }
+ }
+
+ final sliced = numProxy.slice(4, 9);
+ testEquals(sliced, [4, 5, 6, 7, 8]);
+ expect(numList.length, 9);
+ final deleted = numProxy.splice(4, 5);
+ testEquals(deleted, [4, 5, 6, 7, 8]);
+ expect(numList.length, 4);
+
+ final deleted2 = numProxy.splice(0, 4, 3.toJS, 2.toJS, 1.toJS, 0.toJS);
+ testEquals(deleted2, [0, 1, 2, 3]);
+ testEquals(numProxy, [3, 2, 1, 0]);
+ testEquals(numProxy.concat(deleted2), [3, 2, 1, 0, 0, 1, 2, 3]);
+
+ // `join`
+ expect(numProxy.join('-'), '3-2-1-0');
+ expect(numProxy.join(), '3,2,1,0');
+
+ // TODO(srujzs): Test the remaining JS Array methods. While we cover the
+ // common ones and the ones needed for JSArrayImpl, we might be missing some
+ // subtleties (like `isConcatSpreadble` for example) or missing handler
+ // methods that are needed other Array methods.
+}
diff --git a/tests/lib/lib.status b/tests/lib/lib.status
index f660402..b65d85a 100644
--- a/tests/lib/lib.status
+++ b/tests/lib/lib.status
@@ -81,6 +81,7 @@
js/static_interop_test/inline_class/external_member_test: SkipByDesign # Issue 42085. CSP policy disallows injected JS code
js/static_interop_test/inline_class/external_static_member_test: SkipByDesign # Issue 42085. CSP policy disallows injected JS code
js/static_interop_test/inline_class/external_static_member_with_namespaces_test: SkipByDesign # Issue 42085. CSP policy disallows injected JS code
+js/static_interop_test/js_array_proxy_or_ref_test: SkipByDesign # Issue 42085. CSP policy disallows injected JS code
js/static_interop_test/js_array_test: SkipByDesign # Issue 42085. CSP policy disallows injected JS code
js/static_interop_test/js_function_conversions_test: SkipByDesign # Issue 42085. CSP policy disallows injected JS code
js/static_interop_test/jsobject_type_test: SkipByDesign # Issue 42085. CSP policy disallows injected JS code
