linter
linter copied to clipboard
proposal: `runtime_checks_with_js_types`
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: https://github.com/dart-lang/linter/issues/4760#issuecomment-1767230531. 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.
FYI @eernstg @sigmundch
I don't think any of the content here should be surprising, I'm just filing an issue to keep track.
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, JSArray
s, 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
.)
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: https://github.com/dart-lang/sdk/issues/54138. The key is that dart2wasm doesn't differentiate JS values, and therefore we need to rely on interop to do so.
Sounds like we really want to have a lint called runtime_checks_with_js_types
. ;-D
This has landed under the lint rule invalid_runtime_check_with_js_interop_types
.