Skip to content

[dart2js] Iterator is confused with NodeIterator #53532

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

Open
Zekfad opened this issue Sep 14, 2023 · 4 comments
Open

[dart2js] Iterator is confused with NodeIterator #53532

Zekfad opened this issue Sep 14, 2023 · 4 comments
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. web-dart2js web-libraries Issues impacting dart:html, etc., libraries

Comments

@Zekfad
Copy link

Zekfad commented Sep 14, 2023

Coming from Zekfad/fetch_client#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
}
@lrhn lrhn added web-dart2js area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. labels Sep 14, 2023
@sigmundch sigmundch added the web-libraries Issues impacting dart:html, etc., libraries label Sep 14, 2023
Zekfad added a commit to Zekfad/fetch_api that referenced this issue Oct 6, 2023
due to bug in sdk: dart-lang/sdk#53532
this class was getting cast to `DomIterator`
resolves Zekfad/fetch_client#9
@Zekfad
Copy link
Author

Zekfad commented Oct 9, 2023

@srujzs
Copy link
Contributor

srujzs commented Oct 11, 2023

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
Copy link
Author

Zekfad commented Oct 11, 2023

@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
Copy link
Contributor

srujzs commented Oct 11, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web-js Issues related to JavaScript support for Dart Web, including DDC, dart2js, and JS interop. web-dart2js web-libraries Issues impacting dart:html, etc., libraries
Projects
Status: Backlog
Development

No branches or pull requests

4 participants