Rx.Observable.from..take does not close iterator

[vadzim]

RxJS version: 5.0.0-beta.12

NodeJS version: 6.5.0

Code to reproduce:

function *G() {
    try {
        while ( true )
            yield 42
    } finally {
        console.log( `ok` )
    }
}
Rx.Observable.from( G() ).take( 1 ).subscribe( x => console.log( x ) )

Expected behavior:

42
ok

Actual behavior:

42

Additional information:
Following native code closes iterator properly:

function *G() {
    try {
        while ( true )
            yield 42
    } finally {
        console.log( `ok` )
    }
}
for ( let x of G() ) {
    console.log( x )
    break
}

Output:

42
ok

[vadzim]

.from..take has to behave like .fromEventPattern..take in following example:

Rx.Observable.fromEventPattern(
    cb => setTimeout( cb, 0, 42 ),
    () => console.log( `ok` )
).take( 1 ).subscribe( x => console.log( x ) )

Output:

42
ok

[benlesh]

Okay, so the more I think about this, the more I think it might be a feature request more than a bug.

1. The iterator interface only has a next method no throw() or return() methods.
2. A generator does have a return() and throw() as well as implementing the interator interface.
3. It might not be desirable for all users of RxJS to have all generators to be closed by take: Suppose someone wants to consume a few values from a generator without closing it.

[benlesh]

I think the real ask here is that we add spawn to RxJS 5. Which is designed to create an Observable from a generator function.

Observable.spawn(function* () { yield 1; yield 2; yield 3; });

[benlesh]

@vadzim a temporary solution to this would be to use finally to handle finalizing your generator.

const g = (function* () { /* stuff */ }());

Observable.from(g)
  .take(2)
  .finally(() => g.return())
  .subscribe(/* stuff */);

I hope that's helpful.

[vadzim]

Okay, my several thoughts.
1&2. Even for operator in ecmascript checks if return() method exists before calling it. So I think this cannot be a reason to not to support it.
3.1. Some users of ecmascript do not like that behavior of for operator, when it calls return() if you e.g. break it. I hear they said: "What if I want to continue looping?". The right anser is: Use .next() method and while cycle, control lifetime manually and do not use for cycle.
I believe that the most important thing here is consistency. That is, when someone used to some behavior then he must be able to expect the same behavior in similar cases.

I mean, if I can write the code

let i = 0
for ( const x of someGenerator() ) {
    if ( i++ === ) then
        break
    ...
}

I know that return() method will be called because of ES standard. Its another story whether I like this or not. So I used to see such a behavior. I will keep this in my mind designing my generators. I will expect it. So my generators may become unusable or buggy when once a time I decide to use them with RxJs.

Some fantastic example.
Suppose, we have a function that reads a file line by line.

function *readFile( fileName ) {
    const handle = openFileSync( fileName )
    try {
        while ( !eofSync( handle ) ) {
            yield readLineSync( handle )
        }
    } finally {
        closeFileSync( handle )
    }
}

Of course, this is not a real code. But its purpose is to demonstrate that finalizing resources can be very critical. I believe that if someone wants to control smth in some non-standard way, he must be able to do this, but he must do it manually, that is to call next() and return() manually and visibly.

3.2. As I understood, the spirit of Rx is to call function per every subscription. Not to proceed running function, but to call it once again. So when I look at calling generator I expect that its call is finite, that generator will be finished. Si I expect that return() will be called.

[benlesh]

It's important to note that with Observable.from(generate) the generator instance lives outside of the Observable, we wouldn't want the Observable automatically side-effecting and killing the generator, as others might be consuming it.

[vadzim]

If someone definitely wants not to close generator, he can hide return() manually.

const g = generator()
Observable.from( { next: g.next.bind( g ) } )

I believe that hiding return() method is used rarely than not hiding.

[vadzim]

Okay. I ask you to think about consistency one more time, please :)

[vadzim]

Otherwise, using Rx with generators can be like a hidden bomb. You can end up with bugs when you start to use Rx instead of for loops.

[benlesh]

@vadzim I'm still looking into it.

I'm going to model it after how for..of consumes generators. Which, at least in Chrome, and I think in the TC39 spec (although I'm having a hard time locating the specifics, but I can bug @jhusain), does indeed finalize the generator... For example:

const G = function *() {
  try {
    let i = 0;
    while (true) {
      console.log('yielding ' + i);
      yield i++;
    }
  } catch(err) {
    console.log('error block', err);
  } finally {
    console.log('finalized');
  }
}

const g = G();

for(let x of g) {
  console.log('> ' + x);
  if (x === 1) {
    break;
  }
}

/**
"yielding 0"
"> 0"
"yielding 1"
"> 1"
"finalized"
*/

Although I do find it odd behavior because the for..of loop doesn't necessarily "own" the generator, I'll go with that just for consistency's sake.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.