Improve "no-constant-condition" rule for generators

[Turbo87]

This topic has been discussed already in #1299 and #2375 but per #2375 (comment) I'm opening a new issue for this.

#2375 (comment) has introduced a proposal for handling this which seems pretty reasonable:

Bad:

while(true) {
    //...
}

Bad:

function*() {
    while(true) {
        //...
    }
    yield 'foo';
}

Good:

function*() {
    while(true) {
        yield 'foo';
    } 
}

It should be noted that the same should also apply to async functions which have roughly similar semantics. Since both generators and async/await are starting to be adopted a lot more this topic should be revisited.

/cc @platinumazure

[platinumazure]

So basically, my take on it is this:

• The purpose of no-constant-condition is to avoid useless conditionals and infinite loops.
• However, while(true) in a generator, as long as at least one code path (or all code paths? not sure) has a yield statement in it, is not actually an infinite loop and serves a different purpose, basically implementing an infinite generator or coroutine that does not actually do an infinite loop.
• I assume similar logic holds for await within an async function, but I will defer to more knowledgeable folks on that.
• Our last decision on the issue was that generators and async/await had not yet shown themselves to be mainstream and while(true) in generators especially had yet to prove itself. Since then, async/await is in ES2017 and while(true) in generators is definitely becoming more common.

So I think this merits revisiting, and I will 👍 this enhancement.

[not-an-aardvark]

I assume similar logic holds for await within an async function, but I will defer to more knowledgeable folks on that.

This is incorrect -- when generator functions use yield, control flow returns to the code that invoked the generator. However, when an async function uses await in an infinite loop, the returned Promise never fulfills.

[Turbo87]

@not-an-aardvark that is a good point. I guess we should limit this to just generator for now then and discuss async functions at a later point.

[Andarist]

@not-an-aardvark
how so? could u post a simple snippet with the explanation? I think thats not true and ive just tested it in chrome

function delay(ms) {
  const promise = new Promise(resolve => {
    setTimeout(() => resolve(), ms)
  })

  return promise
}

async function backoff(i = 0) {
  while (true) {
    console.log(i++)
    await delay(i * 1000)
  }
}

backoff()

[Turbo87]

@Andarist the promise returned from backoff() will never resolve which doesn't seem like good API design 🤔

[Andarist]

Hm. I guess... although i dont see this as such bad design if somebody really needs an infinite task

[platinumazure]

@Andarist My take on that is, just like for infinite loops, no-constant-condition can't know when you mean to have an infinite task or not so your best bet is to use // eslint-disable-line comments on those cases.

Infinite generators, on the other hand, are a completely different animal altogether and I think we should consider enhancing no-constant-condition to allow an option to exclude infinite loops in generator functions (as long as they have yield statements in them).

So with all that said, I think I lean towards an exception for generators only.

[benjamingr]

while loops in async functions can serve a purpose:

// background worker
(async () => {
  while(true) {
    await delay(15 * 1000);
    updateStockPrices();
  }
})();

That is, global "background work". That said, the above code can be rewritten as:

setInterval(updateStockPrices, 15000);

On the other hand - async generators should definitely be supported (functions that are both async and generators).

[not-an-aardvark]

@eslint/eslint-team We have 3 👍s for this proposal, so we just need a team member to champion it so we can accept it.

[platinumazure]

I'll champion if we're okay with restricting this to generators for now (and if @Turbo87 is willing to modify the original post and issue title accordingly to avoid confusion).

That means we would need one more 👍 from the team since I'm currently one of the three.

[Turbo87]

@platinumazure I've updated the title and post. Thanks for the reminder :)

[VictorHom]

If possible, can I take on this issue?

[kaicataldo]

@VictorHom Please feel free - and thank you!