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.04k stars 1.55k forks source link

[dart2js] `Iterator` is confused with `NodeIterator` #53532

Open Zekfad opened 11 months ago

Zekfad commented 11 months ago

Coming from https://github.com/Zekfad/fetch_client/issues/9

There are new Iterator global class with Chrome 117+

JS compiler confuses it with DomIterator (which I believe is NodeIterator)

Following code throws in Chrome 117+ and works fine on older versions or on Safari/Firefox:

import 'dart:html';
import 'package:fetch_api/fetch_api.dart';

void main() {
  final e = Headers().entries();
  print(e is! DomIterator); // true
}
Zekfad commented 10 months ago

Related parts:

https://github.com/dart-lang/sdk/blob/96bd779a03d55eb934b8259edbcac3088f0f35c7/tools/dom/web_library_bindings.dart#L9326

https://github.com/dart-lang/sdk/blob/96bd779a03d55eb934b8259edbcac3088f0f35c7/sdk/lib/html/dart2js/html_dart2js.dart#L10751

srujzs commented 10 months ago

I might be missing some key details here, but doing a similar example prints false instead of throwing for me on dart2js. On Chrome, it looks like the headers iterator is a subtype of Iterator, but this isn't true on Firefox and Safari, so that's why there's a difference. I don't know why there's a discrepancy or if that's going to be fixed in the browsers.

I think the right solution is what you have: create a @staticInterop interface to interact with this instead. If we want to support this in dart:html, we should add a new class e.g. HeadersIterator that intercepts that type instead.

DomIterator intercepts all Iterators (hence the @Native value). I don't think NodeIterator is related to this issue unless it's a supertype/subtype of one of the aforementioned iterator types.

Zekfad commented 10 months ago

@srujzs thank for your reply.

So DomIterator is actually a general purpose JSIterator, not really part of DOM (for DOM there is NodeIterator which different interface).

JS have a "iterator protocol", and only recently Chrome got public Iterator class which exposes internal iterators. Currently DomIterator#next returns Object?, but I think it should have a strict type for iteration result, in my module fetch_api I'm trying to use more precise mappings.

So, maybe SDK should add iteration result type?

My implementation: https://github.com/Zekfad/fetch_api/blob/master/lib/src/iterator_result.dart Doc: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols

srujzs commented 10 months ago

Yeah I think it'd be useful for users to have a goto Iterator type that's generic to avoid having to recreate their own. Having an Object that you need a type for anyways isn't very useful in DomIterator. Since we're slowly transitioning away from dart:html to dart:js_interop and package:web, it might make sense to add that generic type to dart:js_interop.