From 4efb33e1281b9d65b826978a728ee64091ff05e1 Mon Sep 17 00:00:00 2001 From: Srujan Gaddam Date: Tue, 12 Sep 2023 23:11:55 +0000 Subject: [PATCH] [dart:js_interop_unsafe] Rename extensions and make []/[]= take String properties Strings are the most likely use case of getting and setting properties, and therefore we should make that easier to use. This CL also renames the extensions in dart:js_interop_unsafe to a more relevant name and to avoid conflicts in dart:js_interop. CoreLibraryReviewExempt: Backend-specific library. Change-Id: Ia8ce6593167c648f9710b47cfe27f80c854be407 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324572 Reviewed-by: Sigmund Cherem Commit-Queue: Srujan Gaddam --- .../src/transformations/export_creator.dart | 18 +++++------ .../lib/js_interop_unsafe_patch.dart | 8 ++--- sdk/lib/_internal/wasm/lib/js_helper.dart | 6 +--- .../_internal/wasm/lib/js_interop_patch.dart | 23 +++----------- .../wasm/lib/js_interop_unsafe_patch.dart | 8 ++--- sdk/lib/_internal/wasm/lib/regexp_helper.dart | 2 +- .../js_interop_unsafe/js_interop_unsafe.dart | 20 +++++++----- tests/lib/js_interop_unsafe/basic_test.dart | 31 +++++++++---------- 8 files changed, 51 insertions(+), 65 deletions(-) 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 05adf8e9c7d..316bd1944f8 100644 --- a/pkg/_js_interop_checks/lib/src/transformations/export_creator.dart +++ b/pkg/_js_interop_checks/lib/src/transformations/export_creator.dart @@ -39,7 +39,7 @@ class ExportCreator extends Transformer { this._typeEnvironment, this._diagnosticReporter, this._exportChecker) : _callMethodVarArgs = _typeEnvironment.coreTypes.index .getTopLevelProcedure('dart:js_interop_unsafe', - 'JSObjectUtilExtension|callMethodVarArgs'), + 'JSObjectUnsafeUtilExtension|callMethodVarArgs'), _createDartExport = _typeEnvironment.coreTypes.index .getTopLevelProcedure('dart:js_util', 'createDartExport'), _createStaticInteropMock = _typeEnvironment.coreTypes.index @@ -47,7 +47,7 @@ class ExportCreator extends Transformer { _functionToJS = _typeEnvironment.coreTypes.index.getTopLevelProcedure( 'dart:js_interop', 'FunctionToJSExportedDartFunction|get#toJS'), _getProperty = _typeEnvironment.coreTypes.index.getTopLevelProcedure( - 'dart:js_interop_unsafe', 'JSObjectUtilExtension|[]'), + 'dart:js_interop_unsafe', 'JSObjectUnsafeUtilExtension|[]'), _globalContext = _typeEnvironment.coreTypes.index .getTopLevelProcedure('dart:js_interop', 'get:globalContext'), _jsAny = _typeEnvironment.coreTypes.index @@ -55,7 +55,7 @@ class ExportCreator extends Transformer { _jsObject = _typeEnvironment.coreTypes.index .getClass('dart:_js_types', 'JSObject'), _setProperty = _typeEnvironment.coreTypes.index.getTopLevelProcedure( - 'dart:js_interop_unsafe', 'JSObjectUtilExtension|[]='), + 'dart:js_interop_unsafe', 'JSObjectUnsafeUtilExtension|[]='), _stringToJS = _typeEnvironment.coreTypes.index.getTopLevelProcedure( 'dart:js_interop', 'StringToJSString|get#toJS'), _staticInteropMockValidator = StaticInteropMockValidator( @@ -201,7 +201,7 @@ class ExportCreator extends Transformer { // Get the global 'Object' property. Expression getObjectProperty() => asJSObject(StaticInvocation(_getProperty, - Arguments([StaticGet(_globalContext), toJSString('Object')]))) + Arguments([StaticGet(_globalContext), StringLiteral('Object')]))) ..fileOffset = node.fileOffset; // Get a fresh object literal, using the proto to create it if one was @@ -240,9 +240,9 @@ class ExportCreator extends Transformer { var exports = exportMap[exportName]!; ExpressionStatement setProperty( VariableGet jsObject, String propertyName, StaticInvocation jsValue) { - // `jsObject[propertyName.toJS] = jsValue` + // `jsObject[propertyName] = jsValue` return ExpressionStatement(StaticInvocation(_setProperty, - Arguments([jsObject, toJSString(propertyName), jsValue]))) + Arguments([jsObject, StringLiteral(propertyName), jsValue]))) ..fileOffset = node.fileOffset ..parent = node.parent; } @@ -251,7 +251,7 @@ class ExportCreator extends Transformer { // With methods, there's only one export per export name. if (firstExport is Procedure && firstExport.kind == ProcedureKind.Method) { - // `jsExport[jsName.toJS] = dartMock.tearoffMethod.toJS` + // `jsExport[jsName] = dartMock.tearoffMethod.toJS` block.add(setProperty( VariableGet(jsExporter), exportName, @@ -276,7 +276,7 @@ class ExportCreator extends Transformer { // The AST code looks like: // // ``` - // getSetMap['get'.toJS] = () { + // getSetMap['get'] = () { // return dartInstance.getter; // }.toJS; // ``` @@ -284,7 +284,7 @@ class ExportCreator extends Transformer { // in the case of a getter and: // // ``` - // getSetMap['set'.toJS] = (val) { + // getSetMap['set'] = (val) { // dartInstance.setter = val; // }.toJS; // ``` diff --git a/sdk/lib/_internal/js_shared/lib/js_interop_unsafe_patch.dart b/sdk/lib/_internal/js_shared/lib/js_interop_unsafe_patch.dart index 1e3210afd70..c9e0e026cbe 100644 --- a/sdk/lib/_internal/js_shared/lib/js_interop_unsafe_patch.dart +++ b/sdk/lib/_internal/js_shared/lib/js_interop_unsafe_patch.dart @@ -8,7 +8,7 @@ import 'dart:js_interop' hide JS; import 'dart:js_util' as js_util; @patch -extension JSObjectUtilExtension on JSObject { +extension JSObjectUnsafeUtilExtension on JSObject { @patch @pragma('dart2js:prefer-inline') JSBoolean hasProperty(JSAny property) => @@ -16,12 +16,12 @@ extension JSObjectUtilExtension on JSObject { @patch @pragma('dart2js:prefer-inline') - JSAny? operator [](JSAny property) => + T getProperty(JSAny property) => JS('Object|Null', '#[#]', this, property); @patch @pragma('dart2js:prefer-inline') - void operator []=(JSAny property, JSAny? value) => + void setProperty(JSAny property, JSAny? value) => JS('', '#[#] = #', this, property, value); // TODO(joshualitt): Specialize at callsites. @@ -57,7 +57,7 @@ extension JSObjectUtilExtension on JSObject { } @patch -extension JSFunctionUtilExtension on JSFunction { +extension JSFunctionUnsafeUtilExtension on JSFunction { // TODO(joshualitt): Specialize `callAsConstructor`. @patch @pragma('dart2js:prefer-inline') diff --git a/sdk/lib/_internal/wasm/lib/js_helper.dart b/sdk/lib/_internal/wasm/lib/js_helper.dart index c9e7b22df70..e927ba981c4 100644 --- a/sdk/lib/_internal/wasm/lib/js_helper.dart +++ b/sdk/lib/_internal/wasm/lib/js_helper.dart @@ -10,6 +10,7 @@ import 'dart:_js_annotations' as js; import 'dart:_js_types' as js_types; import 'dart:_wasm'; import 'dart:js_interop'; +import 'dart:js_interop_unsafe'; import 'dart:typed_data'; part 'regexp_helper.dart'; @@ -98,11 +99,6 @@ extension JSArrayExtension on JSArray { external JSNumber get length; } -extension JSObjectExtension on JSObject { - external JSAny? operator [](JSString key); - external void operator []=(JSString key, JSAny? value); -} - class JSArrayIteratorAdapter implements Iterator { final JSArray array; int index = -1; diff --git a/sdk/lib/_internal/wasm/lib/js_interop_patch.dart b/sdk/lib/_internal/wasm/lib/js_interop_patch.dart index be9cf755b80..f3bf957ba7d 100644 --- a/sdk/lib/_internal/wasm/lib/js_interop_patch.dart +++ b/sdk/lib/_internal/wasm/lib/js_interop_patch.dart @@ -473,25 +473,12 @@ JSArray _createJSProxyOfList(List list) { // 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; + jsExportWrapper.setProperty(_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 getIndex = jsExportWrapper['_getIndex']!.toExternRef; + final setIndex = jsExportWrapper['_setIndex']!.toExternRef; + final hasIndex = jsExportWrapper['_hasIndex']!.toExternRef; + final deleteIndex = jsExportWrapper['_deleteIndex']!.toExternRef; final proxy = _box(js_helper.JS(''' (wrapper, getIndex, setIndex, hasIndex, deleteIndex) => new Proxy(wrapper, { diff --git a/sdk/lib/_internal/wasm/lib/js_interop_unsafe_patch.dart b/sdk/lib/_internal/wasm/lib/js_interop_unsafe_patch.dart index 52f9b01a8f5..c417fb85ac3 100644 --- a/sdk/lib/_internal/wasm/lib/js_interop_unsafe_patch.dart +++ b/sdk/lib/_internal/wasm/lib/js_interop_unsafe_patch.dart @@ -14,17 +14,17 @@ import 'dart:js_interop' hide JS; T _box(WasmExternRef? ref) => JSValue.box(ref) as T; @patch -extension JSObjectUtilExtension on JSObject { +extension JSObjectUnsafeUtilExtension on JSObject { @patch JSBoolean hasProperty(JSAny property) => _box(JS( '(o, p) => p in o', toExternRef, property.toExternRef)); @patch - JSAny? operator [](JSAny property) => _box( + T getProperty(JSAny property) => _box( JS('(o, p) => o[p]', toExternRef, property.toExternRef)); @patch - void operator []=(JSAny property, JSAny? value) => JS( + void setProperty(JSAny property, JSAny? value) => JS( '(o, p, v) => o[p] = v', toExternRef, property.toExternRef, @@ -57,7 +57,7 @@ extension JSObjectUtilExtension on JSObject { } @patch -extension JSFunctionUtilExtension on JSFunction { +extension JSFunctionUnsafeUtilExtension on JSFunction { @patch JSObject _callAsConstructor( [JSAny? arg1, JSAny? arg2, JSAny? arg3, JSAny? arg4]) => diff --git a/sdk/lib/_internal/wasm/lib/regexp_helper.dart b/sdk/lib/_internal/wasm/lib/regexp_helper.dart index 081a337b0c0..5a2f7970cc4 100644 --- a/sdk/lib/_internal/wasm/lib/regexp_helper.dart +++ b/sdk/lib/_internal/wasm/lib/regexp_helper.dart @@ -202,7 +202,7 @@ class _MatchImplementation implements RegExpMatch { String? namedGroup(String name) { JSObject? groups = _match.groups; if (groups.isDefinedAndNotNull) { - Object? result = dartifyRaw(groups![name.toJS]?.toExternRef); + Object? result = dartifyRaw(groups![name]?.toExternRef); if (result != null || hasPropertyRaw(groups.toExternRef, name.toExternRef)) { return result?.toString(); diff --git a/sdk/lib/js_interop_unsafe/js_interop_unsafe.dart b/sdk/lib/js_interop_unsafe/js_interop_unsafe.dart index fa094412000..2765f7a7dad 100644 --- a/sdk/lib/js_interop_unsafe/js_interop_unsafe.dart +++ b/sdk/lib/js_interop_unsafe/js_interop_unsafe.dart @@ -25,18 +25,22 @@ library dart.js_interop_unsafe; import 'dart:js_interop'; -extension JSObjectUtilExtension on JSObject { +extension JSObjectUnsafeUtilExtension on JSObject { /// Whether or not this [JSObject] has a given property. external JSBoolean hasProperty(JSAny property); - /// Equivalent to invoking operator `[]` in JS. - external JSAny? operator [](JSAny property); + /// Shorthand helper to get String properties. + JSAny? operator [](String property) => getProperty(property.toJS); - /// Gets a given property from this [JSObject]. - T getProperty(JSAny property) => this[property] as T; + /// Gets a given [property] from this [JSObject]. + external T getProperty(JSAny property); - /// Equivalent to invoking `[]=` in JS. - external void operator []=(JSAny property, JSAny? value); + /// Shorthand helper to set String properties. + void operator []=(String property, JSAny? value) => + setProperty(property.toJS, value); + + /// Sets a given [property] with [value] on this [JSObject]. + external void setProperty(JSAny property, JSAny? value); /// Calls a method on this [JSObject] with up to four arguments and returns /// the result. @@ -57,7 +61,7 @@ extension JSObjectUtilExtension on JSObject { external JSBoolean delete(JSAny property); } -extension JSFunctionUtilExtension on JSFunction { +extension JSFunctionUnsafeUtilExtension on JSFunction { /// Calls this [JSFunction] as a constructor with up to four arguments and /// returns the constructed [JSObject]. external JSObject _callAsConstructor( diff --git a/tests/lib/js_interop_unsafe/basic_test.dart b/tests/lib/js_interop_unsafe/basic_test.dart index f5becd4c2a0..24da2be287d 100644 --- a/tests/lib/js_interop_unsafe/basic_test.dart +++ b/tests/lib/js_interop_unsafe/basic_test.dart @@ -17,9 +17,9 @@ external void eval(String code); void createObjectTest() { JSObject o = newObject(); Expect.isFalse(o.hasProperty('foo'.toJS).toDart); - o['foo'.toJS] = 'bar'.toJS; + o['foo'] = 'bar'.toJS; Expect.isTrue(o.hasProperty('foo'.toJS).toDart); - Expect.equals('bar', (o['foo'.toJS] as JSString).toDart); + Expect.equals('bar', (o['foo'] as JSString).toDart); } void equalTest() { @@ -46,9 +46,9 @@ void equalTest() { '''); JSObject gc = globalContext; void test(String propertyName, bool testCanonicalization) { - Expect.equals(gc[propertyName.toJS], gc[propertyName.toJS]); + Expect.equals(gc[propertyName], gc[propertyName]); if (testCanonicalization) { - Expect.equals(gc[propertyName.toJS], gc[(propertyName + "2").toJS]); + Expect.equals(gc[propertyName], gc[propertyName + "2"]); } } @@ -83,11 +83,10 @@ void typeofTest() { }; void test(String property, String expectedType) { Expect.isTrue( - globalContext[property.toJS]?.typeofEquals(expectedType.toJS).toDart); + globalContext[property]?.typeofEquals(expectedType.toJS).toDart); for (final type in types) { if (type != expectedType) { - Expect.isFalse( - globalContext[property.toJS]?.typeofEquals(type.toJS).toDart); + Expect.isFalse(globalContext[property]?.typeofEquals(type.toJS).toDart); } } } @@ -110,9 +109,9 @@ void instanceOfTest() { globalThis.obj = new JSClass1(); '''); JSObject gc = globalContext; - JSObject obj = gc['obj'.toJS] as JSObject; - JSFunction jsClass1Constructor = gc['JSClass1'.toJS] as JSFunction; - JSFunction jsClass2Constructor = gc['JSClass2'.toJS] as JSFunction; + JSObject obj = gc['obj'] as JSObject; + JSFunction jsClass1Constructor = gc['JSClass1'] as JSFunction; + JSFunction jsClass2Constructor = gc['JSClass2'] as JSFunction; Expect.isTrue(obj.instanceof(jsClass1Constructor).toDart); Expect.isFalse(obj.instanceof(jsClass2Constructor).toDart); } @@ -152,7 +151,7 @@ void evalAndConstructTest() { globalThis.JSClass = JSClass; '''); JSObject gc = globalContext; - JSFunction constructor = gc['JSClass'.toJS] as JSFunction; + JSFunction constructor = gc['JSClass'] as JSFunction; // Var args JSObject jsObj1 = @@ -268,7 +267,7 @@ void deepConversionsTest() { globalThis.keyObject2 = keyObject; '''); JSObject gc = globalContext; - Expect.isNull(gc['a'.toJS]); + Expect.isNull(gc['a']); Expect.equals('foo', gc.getProperty('b'.toJS).toDart); _expectRecEquals( ['a', 'b', 'c'], @@ -320,7 +319,7 @@ void deepConversionsTest() { gc.getProperty('dataView'.toJS).toDart.buffer.asUint8List()); // Confirm a function that takes a roundtrip remains a function. - JSFunction foo = gc['f'.toJS].dartify() as JSFunction; + JSFunction foo = gc['f'].dartify() as JSFunction; Expect.equals( 'hello world', gc.callMethod('invoke'.toJS, foo).toDart); @@ -334,12 +333,12 @@ void deepConversionsTest() { 3, {'baz': 'boo'} ], - ], gc['implicitExplicit'.toJS].dartify() as Iterable); + ], gc['implicitExplicit'].dartify() as Iterable); // Test that JS objects behave as expected in Map / Set. Set set = {}; - JSAny? key1 = gc['keyObject1'.toJS]; - JSAny? key2 = gc['keyObject2'.toJS]; + JSAny? key1 = gc['keyObject1']; + JSAny? key2 = gc['keyObject2']; Expect.isTrue(set.add(key1)); Expect.isTrue(set.contains(key1)); Expect.isFalse(set.add(key2));