Spec from() static converter

[keithamus]

This fixes #159 by specifying the semantics of from(), while also then using that algorithm in takeUntil().

---

Preview | Diff

[domfarolino]

Thanks so much for adding this! It's been on my plate but alas I've not gotten to it yet. One thing is that we won't be able to land this without WPTs, so if you feel like writing those that would be great, otherwise I'm happy to do so, just FYI I am OOO for a bit coming up soon.

[petamoriken]

The ReadableStream.from spec is in the process of being changed to use the newly introduced async iterable WebIDL type, so Observable.from should be similarly specified.
whatwg/streams#1310

[domfarolino]

I've started to revamp this a little bit to make sure the really subtle semantics of [Symbol.iterator] method access and how we deal with various values of that method's record match (a) the existing tests as well as (b) a bunch of new ones I'm adding in https://chromium-review.googlesource.com/c/chromium/src/+/5824955.

[benlesh]

Most of this looks okay, just a couple of comments.

We're also missing ArrayLike. Which is supported in Array.from() et al.

ArrayLike should handle any object with a length and numeric keys/indices.

It's probably not a big deal, and we can add it later, I suppose, but I think that Iterator.from() handles it? (I'll have to double check)... Array.from() definitely does.

[benlesh]

It's important in here to make sure that we add a check and call to the async iterator's return() method, if it has one. Because if it's a generator, I'd expect gen.return() to be called so it hits the finally block:

function sleep(ms) {
  return new Promise(resolve => setTimeout(resolve, ms));
}

async function* infiniteGenerator() {
  let n = 0;
  try {
    while (true) {
      await sleep(100);
      yield n++;
    }
  } finally {
    console.log('this must be called!');
  }
}

const ac = new AbortController();

Observable.from(infiniteGenerator())
  .subscribe(console.log, { signal: ac.signal });
  
setTimeout(() => {
  ac.abort(); // should cause log of "this must be called!"
}, 333);

[benlesh]

Similar to what I said above about async generators. This has to accommodate generators.

function* infiniteGenerator() {
  let n = 0;
  try {
    while (true) {
      yield n++;
    }
  } finally {
    console.log('this must be logged!');
  }
}

const ac = new AbortController();

Observable.from(infiniteGenerator())
  .subscribe((n) => {
    console.log(n);
    if (n === 4) ac.abort(); // Should force the logging of "this must be logged!"
  }, { signal: ac.signal })

[domfarolino]

@benlesh Can you make a WPT PR with tests for the cases you're describing please?

[benlesh]

@domfarolino :) It was in-process while you were typing that: web-platform-tests/wpt#47874

[domfarolino]

Quick review just to document the current state of things. I think we're just about ready modulo the async iterator stuff, which I think needs a little work I've outlined below.

[domfarolino]

A few more things I've noted. I'll work on these and try and fix most of them today.

[domfarolino]

See #125 (comment) where I think we want to reverse this, perhaps requiring special handling to ensure that Strings can be converted into Observables like an Iterable would be.

[benlesh]

Nah... Observable.from(variable) where variable is a string, is almost always a mistake. I think that Iterator.from et al, don't allow string. I think the wording is "non-primitive iterables" or some such thing. I'd have to research. But we don't want to allow string, because there's just not a great use case for it, IMO. They could still do it like Observable.from(someString[Symbol.iterator]()) if they REALLY wanted to.

[domfarolino]

I think that Iterator.from et al, don't allow string

No, see #125 (comment). Regardless, we could start by throwing in this case and eventually relax it if we want, but it seems like for consistency it would be weird if Iterables convert strings properly, Observables convert Iterables properly, but Observables do not convert strings like Iterables.