WICG / local-font-access

Web API for enumerating fonts on the local system
https://wicg.github.io/local-font-access
Apache License 2.0
75 stars 16 forks source link

FontIterator name is not good #31

Closed domenic closed 3 years ago

domenic commented 3 years ago

Consider the following code sample:

const fontManager = navigator.fonts;
const fontIterator = await fontManager.query();
const fontIterator2 = fontIterator[Symbol.asyncIterator]();

const { value, done } = await fontIterator2.next();

In JavaScript, "iterator" has a very specific meaning, which is an object with a next() method which returns a { value, done } tuple. (Or a promise for such a tuple, in the case of async iterators.)

That is, per the JavaScript spec and ecosystem's definitions, fontIterator2 is an iterator. fontIterator is not an iterator; it is an iterable.

So the interface would be better named FontIteratable or Fonts or something like that. (Although IMO it could probably be eliminated entirely... I'll file another issue for that.)

oyiptong commented 3 years ago

I'm quite confused about iterables vs iterators.

As implemented, navigator.fonts.query() returns an iterable object.

const iterable = navigator.fonts.query()
> {Symbol(Symbol.asyncIterator): ƒ}

However, when creating an interator out of it yields the iterator as defined, i.e. FontIterator:

const iterator = iterable[Symbol.asyncIterator]()
> FontIterator {}

This seems to have the next() method, which returns a promise returning the { value, done } tuple.

As such, the interface name seems appropriate, but fontManager returns an iterable.

Does the comment still stand RE: FontIterator?

domenic commented 3 years ago

Ah, I see. I got confused because the spec and Chromium don't match each other. So yes, it does sound like in Chromium the FontIterator object is an iterator, not an iterable.

To fix the spec/implementation mismatch, I see a few steps:

  1. Decide on the API you want programmers to use. This is partially what #33 is about.

  2. Specify that using Web IDL + proper prose. This is something myself and @jakearchibald can help with.

  3. Implement that in Chromium. This will be a bit trickier than usual because Chromium's bindings code does not have proper async iterator support, so you'll need hacks like an FontIterator interface with a next() method, which will not exist in the spec. (In spec-land, Web IDL will automatically generate your iterators whenever you use async iterable<>.)

inexorabletash commented 3 years ago

The API has changed so there is no explicit iterator - an array is returned synchronously.