proposal: runtime_checks_with_js_types

[srujzs]

runtime_checks_with_js_types

Description

These are lints that will make it easier to write and use JS interop code such that it can be deployed on dart2wasm without a divergence in semantics.

Details

Due to the difference in runtime types of the extension types ("JS types") we expose in dart:js_interop, casts and checks using is and as will almost certainly give different behavior depending on the platform.

A simple example is JSString. On dart2wasm, the representation type is a JSValue, while on the JS compilers it is String.

I go into a little more detail on this here: #4760 (comment). I chose to file a separate proposal, because even though there is heavy overlap, that proposal might not be sufficient.

Kind

Guard against errors and support platform compatibility.

Bad Examples

JSString x = ...;
x is String;

true on JS backends, false in dart2wasm. The reverse case is the same result.

JSAny x = ...;
x is JSString;

Always true on dart2wasm, only true if the value actually is a JS string value on the JS backends. Note that this may differ from the proposal in #4760, as this is check doesn't introduce or eliminate an extension type as it's a "downcheck" between two extension types.

List<dynamic> x = ...;
x is List<JSString>;

Depends on the initialization of x! #4760 does catch instances where we eliminate the extension type, so that'll combat the frequency of such checks. I think we should lint on this (whereas maybe not with a cast?) as this is likely to do more harm than good.

List<List<String>> x = ...;
x is List<JSString>;

This is always false on both compilers, but we can still lint, telling users they shouldn't do an is check where a JS type is compared against a non-JS type.

Object x = ...;
x is JSString;
x is List<JSString>;

This is a "downcheck" so we should likely lint. However, the reverse e.g. JSString is Object should probably not be linted, as that's statically true since JSString <: Object.

Good Examples

Add a few examples that demonstrate a “good” adoption of this lint’s principle.

JSString x = ...;
x is JSAny;

This is the reverse of the check above. This is okay because this is statically true as JSString <: JSAny. In the implementation, we should actually verify that though.

List<JSAny> x = ...;
x is CustomList<JSAny>;

Comparisons between JS types where the type is the same should be fine. The above check is not trivial either, as there is a useful check being done (List is a CustomList).

List<JSAny> x = ...;
x is List<JSAny?>;

Checking for nullability is interesting. I think this should be okay as the nullability should not be platform-dependent.

JSAny x = ...;
x as JSString;

Almost all the as cases are equivalent to the is cases, except for this one. This shouldn’t be a lint as it’s a legitimate downcast. If users know that JSAny is a JS string value, a cast is safe and the only way they can get a JSString. It may lead to different runtime semantics if users aren't careful, but we don't want to be in the way of a legitimate cast. Analysis with generics is similar e.g. List<JSAny> as List<JSString> is okay.

Discussion

We may choose to extend these lints further to suggest to users that they should use a conversion function when the intent is obvious. For example, String as JSString is an obvious candidate where we should tell users to use .toJS from dart:js_interop instead (and toDart in the reverse case). This would require a bit more inspection on the types involved.

In general, is is more dangerous and consistently wrong than as, so we may relax linting on casts versus checks.

[srujzs]

FYI @eernstg @sigmundch

I don't think any of the content here should be surprising, I'm just filing an issue to keep track.

[eernstg]

Sounds good!

I noted a couple of things:

In the section about good examples:

this is statically true as JSAny <: JSString

I guess that should be JSString <: JSAny?

At the end there's a "down-check" from JSAny to JSString. If I understood this correctly then this would amount to a check whether the given object is a JSValue on dart2wasm, and that JSValue could in turn be a device that allows us to access various different kinds of objects (including JavaScript strings, but also numbers, JSArrays, and more).

Would it be useful to lint this kind of type test and type check? The point would be that checking o is JSString and o as JSString is unsafe on dart2wasm because the given object could be various very different things, and yet the check/test would succeed. To me it sounds like a software entity (library, package, ...) that aims to support dart2wasm as well as other platforms should not use this kind of type test/check at all. Perhaps they can use .toJS instead? Or maybe there is a method on JSAny that tests reliably (on dart2wasm in particular) whether a given object is a JSString, JSNumber etc? (Could be something like JSString? get asJSStringOrNull.)

[srujzs]

I guess that should be JSString <: JSAny?

Yes, thanks! :)

At the end there's a "down-check" from JSAny to JSString. If I understood this correctly then this would amount to a check whether the given object is a JSValue on dart2wasm, and that JSValue could in turn be a device that allows us to access various different kinds of objects (including JavaScript strings, but also numbers, JSArrays, and more).

I presume you're referring to the bad example where we're checking whether a variable of type JSAny is a JSString. Your understanding is correct - JSValue is simply a box around any JS value. dart2wasm doesn't have separate boxes per JS value type.

Would it be useful to lint this kind of type test and type check?

I again presume you're referring to the bad example. I believe so, for the same reason you've mentioned where the underlying runtime type is not consistent and on dart2wasm, the type can represent many different values.

The canonical way to check the type of the JS value is to use a JS mechanism, like an instanceof or typeof check, for which we provide helpers e.g. typeofEquals. Another proposal that we've thought about to make this easier is this: dart-lang/sdk#54138. The key is that dart2wasm doesn't differentiate JS values, and therefore we need to rely on interop to do so.

[eernstg]

Sounds like we really want to have a lint called runtime_checks_with_js_types. ;-D

[srujzs]

This has landed under the lint rule invalid_runtime_check_with_js_interop_types.