observable icon indicating copy to clipboard operation
observable copied to clipboard

Observable#from() @@iterator side effects

Open domfarolino opened this issue 11 months ago • 1 comments

Imagine the following:

const iterable = {
  get [Symbol.iterator]() {
    console.log('Symbol.iterator getter');
    return function () {
      console.log('Symbol.iterator implementation');
      return {
        next () {
          console.log('next() being called');
          return {value: undefined, done: true};
        }
      };
    }
  }
};

const source = Observable.from(iterable);
source.subscribe();
source.subscribe();

What would you expect the console to look like? I think we have two options:

First option

Symbol.iterator getter
Symbol.iterator implementation
next() being called
Symbol.iterator implementation
next() being called

This option basically means that:

  1. Observable.from() calls the [Symbol.iterator] getter initially to make sure iterable is exactly that
  2. The [Symbol.iterator] function is stored in the Observable
  3. It is called on each subscribe() invocation

Further this means that if [Symbol.iterator] is re-assigned in between from() and subscribe(), the old value will be called on subsequent subscriptions:

const iterable = {
  [Symbol.iterator]() {
    console.log('Symbol.iterator implementation');
    return {
      next () {
        console.log('next() being called');
        return {value: undefined, done: true};
      }
    };
  }
};

const source = Observable.from(iterable);
iterable[Symbol.iterator] = () => {
  throw new Error('custom error');
};

source.subscribe();
source.subscribe();

would output:

Symbol.iterator implementation
next() being called
Symbol.iterator implementation
next() being called

The new overridden [Symbol.iterator] implementation never gets called for the Observable created before the assignment.

Second option

Symbol.iterator getter
Symbol.iterator getter
Symbol.iterator implementation
next() being called
Symbol.iterator getter
Symbol.iterator implementation
next() being called

This option basically means that:

  • Observable.from transiently tests the existence of the Symbol.iterator function (invoking the getter), ensuring the input object is indeed an iterable
  • Stores a reference to the input object in the Observable returned by from(), such that a fresh acquisition of the iterator function is grabbed on every single subscription

This allows the [Symbol.iterator] method that gets invoked, to change (via reassignment, for example) across subscriptions to the same Observable.

I slightly prefer the first option above, as I think it is less prone to error and change, but I am unsure which is more purely idiomatic. For example, is it "bad" for the web to keep alive a [Symbol.iterator] implementation that a web author as re-assigned, just because it was captured by an old-enough Observable? I'm curious if @bakkot has any thoughts on this.

domfarolino avatar Mar 07 '24 17:03 domfarolino

First option sounds right to me. I wouldn't worry too much about the case where the value of an object's [Symbol.iterator] field changes over time - that's a very strange thing to be doing, and I think any authors of such code ought to expect that consumers which are going to do iteration multiple times may be holding on to the old value.

There's no direct analogy in JS, because JS never consumes an iterable more than once, but there is something pretty similar: during iteration (including async iteration) the next method from iterators is cached, which means that once iteration has begun, any changes to the next method are not reflected until iteration is completed. That seems similar-ish to this case.


On the other hand, holding on to the value of [Symbol.iterator] requires keeping more stuff around (since you need to keep the object as well, so you can invoke the [Symbol.iterator] method with the correct receiver). And if the extra lookup is cheaper than keeping the method around, that seems fine too. Like I say, I don't think we ought to care too much about what happens in this case because well-behaved code unlikely to be in this case.

bakkot avatar Mar 07 '24 17:03 bakkot