Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dart:js_interop] Add external isA and asA helpers to dart:js_interop for is and as equivalents #54138

Closed
srujzs opened this issue Nov 22, 2023 · 4 comments
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-js-interop Issues that impact all js interop

Comments

@srujzs
Copy link
Contributor

srujzs commented Nov 22, 2023

Related issue: #52030

Users are used to using is and as for dart:html objects. This allowed them to write simple code like x is Window, and know that x actually is a JS Window object. Replicating this on some arbitrary object x (let's say it's typed as JSAny?) requires the following with dart:js_interop:

x.typeofEquals('object') && (x as JSObject).instanceOfString('Window')

which is definitely an ergonomic downgrade. Even if we move instanceOfString up to JSAny? (which may be worthwhile), it's still less ideal to have to type out the name of the type. And if we had more complex types, like user-defined JS interop types that exist in a particular library, that String gets more annoying to write.

One option is transformations to handle this. Consider a method external bool isA<T>() that exists on JSAny?. This method will do the following (assume that dart:js_interop types are extension types, which they will be):

  1. Validate that T is an extension interop type or a @staticInterop type.
  2. If T is an extension type that is one of the dart:js_interop types or any number of extension types on them, do the following:
  • If the dart:js_interop type is one of the non-JSObject types, lower to the equivalent typeofEquals check e.g. isA<JSString>() -> typeofEquals('string'). We might want to be a little careful here of extension types that may wrap these types e.g. extension type S(JSString s). I'm inclined to say that we should still lower to the same check here and ignore any renaming users may do.
  • If it is, then use the @JS() renaming annotations (including the library's) if any to determine the type users want and then do an instanceofString check using that type e.g. isA<JSArray> -> instanceofString('Array').
  1. If T is @staticInterop type, this is the same as the second bullet point of step 2.

This would make the check now look like:

x.isA<Window>()

T asA<T>() could just be implemented as:

if (!this.isA<T>) throw TypeError(...);
return this as T;

We might not want an asA because users may think they need it after an isA, which they don't. However, it is useful for users who want stronger checking and if we can mark isA as a function whose value doesn't change (handwaving dart2js optimizations here), then maybe we can optimize the duplicate isA.

Note that this doesn't give us type promotion still, but that wouldn't be possible without some language functionality anyways.

cc @kevmoo because migrating without this is admittedly less ergonomic when working with package:web types.

  • Addendum (1/4/24):

A potential wrinkle here is the treatment of types that have object literal constructors e.g.

extension type ObjectLiteral(JSObject _) {
  external ObjectLiteral({JSAny foo});
}

If we wrote isA<ObjectLiteral>(x), what are we asking for here? The above algorithm would try do a x instanceof globalContext.ObjectLiteral, but that's likely not what the user wants. We could make this check equivalent to a typeof x == 'object' to better capture the fact that we're using it as an object literal, but this might not be what the user wants either. After all, there might actually exist a globalContext.ObjectLiteral that the user wants to check here. Should we make this an error? Maybe, but that's a loss of expressivity for users.

@srujzs srujzs added web-js-interop Issues that impact all js interop area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. labels Nov 22, 2023
@eernstg
Copy link
Member

eernstg commented Nov 23, 2023

Interesting! Here's a random drive-by comment:

Validate that T is an extension interop type

Aha, so this is a description of a transformation which is applied to each call site for isA<T>() resp. asA<T>()?

Otherwise, it sounds like it would depend on having information which isn't there: Extension types (interop or not) are not expected to exist at run time. I would actually expect them to be eliminated already in the kernel code that any given backend receives.

But if it is a transformation on each call site (and it occurs at a time where extension types are still present) then it sounds very much like the treatment which is given to every invocation of an external instance member of a JS interop extension type when translating to JavaScript. The generated code is different ("generate a native JavaScript invocation" vs. "generate x.typeofEquals('...') && (x as JSObject).instanceOfString('...')"), but it sounds like that wouldn't be hard to handle.

I wonder if this idea (that is: use a marker like external and/or metadata to indicate that a certain extension type member should give rise to custom code generation at each call site) could be useful in other settings as well?

@srujzs
Copy link
Contributor Author

srujzs commented Nov 27, 2023

Aha, so this is a description of a transformation which is applied to each call site for isA() resp. asA()?

Indeed. Our JS interop transformations in general occur at the call-site (which is why we don't let users do tear-offs of external members) like you've noted, and this would be the same. We do this with a few other methods as well, like createDartExport, which creates a JS wrapper around a Dart object so that it can exported to JS. I'd expect an implementation for this would be fairly simple.

I wonder if this idea (that is: use a marker like external and/or metadata to indicate that a certain extension type member should give rise to custom code generation at each call site) could be useful in other settings as well?

Do you mean outside of interop? In general, I can see it useful for functions that need transformations, but I don't think this is unique to extension type members. external is nice, but it doesn't describe whether the transformation is at the call-site or not.

@eernstg
Copy link
Member

eernstg commented Nov 28, 2023

we don't let users do tear-offs of external members

If you want to lift that restriction then a regular Dart function literal whose body performs the magic interop invocation should be sufficient (there is no special magic about the behavior of ==, as specified here).

I don't think this is unique to extension type members

Right, and we probably don't want to use external for that purpose in a more general setting.

All kinds of invocations could be compiled in a customized manner if the callee declaration carries a marker which is interpreted to be a request for this kind of customized translation. (Basically, it's a macro which is applied at all call sites).

For instance, invocations of copyWith could be translated to provide missing arguments from the receiver:

const provideMissingArgumentsFromReceiver = 0; // Cheat.

class Data {
  final int x, y;
  Data({required this.x, required this.y});
  
  @provideMissingArgumentsFromReceiver
  Data copyWith({required x, required y}) => Data(x: x, y: y);
}

void main() {
  var d = Data(x: 3, y: 4);
  var d2 = d.copyWith(x: 2); // OK, because of a compiler transformation.
}

The expression d.copyWith(x: 2) would be translated into something like d.copyWith(x: 2, y: d.y) (which would in reality be more like let v = d in v.copyWith(x: 2, y: v.y) because we don't want to evaluate the receiver expression multiple times).

The compiler would not report an error for the missing argument named y. It would instead apply the non-standard translation, and the resulting code would compile without errors.

@srujzs
Copy link
Contributor Author

srujzs commented Nov 28, 2023

If you want to lift that restriction then a regular Dart function literal whose body performs the magic interop invocation should be sufficient

Agreed! We recommend users create a function literal instead, but we can likely do that ourselves for users. We maybe need to be a bit careful about accounting for the CFE's generated calls and making sure tear-off equality is retained when called on the same object, but it's doable.

For the example above with copyWith, there will need to be language/macro support for that (due to the required), correct? I can see some of these functions that we currently use transformations for using macros in the future, but I haven't read the spec to determine yet. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. web-js-interop Issues that impact all js interop
Projects
Status: Done
Development

No branches or pull requests

2 participants