repeaterjs / repeater

The missing constructor for creating safe async iterators
https://repeater.js.org
MIT License
459 stars 12 forks source link

Off-by-one issue with next parameter and push promise #31

Closed brainkim closed 5 years ago

brainkim commented 5 years ago

Currently we resolve push results to the value passed to next. However, the repeater implementation is off-by-one in the sense that the first push call resolves to the first value passed to next, the second push to the second value and so on. This behavior differs from generators, which will resume yield with the value passed to the next call to next, dropping the first value passed to next entirely. To mimic async generators, we should be resolving the first push to the second value passed to next, and so on.

In other words, the following test is wrong:

  test("pushes resolve to value passed to next", async () => {
    let push: (value: number) => Promise<number | void>;
    const repeater = new Repeater(async (push1) => {
      push = push1;
    });
    const next1 = repeater.next(-1);
    const push1 = push!(1);
    await expect(push1).resolves.toEqual(-1);
    await expect(next1).resolves.toEqual({ value: 1, done: false });
    const push2 = push!(2);
    await expect(repeater.next(-2)).resolves.toEqual({ value: 2, done: false });
    await expect(push2).resolves.toEqual(-2);
  });

The correct version of this test should be:

    let push: (value: number) => Promise<number>;
    const repeater = new Repeater(async (push1) => {
      push = push1;
    });
    const next1 = repeater.next(-1);
    const next2 = repeater.next(-2);
    const next3 = repeater.next(-3);
    const next4 = repeater.next(-4);
    const push1 = push!(1);
    const push2 = push!(2);
    const push3 = push!(3);
    const push4 = push!(4);
    await expect(next1).resolves.toEqual({ value: 1, done: false });
    await expect(next2).resolves.toEqual({ value: 2, done: false });
    await expect(next3).resolves.toEqual({ value: 3, done: false });
    await expect(next4).resolves.toEqual({ value: 4, done: false });
    await expect(push1).resolves.toEqual(-2);
    await expect(push2).resolves.toEqual(-3);
    await expect(push3).resolves.toEqual(-4);
    await expect(
      Promise.race([
        push4,
        new Promise((resolve) => setTimeout(() => resolve(-1000), 1)),
      ]),
    ).resolves.toEqual(-1000);