fix: prevent stale re-renders of #each items during sequence update (#468)

packages/blaze/builtins.js

@@ -217,6 +217,9 @@ Blaze.Each = function (argFunc, contentFunc, elseFunc) {
   eachView.elseFunc = elseFunc;
   eachView.argVar = undefined;
   eachView.variableName = null;
+  // Fired by `afterDiff` to revive item view renders that were deferred
+  // while this each view was pending a sequence update. See meteor/blaze#468.
+  eachView._eachItemPendingDep = new Tracker.Dependency();
 
   // update the @index value in the scope of all subviews in the range
   const updateIndices = function (from, to) {
@@ -250,6 +253,19 @@ Blaze.Each = function (argFunc, contentFunc, elseFunc) {
     eachView.stopHandle = ObserveSequence.observe(function () {
       return eachView.argVar.get()?.value;
     }, {
+      // Called immediately when the sequence source is invalidated,
+      // BEFORE the Tracker flush re-runs other autoruns. This freezes
+      // item views so their helpers don't re-run with stale data.
+      // See meteor/blaze#468.
+      onInvalidate: function () {
+        if (!eachView._domrange) return;
+        const members = eachView._domrange.members;
+        for (let i = 0; i < members.length; i++) {
+          if (members[i] && members[i].view) {
+            members[i].view._eachItemPendingUpdate = eachView._eachItemPendingDep;
+          }
+        }
+      },
       addedAt: function (id, item, index) {
         Tracker.nonreactive(function () {
           let newItemView;
@@ -340,6 +356,20 @@ Blaze.Each = function (argFunc, contentFunc, elseFunc) {
             subviews.splice(toIndex, 0, itemView);
           }
         });
+      },
+      // Called after the diff is applied. Clear the pending flag on
+      // surviving item views, then fire the revival dependency so any item
+      // render that was deferred during the update re-runs with fresh data.
+      afterDiff: function () {
+        if (eachView._domrange) {
+          const members = eachView._domrange.members;
+          for (let i = 0; i < members.length; i++) {
+            if (members[i] && members[i].view) {
+              delete members[i].view._eachItemPendingUpdate;
+            }
+          }
+        }
+        eachView._eachItemPendingDep.changed();
       }
     });
 

packages/blaze/view.js

@@ -471,6 +471,28 @@ Blaze._materializeView = function (view, parentView, _workStack, _intoArray) {
   Tracker.nonreactive(function () {
     view.autorun(function doRender(c) {
       // `view.autorun` sets the current view.
+
+      // Skip re-render if this view or an ancestor is an #each item that
+      // is pending a sequence update. This prevents stale renders where an
+      // item's helpers re-run before ObserveSequence has had a chance to
+      // update or remove it. Instead of returning silently — which would
+      // leave this computation with zero reactive dependencies and thus
+      // permanently dead — we depend on the owning each view's revival
+      // dependency. ObserveSequence's `afterDiff` fires that dependency
+      // once the pending flag is cleared, so this computation re-runs and
+      // re-collects its real dependencies with fresh data. See
+      // meteor/blaze#468.
+      if (!c.firstRun) {
+        let v = view;
+        while (v) {
+          if (v._eachItemPendingUpdate) {
+            v._eachItemPendingUpdate.depend();
+            return;
+          }
+          v = v.parentView;
+        }
+      }
+
       view.renderCount = view.renderCount + 1;
       view._isInRender = true;
       // Any dependencies that should invalidate this Computation come

packages/observe-sequence/observe_sequence.js

@@ -112,9 +112,16 @@ ObserveSequence = {
     // general 'key' argument which could be a function, a dotted
     // field name, or the special @index value.
     let lastSeqArray = []; // elements are objects of form {_id, item}
-    const computation = Tracker.autorun(function () {
+    const computation = Tracker.autorun(function (c) {
       const seq = sequenceFunc();
 
+      // When this computation is invalidated (sequence source changed),
+      // immediately notify callers so they can freeze item views BEFORE
+      // the flush re-runs other autoruns. See meteor/blaze#468.
+      if (callbacks.onInvalidate) {
+        c.onInvalidate(() => callbacks.onInvalidate());
+      }
+
       Tracker.nonreactive(function () {
         let seqArray; // same structure as `lastSeqArray` above.
 
@@ -142,9 +149,27 @@ ObserveSequence = {
           throw badSequenceError(seq);
         }
 
-        diffArray(lastSeqArray, seqArray, callbacks);
-        lastSeq = seq;
-        lastSeqArray = seqArray;
+        // Allow callers to prepare for the diff (e.g., freeze item views
+        // that are about to be removed). See meteor/blaze#468.
+        if (callbacks.beforeDiff) {
+          callbacks.beforeDiff(lastSeqArray, seqArray);
+        }
+
+        try {
+          diffArray(lastSeqArray, seqArray, callbacks);
+
+          // Only record the new baseline once the diff has fully applied.
+          // If a diff callback throws, the DOM no longer matches seqArray,
+          // so the last consistent baseline is kept for the next diff.
+          lastSeq = seq;
+          lastSeqArray = seqArray;
+        } finally {
+          // Always run afterDiff, even if a diff callback threw, so item
+          // views are never left permanently frozen. See meteor/blaze#468.
+          if (callbacks.afterDiff) {
+            callbacks.afterDiff();
+          }
+        }
       });
     });
 

packages/observe-sequence/observe_sequence_tests.js

@@ -742,3 +742,48 @@ Tinytest.addAsync('observe-sequence - cursor to other cursor, same collection',
   ]);
 });
 
+// #468 — `afterDiff` must always run, even if a diff callback (e.g. an
+// item view's first render in Blaze.Each) throws mid-diff. Otherwise
+// callers that freeze state in `onInvalidate`/`beforeDiff` and release it
+// in `afterDiff` would be left permanently frozen by a transient error.
+Tinytest.add(
+  'observe-sequence - afterDiff runs even when a diff callback throws',
+  function (test) {
+    const dep = new Tracker.Dependency();
+    let seq = [{ _id: '1' }];
+    let afterDiffCount = 0;
+
+    const handle = ObserveSequence.observe(function () {
+      dep.depend();
+      return seq;
+    }, {
+      addedAt: function (id) {
+        if (id === '2') {
+          throw new Error('intentional failure while adding item 2');
+        }
+      },
+      afterDiff: function () {
+        afterDiffCount++;
+      },
+    });
+
+    // Initial diff added item '1' and ran afterDiff once.
+    test.equal(afterDiffCount, 1);
+
+    // Re-run with a throwing addedAt. The error surfaces on the re-run
+    // (logged or rethrown by the flush); either way afterDiff must run.
+    seq = [{ _id: '1' }, { _id: '2' }];
+    dep.changed();
+    try {
+      Tracker.flush();
+    } catch (e) {
+      // Expected: the thrown diff callback may surface here.
+    }
+
+    test.equal(afterDiffCount, 2,
+      'afterDiff did not run after a diff callback threw');
+
+    handle.stop();
+  }
+);
+

packages/spacebars-tests/template_tests.html

@@ -1172,3 +1172,31 @@
     <div>{{item}}</div>
   {{/each}}
 </template>
+
+<!-- #468 stale data context tests -->
+<template name="spacebars_template_test_each_stale_parent1">
+  {{> spacebars_template_test_each_stale_child1 foo=mode}}
+</template>
+<template name="spacebars_template_test_each_stale_child1">
+  {{#each getItems}}
+    <span>{{msg}}-{{logRender msg}}</span>
+  {{/each}}
+</template>
+
+<template name="spacebars_template_test_each_stale_parent2">
+  {{> spacebars_template_test_each_stale_child2 foo=mode}}
+</template>
+<template name="spacebars_template_test_each_stale_child2">
+  {{#each getItems}}
+    <span>{{msg}}-{{logRender msg}}</span>
+  {{/each}}
+</template>
+
+<template name="spacebars_template_test_each_stale_parent3">
+  {{> spacebars_template_test_each_stale_child3 foo=mode}}
+</template>
+<template name="spacebars_template_test_each_stale_child3">
+  {{#each item in getItems}}
+    <span>{{item.msg}}-{{logRenderItem item}}</span>
+  {{/each}}
+</template>

packages/spacebars-tests/template_tests.js

@@ -4480,3 +4480,175 @@ Tinytest.add(
     );
   }
 );
+
+// #468 — #each stale data context
+// When the parent data context changes and the #each sequence returns
+// different items, item views should NOT re-render with stale data
+// before being removed.
+Tinytest.add(
+  'spacebars-tests - template_tests - #each no stale render on different IDs',
+  function (test) {
+    const parentTmpl = Template.spacebars_template_test_each_stale_parent1;
+    const childTmpl = Template.spacebars_template_test_each_stale_child1;
+
+    const mode = new ReactiveVar('foo');
+    const renderLog = [];
+
+    parentTmpl.helpers({
+      mode: function () { return mode.get(); },
+    });
+
+    childTmpl.helpers({
+      getItems: function () {
+        const foo = Template.currentData().foo;
+        if (foo === 'foo') {
+          return [{ _id: '1', msg: 'foo-item' }];
+        }
+        return [{ _id: '2', msg: 'bar-item' }];
+      },
+      logRender: function (msg) {
+        const dataFoo = Template.instance().data.foo;
+        renderLog.push({ msg, dataFoo });
+        return '';
+      },
+    });
+
+    const div = renderToDiv(parentTmpl);
+
+    // Initial render
+    test.equal(renderLog.length, 1);
+    test.equal(renderLog[0].msg, 'foo-item');
+    test.equal(renderLog[0].dataFoo, 'foo');
+
+    // Switch — should NOT produce a stale render where msg="foo-item" with dataFoo="bar"
+    renderLog.length = 0;
+    mode.set('bar');
+    Tracker.flush();
+
+    // Every render should have consistent msg and dataFoo
+    renderLog.forEach(function (entry) {
+      if (entry.msg === 'foo-item') {
+        test.equal(entry.dataFoo, 'foo', 'stale: foo-item rendered with dataFoo=bar');
+      }
+      if (entry.msg === 'bar-item') {
+        test.equal(entry.dataFoo, 'bar', 'stale: bar-item rendered with dataFoo=foo');
+      }
+    });
+
+    // Final render should be bar-item
+    const last = renderLog[renderLog.length - 1];
+    test.equal(last.msg, 'bar-item');
+    test.equal(last.dataFoo, 'bar');
+  }
+);
+
+
+Tinytest.add(
+  'spacebars-tests - template_tests - #each no stale render with each-in syntax',
+  function (test) {
+    const parentTmpl = Template.spacebars_template_test_each_stale_parent3;
+    const childTmpl = Template.spacebars_template_test_each_stale_child3;
+
+    const mode = new ReactiveVar('foo');
+    const renderLog = [];
+
+    parentTmpl.helpers({
+      mode: function () { return mode.get(); },
+    });
+
+    childTmpl.helpers({
+      getItems: function () {
+        const foo = Template.currentData().foo;
+        if (foo === 'foo') {
+          return [{ _id: '1', msg: 'foo-item' }];
+        }
+        return [{ _id: '2', msg: 'bar-item' }];
+      },
+      logRenderItem: function (item) {
+        const dataFoo = Template.instance().data.foo;
+        renderLog.push({ msg: item.msg, dataFoo });
+        return '';
+      },
+    });
+
+    const div = renderToDiv(parentTmpl);
+    renderLog.length = 0;
+    mode.set('bar');
+    Tracker.flush();
+
+    renderLog.forEach(function (entry) {
+      if (entry.msg === 'foo-item') {
+        test.equal(entry.dataFoo, 'foo', 'stale: foo-item rendered with dataFoo=bar');
+      }
+    });
+
+    const last = renderLog[renderLog.length - 1];
+    test.equal(last.msg, 'bar-item');
+    test.equal(last.dataFoo, 'bar');
+  }
+);
+
+
+// #468 (parallel path) — a SURVIVING item view (same _id, changed inner
+// data) must still re-render to its new data after the sequence update,
+// and must never render a stale (msg, dataFoo) pair in between. This
+// guards against the freeze-on-pending logic stranding views that are
+// kept (changedAt) rather than removed.
+Tinytest.add(
+  'spacebars-tests - template_tests - #each surviving item re-renders after sequence update',
+  function (test) {
+    const parentTmpl = Template.spacebars_template_test_each_stale_parent2;
+    const childTmpl = Template.spacebars_template_test_each_stale_child2;
+
+    const mode = new ReactiveVar('foo');
+    const renderLog = [];
+
+    parentTmpl.helpers({
+      mode: function () { return mode.get(); },
+    });
+
+    childTmpl.helpers({
+      // Same _id across the flip => ObserveSequence reports changedAt
+      // (the item survives) rather than removedAt/addedAt.
+      getItems: function () {
+        const foo = Template.currentData().foo;
+        return [{ _id: '1', msg: foo === 'foo' ? 'foo-msg' : 'bar-msg' }];
+      },
+      logRender: function (msg) {
+        const dataFoo = Template.instance().data.foo;
+        renderLog.push({ msg, dataFoo });
+        return '';
+      },
+    });
+
+    const div = renderToDiv(parentTmpl);
+
+    test.equal(renderLog.length, 1);
+    test.equal(renderLog[0].msg, 'foo-msg');
+    test.equal(renderLog[0].dataFoo, 'foo');
+    test.matches(canonicalizeHtml(div.innerHTML), /foo-msg/);
+
+    renderLog.length = 0;
+    mode.set('bar');
+    Tracker.flush();
+
+    // No stale pairing during the transition.
+    renderLog.forEach(function (entry) {
+      if (entry.msg === 'foo-msg') {
+        test.equal(entry.dataFoo, 'foo', 'stale: foo-msg rendered with dataFoo=bar');
+      }
+      if (entry.msg === 'bar-msg') {
+        test.equal(entry.dataFoo, 'bar', 'stale: bar-msg rendered with dataFoo=foo');
+      }
+    });
+
+    // The surviving view must have re-rendered to the new data — both in
+    // the render log and in the live DOM (catches a frozen/stuck view).
+    const last = renderLog[renderLog.length - 1];
+    test.equal(last.msg, 'bar-msg');
+    test.equal(last.dataFoo, 'bar');
+    test.matches(canonicalizeHtml(div.innerHTML), /bar-msg/);
+    test.equal(/foo-msg/.test(canonicalizeHtml(div.innerHTML)), false,
+      'stale DOM: surviving view still shows old data after update');
+  }
+);