dart-lang / web

Lightweight browser API bindings built around JS static interop.
https://pub.dev/packages/web
BSD 3-Clause "New" or "Revised" License
134 stars 23 forks source link

Add generics to `Promise`? #31

Closed kevmoo closed 8 months ago

kevmoo commented 1 year ago

"Is such a thing even possible?" CC @joshualitt

joshualitt commented 1 year ago

Honestly, we need to sit down and discuss this. Pre-inline classes, we can't do this. Post inline-classes we can, but should we? Type arguments for inline classes will be checked statically, but they won't be checked at runtime. On the one hand, then we get some documentation, but on the other hand it may lead users into a false sense of security.

So we need to decide, is it better to have type arguments that aren't reified, and to force users to write casts that are basically noops, or is it better to not have type arguments at all and to nudge users toward instanceof / typeof?

To be totally honest, I'm a bit on the fence, and I see both arguments. If I had to choose, I lean weakly towards not having type argument because I think they are deceiving, and may lead users to think they can rely on Dart type checking, vs. instanceof / typeof. I'd rather we completely disallow is and as on static interop types, and in the error messages we can point users towards typeof / instanceof. That will ensure users are guided towards best practices.

Thoughts?

@sigmundch @srujzs

sigmundch commented 1 year ago

I also lean towards not having the type-parameters. Every use of type parameters in JSInterop until now has been for documentation purposes, but also deceptive.

That said, I suggest we first create a few scenarios and evaluate once we have examples in front of us.

kevmoo commented 1 year ago

At a minimum, we could add a /// doc comment telling folks what type to expect.

joshualitt commented 1 year ago

Yes, I think we should document the generics, including Promise, JSArray, and any other generic objects.

Along that line of thinking, a big part of this is educating users on JS types(including static interop types). JS types don't really exist at runtime. Thus, rather than learning to rely on the Dart type system, which will just give them a false sense of security, they should treat JS types and static interop APIs like the actual JS types and JS functions they represent. That is, the types are just for documentation. If they are just passing the type back to JS, they may not care. If they do care about the type, then they need to use typeof and instanceof operators to ensure they have the type they expect, and then they can dispatch accordingly.

The nice thing here about this approach is it will always be consistent and exactly what a JS developer writing JS would have to do. Furthermore, we can include lints / static errors to ensure our users stay on the rails. That said, the vast majority of JS types just flow to and from JS, so most of the time users won't even have to think about these things.

srujzs commented 1 year ago

I'm assuming Promise is implemented as an inline class. IIRC, one of the reasons we disliked type parameters on regular JS interop classes is because there is a runtime type associated with it, and therefore that introduces potential for confusion between the JS type and the written Dart type. With inline classes, this isn't the case, so type parameters on inline classes are only statically checked, regardless if they're interop inline classes or not. So using type parameters in this case is idiomatic unlike before.

That being said, I think it's fair to say that that behavior is confusing, especially since inline classes is a new concept for many users. Furthermore, inline classes allow users to introduce their own version of Promise with a type parameter anyways, so it's just a matter of ergonomics if we want to expose Promise with a type parameter.

kevmoo commented 1 year ago

I'd love to understand better some of the tradeoffs here. Having the generic in Promise makes SO MANY things easier. Maybe there's a way we can cheat here – we own the whole stack!

On Mon, Mar 6, 2023 at 8:08 AM Srujan Gaddam @.***> wrote:

I'm assuming Promise is implemented as an inline class. IIRC, one of the reasons we disliked type parameters on regular JS interop classes is because there is a runtime type associated with it, and therefore that introduces potential for confusion between the JS type and the written Dart type. With inline classes, this isn't the case, so type parameters on inline classes are only statically checked, regardless if they're interop inline classes or not. So using type parameters in this case is idiomatic.

That being said, I think it's fair to say that that behavior is confusing, especially since inline classes is a new concept for many users. Furthermore, inline classes allow users to introduce their own version of Promise with a type parameter anyways, so it's just a matter of ergonomics if we want to expose Promise with a type parameter.

— Reply to this email directly, view it on GitHub https://github.com/dart-lang/web/issues/31#issuecomment-1456420816, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEFCVRUBN32JSUC4NFE23W2YDZFANCNFSM6AAAAAAVOAGGRM . You are receiving this because you authored the thread.Message ID: @.***>

joshualitt commented 1 year ago

...Having the generic in Promise makes SO MANY things easier.

So does dynamic ;-)

This is a very nuanced topic, and there is no right answer. I don't think anyone in this thread has strong feelings for or against this. I'm definitely open to being persuaded here, but for an MVP, it probably makes sense to do the restrictive thing first and then relax it later, vs. the alternative.

The crux of the problem is that the type argument when it is used on an external function is just a fancy comment, but users may be led to believe otherwise.

In general, we're moving away from pretending that the Dart type system is the source of truth for JS. This is because it leads users into a false sense of security. It is better for everyone if users program JS interop 'defensively', as opposed to assuming the guarantees of the Dart type system extend to JS interop.

With JS interop, we give the appearance of many types, but in reality every JS type is just a view on the only static JS type that there is, JSAny. If a user actually cares about the type of a JS object, then they need to use instanceof / typeof, not Dart's type checking operations. In fact, I'm hoping we disallow is / as checks on JS interop types entirely, and instead provide users checked and unchecked methods for performing these functions safely. Why provide unchecked variants? Simply so that users have to stop and think before they use is / as on JS types, thus introducing subtle bugs into their code. We can always have a flag to flip unchecked variants into checked variants, thus giving users a quick way to sanity check their code. I digress...

So returning to type arguments. Before inline classes, we have no choice here, we can't because type arguments on classes have a specific semantics we can't match. After inline classes, we can support this but should we? Is it really worse than just writing an explicit type?

I truly honestly don't know. The reason it might be worse than just writing an explicit type, is because now we have a non-local type to reason about.

For example, here is a regular inline class:

inline class Foo<T> {
  ...
  T doStuff() => methodReturnsT();
}

In this case, T is checked statically by the CFE and all is good and safe and sound.

What about this case:

inline class JSInteropFoo<T> {
  ...
  external T doOtherStuff();
  T doStuff() => methodReturnsT();
}

Now the T is checked in some cases and not others, some uses of the T are sound, some are not. A user may try to use a non-JS type, and now we have to check for that too. But of course, the type argument may only be used on non-external functions, and in that case a non-JS type is okay, so now we have to reason about that. These are just the complications I can think about from 2 minutes of thinking about this.

TLDR: Type arguments are really complicated, and they are tied inexorably to the Dart type system. Allowing them on JS interop types is not as clear cut as it seems. We may very well allow this at some point, but we really need to think through and ask ourselves if we are overpromising, or can we fully meet the guarantees for the Dart language w.r.t. type arguments on inline classes. If we can, then great, let's go for it, but if we can't then it could be a dangerous road to go down, for us and for our users.

devoncarew commented 1 year ago

For some flavor here, when I re-wrote the setup-dart action using the new static interop, the Promise class felt like using a very old version of Dart; not being any to annotate the js version of the future with the type that would come out of it was a little frustrating.

I gather that we don't want to make it seem like the type is something that can be relied on? Perhaps if we do have good docs that every JS type is really a JSAny, then having promise be Promise<T extends JSAny?> would be a good balance of things? It could serve as docs for what the promise was intending to return, but not over-promise in terms of the type that would come out?

jodinathan commented 1 year ago

Promise without the underlying type seems to be a huge step back.

On the one hand, then we get some documentation, but on the other hand it may lead users into a false sense of security.

As the author of js_bindings and an avid user of JS interop, I've never had this kind of problem.
I wonder if not having generics is a design decision based on the exception rather than the rule?

eernstg commented 1 year ago

Interesting discussion! I think it would be useful to keep several different perspectives in mind when the topic is in the area of 'deceptive types'.

(Warning: Long rant ahead, perhaps not useful, but I'm trying to promote a couple of concepts that seem relevant to this discussion. I think the considerations about inline classes are relevant even in the case where you might plan to model a JS promise using a class JSPromise at the same level as JSObject -- the discussion above doesn't reveal that particular distinction, as far as I can see.)

Three ways to be unsafe

First, any inline type is somewhat deceptive in the sense that we can cast it away. This means that we can obtain an added discipline when accessing a specific object by giving it an inline type, but we can always eliminate that protection by casting the expression to the underlying representation type.

Let's call this phenomenon cast-out unsafety.

Otherwise, if we "just say no" to casting (and take care at upcasts to top types) then we can help ourselves maintaining that extra discipline in the use of the object. A standard example could be IdNumber where we wish to use an int object to model an ID number of sorts. In particular, arithmetic operations like + and - are always a bug on an ID number, so we use an inline class to prohibit those operations. For example:

inline class IdNumber {
  final int rep;
  IdNumber(this.rep);
}

void main() {
  // Make sure we don't use `+` on some ID numbers:
  // Access them using the type `IdNumber`.
  var x = IdNumber(2), y = IdNumber(3);

  // We're now protected against additions.
  // x + y; // Compile-time error.
  // x + 1; // Compile-time error.
  // 1 + y; // Compile-time error.

  // But `IdNumber` is not safe!
  print((x as dynamic) + y); // Prints '5', no complaints.
}

The perspective here is that we're protecting ourselves against using a part of the behavior of the representation object which is inherently meaningless/buggy for the role played by that object. The same setup can be used to protect the representation object against operations which are technically supported by the given representation object, but they just happen to be inherently wrong for the given object, in the given application context. For instance, we might want to ensure that a given List doesn't get modified, using an inline class where mutating methods are omitted and other List methods are available.

So far, there is no type system mismatch, we're only working on regular Dart objects, and we only need to worry about cast-away unsafety.

One step further, we could manage a JSON value via a set of inline classes, and use that to ensure that we're only inserting null, a bool, a String, ..., a List<Object?> or a Map<String, Object?>. The inline classes could also facilitate navigation in that kind of structure, based on the assumption that we will never encounter any other type of object. This basically means that we're modeling a recursive type:

// Pretend that we have recursive type aliases.
typedef Json = Null | bool | num | String | List<Json> | Map<String, Json>;

However, we don't have this kind of type, and it would introduce a lot of new complexity into the type system if we were to support things like List<Json> <: List<List<Json>> <: List<List<List<Json>>> <: ... in their full generality.

Union types provides a minimal example of a type system feature that Dart doesn't have, and inline classes could arguably be used to model:

inline class Union2<X1, X2> {
  final Object? value;

  Union2._(this.value);

  bool get isValid => value is X1 || value is X2;

  R? split<R>({
    R Function(X1)? on1,
    R Function(X2)? on2,
    R Function(Object?)? onOther,
    R Function(Object?)? onInvalid,
  }) {
    var v = value;
    if (v is X1) return (on1 ?? onOther)?.call(v);
    if (v is X2) return (on2 ?? onOther)?.call(v);
    if (onInvalid != null) return onInvalid(v);
    throw "Invalid Union2<$X1, $X2> value: $value";
  }

  static Union2<X1, Never> in1<X1>(X1 value) => Union2<X1, Never>._(value);
  static Union2<Never, X2> in2<X2>(X2 value) => Union2<Never, X2>._(value);
}

int f(Union2<int, String> u) => u.split(
      on1: (i) => i,
      on2: (s) => s.length,
    )!;

void main() {
  // We can obtain union typed objects, pass them, and use them.
  var u1 = Union2.in1(1);
  var u2 = Union2.in2('Hello!');
  print(f(u1) + f(u2));

  // But we can easily violate the invariant.
  var badUnion = true as Union2<int, String>; // Cast don't care.
  print(badUnion.isValid); // 'false'.
  print(f(badUnion)); // Throws.
}

This introduces a new kind of unsafety; we could call it cast-in unsafety. The point is that we may strive to maintain a certain discipline (during constructor invocations, using members like isValid, etc) which involves invariants that the Dart type system can't express. The unsafety arises because we can simply skip the constructor invocations and other validation code in various ways, with a direct cast to the inline type as a prime example.

For JS interop, we have a third kind of unsafety; we could call it untypability unsafety. It is caused by the fundamental mismatch between the JavaScript semantics and the Dart type system: It is not necessarily possible to come up with a Dart type that describes the behavior of any given JavaScript object in a way that is both safe and useful. For example, the Dart type system has no way to model the situation where the prototype of a JavaScript object is mutated such that it gets a new method or changes the parameter list shape of an existing method, and so on.

In this case the problem is not that somebody put together a wrong pair of inline type and representation type, the problem is that there is no Dart type which is both sufficiently expressive and sufficiently safe.

Promises and type parameters

An object obtained from a JavaScript computation could be handled as a JSObject, or via an inline class MyJsThing ..., or via a generic inline class MyJsThing<X1, ... Xk> .... It's all about choosing a useful level of precision in the description that we're associating with the given JavaScript object during program execution.

Now return to the actual topic of this issue, promises with or without a type parameter (I'm assuming that we will use an inline class to model promises):

I get the impression that cast-out unsafety is a secondary concern, we always just need to be aware of that.

Cast-in unsafety may be the biggie, that is, the main worries could be that there will be too many situations where a given promise is considered to have a given type argument T, but it will actually resolve to an object whose type is not T. Also, this situation could arise because objects are handled using general types like JSObject rather than consistently accessing them using an inline type, and then we just don't know what the type argument should be, if we are even aware that a given JSObject is a promise at all. The problem could also arise already when the promise is received for the first time, and there might never be a location in code where a "correct" type argument can be chosen for that promise because we never know which type to choose.

The third kind, untypability unsafety, would arise if we simply can't trust a promise to resolve to an object of any particular non-top type, because they just do things that are too complex for a Dart type to capture. However, I wouldn't expect that.

If this analysis is reasonable then it might make sense to have a type argument on promises and document that it is an expression of the Dart-side expectation about the type of object that it resolves to, not a language-supported guarantee.

I can understand if you prefer to avoid the type argument, exactly because it takes a whole sentence to even begin explaining the meaning of a typing property which is a static expectation with no firm connection to run time. However, every static type given to a JSObject will have this issue, more or less.

johnpryan commented 1 year ago

I am trying out new JS interop and I ran into this issue pretty quickly. It feels unsafe to be casting the Future's return type:

    // Decode the audio file
    audioBufferSource.buffer = await _audioContext!
        .decodeAudioData(jsArrayBuffer)
        .toDart as web.AudioBuffer;

Are generic types supported? I would also like JSPromiseToFuture to have a generic type:

extension JSPromiseToFuture on JSPromise<T> {
  external Future<T> get toDart;
}
srujzs commented 1 year ago

@eernstg Sorry I missed your comment before - it's fantastic and captures a lot of the uncertainty around generics with interop inline types.

Cast-out unsafety always comes up when we're dealing with interop, so you're right that there isn't anything special here.

Cast-in unsafety is the recurring issue when it comes to generics and interop (JSArray has similar issues). When you return a JSPromise<T> from some external function, that T may be unsound, and you don't realize if it is until later (usually through a conversion to Future<T> followed by an await). If we converted to a Future and the converted Future catches type errors when awaited on, then the temporary unsoundness is reasonable. List.cast does something very similar.

Another complication here is that the JS backends and dart2wasm have different runtime type systems. In dart2wasm, Dart types are on a different heap than JS types, and dart:js_interop types themselves may not be discernible from one another at runtime as they're all essentially boxed external pointers. If we wrote JSPromise<double> and that double is actually a JS number, then that will eventually lead to a type error on dart2wasm. That's unlike external functions that return double, where we can convert a JS number to a Dart double at the boundary. We may be able to do something like that with JSPromise, but I think nested generics complicate that.

One option is to bound the type parameter as T extends JSAny?. You do mostly get soundness if you assume all Promises will return a JS value. That's a safe assumption but not always true, however - when we expose futureToPromise, you can create a Promise that returns a Dart type.

I agree that generics (imperfect for interop as they are) offer value, and they seem to be popular enough that I'm okay with adding them. It's definitely useful in the above case re: decodeAudioData, because as a user, you're not sure how the API expects you to use the result of a Future/Promise. There are wrinkles, though, and I don't think we're going to get a single solution that gives us a clean way of addressing all those wrinkles.

srujzs commented 8 months ago

JSArray and JSPromise are now generic extension types whose singular type parameter is bound to JSAny?. Since we've moved package:web to emit extension types instead, we've also added support for generics for those two types in package:web in https://github.com/dart-lang/web/commit/5b026ab7ca26fc6a28b1594f8f4c11aed2ec1b0c. Closing as completed.