@srujzs and I were talking yesterday about the state of js_util in dart2wasm and how some stuff that's fundamentally broken about it makes it difficult to consistently implement js_interop behavior. (Examples given below.) So can we just stop supporting js_util?
Here's the allowlist of interop libraries:
|
static const allowedInteropLibrariesInDart2WasmPackages = [ |
|
// Both these packages re-export other interop libraries |
|
'js', |
|
'js_util', |
|
// Flutter/benchmarks. |
|
'flutter', |
|
'engine', |
|
'ui', |
|
// Non-SDK packages that have been migrated for the Wasm experiment but |
|
// still have references to older interop libraries. |
|
'package_info_plus', |
|
'test', |
|
'url_launcher_web', |
|
]; |
Flutter has been migrated off js_util. A quick grep shows that package_info_plus, test, and url_launcher_web all seem to be migrated too, and there are no hits for js_util in our benchmarks either.
We don't have to do any immediate cleanup or replace the current impl with throw UnsupportedError() or whatever, but it would be nice to delete tests/web/wasm/js_util_test.dart (and any other relevant tests) so that we don't have to support the behavior they expect.
@osa1 @mkustermann
Motivation
The dart2wasm implementation of js_util is inconsistent with the JS backends' implementation. Because js_util and js_interop share some implementation code (mainly via js_helper), this makes it difficult to make a consistent js_interop implementation that doesn't break our current js_util expectations.
Consider issue #54573. In order to make dart2wasm consistent with the JS backends, we need dartifyRaw to add a case checking for Promises and converting them to Futures. That's easy enough, but breaks js_util, which already relies on certain behavior from dartifyRaw to implement some of its APIs.
For example, getProperty is inconsistent. On the JS backends, we simply fetch a JS value, e.g. a Promise:
|
@patch |
|
T getProperty<T>(Object o, Object name) => |
|
JS<dynamic>('Object|Null', '#[#]', o, name); |
But in dart2wasm, we convert it:
|
@patch |
|
T getProperty<T>(Object o, Object name) => |
|
dartifyRaw(getPropertyRaw(jsifyRaw(o), jsifyRaw(name))) as T; |
This currently wraps the promise in a JSValue.
promiseToFuture is also inconsistent. The dart2wasm implementation has to jsify its argument:
Meanwhile, the JS implementation does no such conversion.
These inconsistencies "cancel", allowing promiseToFuture to be invoked on the result of getProperty, e.g.
|
Future f = promiseToFuture(getProperty(gt, 'resolvedPromise')); |
But once we change dartifyRaw to produce a Future from a Promise, promiseToFuture will no longer be able to successfully jsifyRaw the result. Adding the Future-to-Promise conversion to jsify is possible, but is inconsistent with the JS backends (and shoehorns even more cases into this API we want to narrow).
@srujzs and I were talking yesterday about the state of
js_utilin dart2wasm and how some stuff that's fundamentally broken about it makes it difficult to consistently implementjs_interopbehavior. (Examples given below.) So can we just stop supportingjs_util?Here's the allowlist of interop libraries:
sdk/pkg/_js_interop_checks/lib/js_interop_checks.dart
Lines 125 to 138 in c11f911
Flutter has been migrated off
js_util. A quick grep shows thatpackage_info_plus,test, andurl_launcher_weball seem to be migrated too, and there are no hits forjs_utilin our benchmarks either.We don't have to do any immediate cleanup or replace the current impl with
throw UnsupportedError()or whatever, but it would be nice to deletetests/web/wasm/js_util_test.dart(and any other relevant tests) so that we don't have to support the behavior they expect.@osa1 @mkustermann
Motivation
The dart2wasm implementation of
js_utilis inconsistent with the JS backends' implementation. Becausejs_utilandjs_interopshare some implementation code (mainly viajs_helper), this makes it difficult to make a consistentjs_interopimplementation that doesn't break our currentjs_utilexpectations.Consider issue #54573. In order to make dart2wasm consistent with the JS backends, we need
dartifyRawto add a case checking forPromises and converting them toFutures. That's easy enough, but breaksjs_util, which already relies on certain behavior fromdartifyRawto implement some of its APIs.For example,
getPropertyis inconsistent. On the JS backends, we simply fetch a JS value, e.g. aPromise:sdk/sdk/lib/_internal/js_shared/lib/js_util_patch.dart
Lines 80 to 82 in 7cec6b3
sdk/sdk/lib/_internal/wasm/lib/js_util_patch.dart
Lines 92 to 94 in 7cec6b3
JSValue.promiseToFutureis also inconsistent. The dart2wasm implementation has to jsify its argument:sdk/sdk/lib/_internal/wasm/lib/js_util_patch.dart
Line 185 in 7cec6b3
These inconsistencies "cancel", allowing
promiseToFutureto be invoked on the result ofgetProperty, e.g.sdk/tests/web/wasm/js_util_test.dart
Line 364 in 7cec6b3
But once we change
dartifyRawto produce aFuturefrom aPromise,promiseToFuturewill no longer be able to successfullyjsifyRawthe result. Adding theFuture-to-Promiseconversion tojsifyis possible, but is inconsistent with the JS backends (and shoehorns even more cases into this API we want to narrow).