dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.24k stars 1.57k forks source link

How to use for loop with staticInterop? #48327

Open jodinathan opened 2 years ago

jodinathan commented 2 years ago

I am trying to use staticInterop to implement array interop in JS here

From what I understand, to be able to use for loops on instances the class need to implement Iterable with its members, however, using staticInterop we can't have members.

What can I do to be able to iterate the array with regular for loop?

kevmoo commented 2 years ago

@sigmundch – can you direct?

sigmundch commented 2 years ago

There are two angles on this question worth discussing:


What to do about arrays?

/cc @srujzs @rakudrama @rileyporter

@jodinathan - I'm noticing that you are trying to use JSinterop to wrap javascript arrays, but I'd like to note that the default List implementation in both DDC and dart2js use directly native arrays. Is there a functionality gap you need to address?

Our goal with static-interop is to make it possible to write most of the browser bindings outside the dart sdk (e.g. dart:html, svg, indexed-db, etc), but not necessarily to replace what we do with types exposed in our core libraries (like numbers or arrays).

Unfortunately there are a few array-like APIs in the browser bindings too (e.g. NodeList), maybe we need to handle those separately?


What to do about other language features that are based on interfaces?

@leafpetersen - curious on your take on this. So far we have used queries like "implements X" to enable certain features, and I wonder if we want to revisit this in the context of static-views/extension-types.

This issue refers to the specific case of for-in loops and Iterable. Ironically, the approach we used to have for for-in loops long ago, would have worked out of the box for static-views today. Back then for-in loops supported iteration on any type that had an iterator property (this was changed about 3 years ago: https://github.com/dart-lang/language/pull/291)

jodinathan commented 2 years ago

@sigmundch there are 11 classes that have the iterable type and another 11 classes that use the maplike type.
From what I see, most are from modern IDL specs, so I think we can conclude that any new stuff will probably have use of those types.
I think it is better to have something like views or extension-types to implement the needed members so we can have full use of Iterable and Map features from Dart.

sigmundch commented 2 years ago

One idea that may be worth exploring is to use wrappers only at the time of iteration. Traditionally for JS-interop we don't want to wrap every object returned from JSinterop APIs because it becomes really expensive, however we could consider using a short-lived wrapper for the purpose of iteration. In this case, say if you were providing NodeList as an iterable, I could imagine the following:

@JS()
@staticInterop
class NodeList { //* See note below why this is not generic
  ...
}

class NoteListIterable extends ListBase<Node> {
   final NodeList list;

   Node? operator[](int index) => list[key];
   void operator[]=(int index, Node? value) => list[key] = value;
   int get length => list.length;
   set length(int value) => list.length = value;
}
...

method() {
   var list = divElement.childNodes; // e.g. js_binding API to read childNodes
   for (var child in NodeListIterable(list)) {
      print(child);
   }
}

*PS: I noticed that your example above declares a generic JSArray class, but unfortunately that's not supported under JSInterop. Unfortunately we don't report a compile-time error today. Long story short, this is not supported because those types get created in JS, the will not have any tagging of the type parameter, as a result, we wont be able to reify it.

rakudrama commented 2 years ago

We should be thinking in terms of supporting iterating JavaScript objects that support the JavaScript iterable protocol. Many array-likes already do this.

If the wrapper is generic, I think we only need one, like this, but hopefully with a nicer name:

  for (var child in DartIterableViewOfJavaScriptIterable<Nodet>(list)) ...

Perhaps we would like another for the array-like classes that we would like to index like NodeList - DartListViewOfJavaScriptArrayLike<ElementType>.

These wrappers provide the Dart interface and Dart methods built on, but an array-like (with indexing via property accesses) cannot be easily passed back to JavaScript without unwrapping.

Do we also want to support full iteration in the other direction, so that Dart Iterable implementations also automatically support the JavaScript protocol? None of the methods in the Dart protocol (get:iterator, moveNext() and get:current) have type parameters, so it should be possible if the compiler gave each Dart Iterable class a method for [Symbol.iterator].

jodinathan commented 2 years ago

@sigmundch

*PS: I noticed that your example above declares a generic JSArray class, but unfortunately that's not supported under JSInterop. Unfortunately we don't report a compile-time error today. Long story short, this is not supported because those types get created in JS, the will not have any tagging of the type parameter, as a result, we wont be able to reify it.

But the generic part is important to inference, autocomplete etc.

Instead of extending the JsArray I could source generate all extension methods to provide the correct type, just dunno if that can harm the bundle size. I thought tree-shaking would blast away everything not used, but reading the generated JS somethings are still there, like all typedef. (hadn't given this much time, so just speculations right now)

sigmundch commented 2 years ago

But the generic part is important to inference, autocomplete etc.

Completely agree, it's just a hard tradeoff we have at the moment: if something is external and created outside the scope of Dart, then we can't assume it will be populated with Dart-specific information (like type arguments.)

So far we treated internal Dart objects as controlled by our runtime (e.g. that's how regular List works, even though it is implemented by JSArray under the hood), and external JavaScript objects as not-controlled and not having reified generics.

Technically we could go with a middle ground: reify type arguments on external JavaScript objects that are fully created and controlled by the Dart application. Unfortunately it wouldn't be sound if we expect JavaScript code to be able to make updates to such objects, unless we introduce additional checks on the Dart side for every API that uses those type arguments.

jodinathan commented 2 years ago

Technically we could go with a middle ground: reify type arguments on external JavaScript objects that are fully created and controlled by the Dart application.

I think this is the best approach.

Unfortunately it wouldn't be sound if we expect JavaScript code to be able to make updates to such objects, unless we introduce additional checks on the Dart side for every API that uses those type arguments.

Maybe do some checks in DDC?
I think in cases where JS changes the objects, probably the dev is already expecting that to happen and will work with that in mind.

jodinathan commented 2 years ago

maybe we could test the extension-types in this case @sigmundch ?

sigmundch commented 2 years ago

I think in cases where JS changes the objects, probably the dev is already expecting that to happen and will work with that in mind.

This is tricky, because an unsound issue in one place can lead dart2js to generate invalid code from optimizations, making it very difficult to debug.

sigmundch commented 2 years ago

maybe we could test the extension-types in this case @sigmundch ?

Not sure I understand what you mean :), could you clarify?

For context, that flag is in early stages and unfortunately not in a state we can use. The feature it represents is still being designed (it is also referred to as static views, here is a recent draft of the proposal). For what it's worth, our goal is to jump into it as soon as it is in a state that we can use :). In fact, the current work with @staticInterop is a step in that direction. It is a way for us to expose the style of capabilities we will have at that point, but using today's existing syntax and language support.

jodinathan commented 2 years ago

Not sure I understand what you mean :), could you clarify?

I thought we could statically extend a type.

In our case, we could statically add Iterable to the NodeList and use regular for-in loop type without NodeList having to extend anything

jodinathan commented 2 years ago

I've added a wrapper and now is possible to use for (final node in myNodeList.toList()).

Works until we have static-views or something like that to really fix the issue

sigmundch commented 2 years ago

I thought we could statically extend a type.

In our case, we could statically add Iterable to the NodeList and use regular for-in loop type without NodeList having to extend anything

I see, currentlystatic-views don't let you add interfaces to a type. Note that static views are like a wrapper that is erased at runtime, as such, information will be forgotten in any context where the original type is not available. As an example:

NodeList myNodeList = ...;
myNodeList.forEach(print); // OK: forEach is found statically in the static-view
Iterable iterable = myNodeList;
iterable.forEach(print); // ERROR: forEach is not found virtually

The second forEach is a virtual call and as such it can only be supported on values that implement Iterable. Because NodeList is a static-view, the underlying type doesn't implement Iterable.

To really support virtual calls you'd need a real wrapper and not an erasable zero-cost wrapper like static-views do.

That said, I wonder if there is a a way to extend static-views to reduce the need for real wrappers. Your use case is interesting because the use of the wrapper is immediately happening in the for-in. This is one reason I wonder if for-in could be redesigned to consider working not just on Iterable but also on types that adhere to certain API.

jodinathan commented 2 years ago

I thought with static views we could do something like:

@JS('Array')
view JsArray<E> implements Iterable<E>, Iterator<E> {
  Iterator<E> get iterator => this;

  bool moveNext() => !next().done;

  external E operator[](int index); // or use js_util
  // other iterable methods that are external
}

@JS()
@staticOperator
class Document {}

typedef NodeList = JsArray<Node>;

extension AdvDocument on Document {
  external NodeList get nodes;
}

then the usage is just static:

final nodes = document.nodes;
final canMoveNext = nodes.moveNext();

print(canMoveNext);

which would compile to a complete static dynamic-like calls:

const nodes = document.nodes;
const canMoveNext = !nodes.next().done;

print(canMoveNext);

then we can use for-in because we know at compile time that the view implements Iterable

for (final node in document.nodes) {}

(I guess the only problem there would be that the Dart and JS Iterable protocol are different but I also guess that we could adjust to it)

however, this would not be possible:

void foo(dynamic bar) {
  if (bar is JsArray) {  
    // we can't know if bar is JsArray at runtime because it doesn't exist
  }
}