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

[WIP] Type Iterator.from #9231

Closed
wants to merge 1 commit into from

Conversation

hurrymaplelad
Copy link
Contributor

@hurrymaplelad hurrymaplelad commented Nov 22, 2024

For #9230, which has context on Iterator.from().

I haven't gotten this working yet. Notes in comments.

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

from<+Yield,+Return,-Next>(source: $Iterable<Yield,Return,Next>): $Iterator<Yield,Return,Next>;
}

declare var Iterator: IteratorConstructor;
Copy link
Contributor Author

@hurrymaplelad hurrymaplelad Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how TypeScript declares Iterator.from, but Flow does not permit the re-declaration (error: name-already-bound). Try Flow.

Would be nice if they worked the same way. Nothing in #8989 about this yet.

@hurrymaplelad
Copy link
Contributor Author

Declaring from() as a static method on $Iterator could also work, but:

  1. It'd be too restrictive. Most places that accept an iterator don't care if it's using the builtin prototype.
  2. Flow doesn't allow interface statics. See Static methods on an interface #2048.

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@hurrymaplelad
Copy link
Contributor Author

Found a working path as part of #9232. Merged into that PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants