`.take` should close the underlying iterator as soon as the final item is taken

[bakkot]

Right now if you do

let bounded = iter.take(3);
bounded.next();
bounded.next();
bounded.next();

this will not close iter. You have to call bounded.next() again to actually close out the underlying iterator. That seems surprising to me.

[ljharb]

I’d expect the iterator not to be closed until the done result was observable. Isn’t that the current behavior?

[bakkot]

Observable to whom? The take helper knows it's done with the underlying iterator after n calls to next; that seems the natural time to close it.

[ljharb]

To the caller i guess? I guess I’d expect iter to be closed here, but not bounded yet.

[bakkot]

iter is the thing I'm talking about, yes.

To be more precise, I'm talking about when iter.return() will get called - is it during the third call to bounded.next, at which point bounded knows it is done with iter, or the fourth, at which point the consumer of bounded knows that?

[ljharb]

I would still expect the fourth, but I definitely see your argument for the third.

[Jamesernator]

I would strongly argue that it shouldn't, in particular in the case of async generators closing the iterator may perform potentially long cleanup work. This could mean that the final element is delayed rather than the delay occuring at the end of the loop as expected.

i.e. You would wind up with a situation like:

async function* iter() {
    try {
        // emit items
    } finally {
        await longCleanup();
    }
}

for await (const item of iter().take(3)) {
    console.log(item);
}

Behaving like:

print item 1
print item 2
expensive cleanup, possibly long delay
print item 3

The current behaviour is more like an explicit break would already work today, i.e. cleanup happens at the end of the loop not before an item:

let counter = 0;
for await (const item of iter) {
    console.log(item);
    counter += 1;
    if (counter === 3) {
        break;
    }
}

[Jamesernator]

Actually this gets weirder, given cleanup might throw an error or produce other behaviour, like:

function* iter() {
    try {
        for (let i = 0; ; i++) {
            yield i;
        }
    } finally {
        throw new Error("cleanup failed");
    }
}

for (const item of iter().take(3)) {
    console.log(item);
}

What should the above print? Like it should probably still print 1,2,3 as errors don't occur until after the items have been consumed, but then you need to store the error (which has already happened) and throw it later, which seems iffy.

This makes this especially dangerous with things like locks or similar:

async function* withResource() {
    try {
        // emit items
    } finally {
        await releaseLock();
    }
}

for await (const item of iter().take(3)) {
    // Is it really safe to release the lock while BEFORE
    // processing the third item?
    console.log(item);
}

[devsnek]

It should be closed on the call that returns done: true right? I don't think we should be making these too clever, that will just create weird footguns like James points out.

[michaelficarra]

This doesn't seem worth it. Closing.