defer generator resumption until start of frame

[mbostock]

Generators now resume at the start of the next animation frame, but before any other cells compute. Previously, generators resumed immediately (in a microtask) on the first value, and then (roughly) concurrently with other variables on the next frame for subsequent values. This new approach provides more predictable and consistent semantics.

For example, this now does a nice fade-in:

{
  const div = htl.html`<div style="background-color: white; transition: background-color 1000ms ease;">hello`;
  yield div;
  div.style.backgroundColor = "red";
}

See this (internal only) notebook for more explanation: https://observablehq.com/d/e6fff2ad18deec3c

Ref. https://github.com/observablehq/feedback/issues/243

Previously #318 #220 #108.

[mbostock]

This isn’t doing quite enough error handling. I’m going to push a followup commit shortly.

[mbostock]

Okay, this is ready for review @visnup! 🙏

[mbostock]

I walked back this part:

When a generator yields a value (even if the value is a promise), that value immediately becomes the cell’s apparent value for the rest of the notebook. This means that another cell referencing this value will begin computing (i.e., show a gray border indicating active computation) and will wait until the promise resolves rather than seeing the stale value.

It turns out this could lead to a cell never evaluating because its value is always considered invalidated (the version test always fails). For example, consider these two cells:

i = {
  for (let i = 0; true; ++i) {
    yield Promises.delay(1000, i);
  }
}

j = {
  await Promises.delay(100);
  return i;
}

Previously in this PR (prior to 88dda79), the promise of i was available immediately when it yielded. This would cause cell j to go into a pending state for 1s and then run, but by the time the value of j resolved after the 100ms await, the new promise of i (albeit again pending) had already yielded, and so j was considered invalidated and would not display. This would continue forever.

The main thing I’d like to focus on here is when the generator resumes, so I walked back the more aggressive change to expose the yielded value sooner; now, as in main, the yielded value is only exposed when it resolves. There are some alternate approaches we could consider if we want to be more sweeping in our changes here, but I figure it’s best to be conservative.

[mbostock]

Okay, I’ve tidied up and commented the code a bit. This is now as conservative as I can make it, which I think is good. Aside from the tidying, the real difference is the switch from

runtime._compute().then(recompute)

to

runtime._compute().then(() => runtime._precompute(recompute))

The difference being that with the latter, the recompute is deferred until the start of the second animation frame, rather than concurrently with the first animation frame.

[mbostock]

The magic number 3 comes from the depth of the promise chain required to pull the next value from a generator, assuming that the generator is synchronous. Look at recompute:

https://github.com/observablehq/runtime/blob/0d0104fbe78a8dbdda98c1b1c8b6415e3b89d53d/src/runtime.js#L300-L315

Which calls compute:

https://github.com/observablehq/runtime/blob/0d0104fbe78a8dbdda98c1b1c8b6415e3b89d53d/src/runtime.js#L291-L298

Compute constructs a promise that synchronously resolves with the result of generator.next (depth 1), chained with a .then to extract the {done, value} (depth 2), chained with a .then to call onfulfilled which callls postcompute to set the outputs (depth 3). That is the shortest possible depth to pull the next value of the generator (since the generator can be asynchronous, or synchronous and yield a promise).

So, the 3 is in a sense arbitrary, but I don’t know how else to express this constraint, and I also don’t think it’s likely to ever change.

[visnup]

Ok, I tried:

1. increasing the chain by inserting an arbitrary .then(a => a) in compute and tests fail
2. reducing from 3 to 2 and the tests fail

so, I'm happy that the tests will catch issues with the 3 being too small. it of course, may be too large and we won't be able to detect that very easily though.

[visnup]

I guess there could be some test that decreases the magic 3 to a 2 and expect a failure, but that seems like a super awkward test to write.

[mbostock]

Oh, I figured out the off-by-one. It’s because Promise.resolve() is already resolved (hence depth 0), but Promise.resolve().then(() => {}) is not resolved (hence depth 1).