diff --git a/packages/blaze/builtins.js b/packages/blaze/builtins.js index dc7525e04..0e58a1478 100644 --- a/packages/blaze/builtins.js +++ b/packages/blaze/builtins.js @@ -250,6 +250,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 = true; + } + } + }, addedAt: function (id, item, index) { Tracker.nonreactive(function () { let newItemView; @@ -340,6 +353,17 @@ 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 so they can re-render normally again. + afterDiff: function () { + if (!eachView._domrange) return; + const members = eachView._domrange.members; + for (let i = 0; i < members.length; i++) { + if (members[i] && members[i].view) { + delete members[i].view._eachItemPendingUpdate; + } + } } }); diff --git a/packages/blaze/view.js b/packages/blaze/view.js index ff5547e7f..a840fb362 100644 --- a/packages/blaze/view.js +++ b/packages/blaze/view.js @@ -471,6 +471,19 @@ 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's pending a sequence update. This prevents stale renders + // where an item's helpers re-run before ObserveSequence has had + // a chance to remove it. See meteor/blaze#468. + if (!c.firstRun) { + let v = view; + while (v) { + if (v._eachItemPendingUpdate) return; + v = v.parentView; + } + } + view.renderCount = view.renderCount + 1; view._isInRender = true; // Any dependencies that should invalidate this Computation come diff --git a/packages/observe-sequence/observe_sequence.js b/packages/observe-sequence/observe_sequence.js index a18ce2acf..85c87d999 100644 --- a/packages/observe-sequence/observe_sequence.js +++ b/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,7 +149,18 @@ ObserveSequence = { throw badSequenceError(seq); } + // 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); + } + diffArray(lastSeqArray, seqArray, callbacks); + + if (callbacks.afterDiff) { + callbacks.afterDiff(); + } + lastSeq = seq; lastSeqArray = seqArray; }); diff --git a/packages/spacebars-tests/template_tests.html b/packages/spacebars-tests/template_tests.html index 34322ade9..7f29ed409 100644 --- a/packages/spacebars-tests/template_tests.html +++ b/packages/spacebars-tests/template_tests.html @@ -1172,3 +1172,31 @@
{{item}}
{{/each}} + + + + + + + + + + diff --git a/packages/spacebars-tests/template_tests.js b/packages/spacebars-tests/template_tests.js index 2821daf54..e2492f6e6 100644 --- a/packages/spacebars-tests/template_tests.js +++ b/packages/spacebars-tests/template_tests.js @@ -4480,3 +4480,110 @@ 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'); + } +);