From d7afbb01f15845847224c68572e9f686322b4093 Mon Sep 17 00:00:00 2001
From: Mike Bostock <mbostock@gmail.com>
Date: Sat, 12 Oct 2024 17:52:04 -0400
Subject: [PATCH] fix stale computation with no inputs

---
 src/runtime.js               |   8 +--
 test/variable/define-test.js | 105 ++++++++++++++++++++++++++++++++++-
 2 files changed, 107 insertions(+), 6 deletions(-)

diff --git a/src/runtime.js b/src/runtime.js
index 2262234..63f0215 100644
--- a/src/runtime.js
+++ b/src/runtime.js
@@ -230,13 +230,11 @@ function variable_compute(variable) {
   // Wait for the previous definition to compute before recomputing.
   const promise = variable._promise = variable._promise
     .then(init, init)
+    .then(define)
     .then(generate);
 
-  // If the variable doesn’t have any inputs, we can optimize slightly.
   function init() {
-    return inputs.length
-      ? Promise.all(inputs.map(variable_value)).then(define)
-      : new Promise(resolve => resolve(definition.call(value0)));
+    return Promise.all(inputs.map(variable_value));
   }
 
   // Compute the initial value of the variable.
@@ -262,7 +260,7 @@ function variable_compute(variable) {
       }
     }
 
-    return variable._definition.apply(value0, inputs);
+    return definition.apply(value0, inputs);
   }
 
   // If the value is a generator, then retrieve its first value, and dispose of
diff --git a/test/variable/define-test.js b/test/variable/define-test.js
index 5f48083..472c75f 100644
--- a/test/variable/define-test.js
+++ b/test/variable/define-test.js
@@ -1,6 +1,6 @@
 import {Runtime} from "@observablehq/runtime";
 import assert from "assert";
-import {valueof} from "./valueof.js";
+import {sleep, valueof} from "./valueof.js";
 
 it("variable.define(name, inputs, definition) can define a variable", async () => {
   const runtime = new Runtime();
@@ -532,3 +532,106 @@ it("variable.define does not report stale rejections", async () => {
   assert.deepStrictEqual(values, []);
   assert.deepStrictEqual(errors, ["error2"]);
 });
+
+it("variable.define waits for the previous value to settle before computing", async () => {
+  const runtime = new Runtime();
+  const main = runtime.module();
+  const log = [];
+  let resolve1;
+  let resolve2;
+  const promise1 = new Promise((r) => (resolve1 = r));
+  const promise2 = new Promise((r) => (resolve2 = r));
+  const foo = main.variable(true);
+  foo.define("foo", [], () => {
+    log.push("1 start");
+    promise1.then(() => log.push("1 end"));
+    return promise1;
+  });
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start"]);
+  foo.define("foo", [], () => {
+    log.push("2 start");
+    promise2.then(() => log.push("2 end"));
+    return promise2;
+  });
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start"]);
+  resolve1();
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start", "1 end", "2 start"]);
+  resolve2();
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start", "1 end", "2 start", "2 end"]);
+});
+
+it("variable.define does not wait for other variables", async () => {
+  const runtime = new Runtime();
+  const main = runtime.module();
+  const log = [];
+  let resolve1;
+  let resolve2;
+  const promise1 = new Promise((r) => (resolve1 = r));
+  const promise2 = new Promise((r) => (resolve2 = r));
+  const foo = main.variable(true);
+  foo.define("foo", [], () => {
+    log.push("1 start");
+    promise1.then(() => log.push("1 end"));
+    return promise1;
+  });
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start"]);
+  const bar = main.variable(true);
+  foo.delete();
+  bar.define("foo", [], () => {
+    log.push("2 start");
+    promise2.then(() => log.push("2 end"));
+    return promise2;
+  });
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start", "2 start"]);
+  resolve1();
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start", "2 start", "1 end"]);
+  resolve2();
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start", "2 start", "1 end", "2 end"]);
+});
+
+it("variable.define skips stale definitions", async () => {
+  const runtime = new Runtime();
+  const main = runtime.module();
+  const log = [];
+  let resolve1;
+  let resolve2;
+  let resolve3;
+  const promise1 = new Promise((r) => (resolve1 = r));
+  const promise2 = new Promise((r) => (resolve2 = r));
+  const promise3 = new Promise((r) => (resolve3 = r));
+  const foo = main.variable(true);
+  foo.define("foo", [], () => {
+    log.push("1 start");
+    promise1.then(() => log.push("1 end"));
+    return promise1;
+  });
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start"]);
+  foo.define("foo", [], () => {
+    log.push("2 start");
+    promise2.then(() => log.push("2 end"));
+    return promise2;
+  });
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start"]);
+  foo.define("foo", [], () => {
+    log.push("3 start");
+    promise3.then(() => log.push("3 end"));
+    return promise3;
+  });
+  resolve1();
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start", "1 end", "3 start"]);
+  resolve2();
+  resolve3();
+  await sleep();
+  assert.deepStrictEqual(log, ["1 start", "1 end", "3 start", "3 end"]);
+});