From 60f6ccfb7dc9ce80f9a69e120ff3964b18b6605a Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Mon, 2 Mar 2020 22:44:19 -0800 Subject: [PATCH] Ensure limit is reset when initialCount is disabled. Note that any falsey value for initialCount (including `0`) is interpreted as "chunking disabled". This is consistent with 1.x logic, and follows from the logic of "starting chunking by rendering zero items" doesn't really make sense. --- lib/elements/dom-repeat.js | 26 +++-- test/unit/dom-repeat.html | 213 +++++++++++++++++++++++-------------- 2 files changed, 150 insertions(+), 89 deletions(-) diff --git a/lib/elements/dom-repeat.js b/lib/elements/dom-repeat.js index 0a88c81d0f..ef920fad84 100644 --- a/lib/elements/dom-repeat.js +++ b/lib/elements/dom-repeat.js @@ -245,15 +245,16 @@ export class DomRepeat extends domRepeatBase { }, /** - * Defines an initial count of template instances to render after setting - * the `items` array, before the next paint, and puts the `dom-repeat` - * into "chunking mode". The remaining items (and any future items as a - * result of pushing onto the array) will be created and rendered - * incrementally at each animation frame thereof until all instances have - * been rendered. + * When greater than zero, defines an initial count of template instances + * to render after setting the `items` array, before the next paint, and + * puts the `dom-repeat` into "chunking mode". The remaining items (and + * any future items as a result of pushing onto the array) will be created + * and rendered incrementally at each animation frame thereof until all + * instances have been rendered. */ initialCount: { - type: Number + type: Number, + observer: '__initialCountChanged' }, /** @@ -471,6 +472,17 @@ export class DomRepeat extends domRepeatBase { this.observe.replace('.*', '.').split(' '); } + __initialCountChanged(value) { + // When not chunking, ensure the list is unlimited + if (!value) { + this.__limit = Infinity; + } + // When chunking, `__limit` is managed in `__updateLimit` called during + // `__render`, which ensures the limit takes into account initialCount, the + // filtered items length, and any increases based on the throttled + // __chunkCount + } + __handleObservedPaths(path) { // Handle cases where path changes should cause a re-sort/filter if (this.__sortFn || this.__filterFn) { diff --git a/test/unit/dom-repeat.html b/test/unit/dom-repeat.html index 9dce86e49d..18390dea76 100644 --- a/test/unit/dom-repeat.html +++ b/test/unit/dom-repeat.html @@ -3945,28 +3945,38 @@

x-repeat-limit

}); -suite('chunked rendering', function() { +suite.only('chunked rendering', function() { let chunked; - let verifyAfterChange; - const verify = () => verifyAfterChange && verifyAfterChange(); + let repeat; + let resolve; + let targetCount; + const handleChange = () => { + if (!targetCount || chunked.$.repeater.renderedItemCount >= targetCount) { + resolve(Array.from(chunked.root.querySelectorAll('*:not(template):not(dom-repeat)'))); + } + }; + const waitUntilRendered = async (count) => { + targetCount = count; + return await new Promise(r => resolve = r); + }; setup(() => { chunked = document.createElement('x-repeat-chunked'); - chunked.addEventListener('dom-change', verify); + chunked.addEventListener('dom-change', handleChange); document.body.appendChild(chunked); + repeat = chunked.$.repeat; }); teardown(() => { - chunked.removeEventListener('dom-change', verify); + chunked.removeEventListener('dom-change', handleChange); document.body.removeChild(chunked); chunked = null; - verifyAfterChange = null; }); // Framerate=25, element cost = 4ms: should never make more than // (1000/25) / 4 = 10 elements per frame const MAX_PER_FRAME = (1000 / 25) / 4; - test('basic chunked rendering', function(done) { + test('basic chunked rendering', async () => { var checkItemOrder = function(stamped) { for (var i=0; ix-repeat-limit } }; + // Set items to chunk + chunked.items = chunked.preppedItems.slice(); + + let stamped = []; let lastStamped; let frameCount = 0; - verifyAfterChange = function() { - var stamped = Array.from(chunked.root.querySelectorAll('*:not(template):not(dom-repeat)')); + // Loop until chunking is complete + while (stamped.length < chunked.items.length) { + frameCount++; + stamped = await waitUntilRendered(); checkItemOrder(stamped); if (!lastStamped) { // Initial rendering of initial count @@ -3991,21 +4007,14 @@

x-repeat-limit

assert.isAtMost((stamped.length - lastStamped.length), MAX_PER_FRAME, `list should not render more than ${MAX_PER_FRAME} per frame`); } - if (stamped.length < chunked.items.length) { - frameCount++; - lastStamped = stamped; - } else { - // Final rendering at exact item count - assert.equal(stamped.length, 100, 'final count wrong'); - assert.isAtLeast(frameCount, 10, 'should have taken at least 10 frames to render'); - done(); - } - }; - chunked.items = chunked.preppedItems.slice(); - + lastStamped = stamped; + } + // Final rendering at exact item count + assert.equal(stamped.length, 100, 'final count wrong'); + assert.isAtLeast(frameCount, 10, 'should have taken at least 10 frames to render'); }); - test('mutations during chunked rendering', function(done) { + test('mutations during chunked rendering', async () => { var checkItemOrder = function(stamped) { var last = -1; @@ -4038,10 +4047,15 @@

x-repeat-limit

} }; + // Set items to chunk + chunked.items = chunked.preppedItems.slice(); + + let stamped = []; let lastStamped; - var frameCount = 0; - verifyAfterChange = function() { - var stamped = Array.from(chunked.root.querySelectorAll('*:not(template):not(dom-repeat)')); + let frameCount = 0; + // Loop until chunking is complete + while (stamped.length < chunked.items.length) { + stamped = await waitUntilRendered(); checkItemOrder(stamped); if (!lastStamped) { // Initial rendering of initial count @@ -4055,24 +4069,20 @@

x-repeat-limit

assert.isAtMost((stamped.length - lastStamped.length), MAX_PER_FRAME, `list should not render more than ${MAX_PER_FRAME} per frame`); } - if (stamped.length < chunked.items.length) { - if (frameCount++ < 5) { - mutateArray(chunked, stamped); - } - lastStamped = stamped; - } else { - // Final rendering at exact item count - assert.equal(stamped.length, chunked.items.length, 'final count wrong'); - assert.isAtLeast(frameCount, 10, 'should have taken at least 10 frames to render'); - done(); + if (stamped.length < chunked.items.length && frameCount < 5) { + mutateArray(chunked, stamped); } - }; - chunked.items = chunked.preppedItems.slice(); + lastStamped = stamped; + frameCount++; + } + // Final rendering at exact item count + assert.equal(stamped.length, chunked.items.length, 'final count wrong'); + assert.isAtLeast(frameCount, 10, 'should have taken at least 10 frames to render'); }); - test('mutations during chunked rendering, sort & filtered', function(done) { + test('mutations during chunked rendering, sort & filtered', async () => { var checkItemOrder = function(stamped) { var last = Infinity; @@ -4094,12 +4104,23 @@

x-repeat-limit

chunked.splice('items', Math.round(stamped.length/2), 3); }; - var lastStamped; - var frameCount = 0; - verifyAfterChange = function() { - var stamped = Array.from(chunked.root.querySelectorAll('*:not(template):not(dom-repeat)')); + // Set items to chunk + chunked.$.repeater.sort = function(a, b) { + return b.prop - a.prop; + }; + chunked.$.repeater.filter = function(a) { + return (a.prop % 2) === 0; + }; + chunked.items = chunked.preppedItems.slice(); + + let stamped = []; + let lastStamped; + let frameCount = 0; + let filteredLength = chunked.items.filter(chunked.$.repeater.filter).length; + // Loop until chunking is complete + while (stamped.length < filteredLength) { + stamped = await waitUntilRendered(); checkItemOrder(stamped); - var filteredLength = chunked.items.filter(chunked.$.repeater.filter).length; if (!lastStamped) { // Initial rendering of initial count assert.equal(stamped.length, 10); @@ -4112,37 +4133,35 @@

x-repeat-limit

assert.isAtMost((stamped.length - lastStamped.length), MAX_PER_FRAME, `list should not render more than ${MAX_PER_FRAME} per frame`); } - if (stamped.length < filteredLength) { - if (frameCount++ < 4) { - mutateArray(chunked, stamped); - } - lastStamped = stamped; - } else { - assert.equal(stamped.length, filteredLength, 'final count wrong'); - assert.isAtLeast(frameCount, 5, 'should have taken at least 5 frames to render'); - done(); + if (stamped.length < chunked.items.length && frameCount < 4) { + mutateArray(chunked, stamped); + filteredLength = chunked.items.filter(chunked.$.repeater.filter).length; } - }; - chunked.$.repeater.sort = function(a, b) { - return b.prop - a.prop; - }; - chunked.$.repeater.filter = function(a) { - return (a.prop % 2) === 0; - }; - chunked.items = chunked.preppedItems.slice(); - + lastStamped = stamped; + frameCount++; + } + // Final rendering at exact item count + assert.equal(stamped.length, filteredLength, 'final count wrong'); + assert.isAtLeast(frameCount, 5, 'should have taken at least 5 frames to render'); }); suite('resetting items array', () => { [false, true].forEach(reuse => { - test(`reuseChunkedInstances=${reuse}`, (done) => { + test(`reuseChunkedInstances=${reuse}`, async () => { - let i = 3; + // Set items to chunk + chunked.$.repeater.reuseChunkedInstances = reuse; + chunked.items = chunked.preppedItems.slice(); + + let resetCount = 3; + let stamped = []; let lastStamped; - verifyAfterChange = function() { - var stamped = Array.from(chunked.root.querySelectorAll('*:not(template):not(dom-repeat)')); + let frameCount = 0; + // Loop until chunking is complete + while (stamped.length < chunked.items.length) { + stamped = await waitUntilRendered(); if (!lastStamped) { // Initial rendering of initial count assert.equal(stamped.length, 10); @@ -4155,30 +4174,61 @@

x-repeat-limit

assert.isAtMost((stamped.length - lastStamped.length), MAX_PER_FRAME, `list should not render more than ${MAX_PER_FRAME} per frame`); } - if (stamped.length < chunked.items.length) { - lastStamped = stamped; - } else { - assert.equal(stamped.length, chunked.items.length, 'final count wrong'); - if (--i > 0) { - if (!reuse) { - lastStamped = null; - } - chunked.items = chunked.preppedItems.slice(); - } else { - done(); + lastStamped = stamped; + frameCount++; + if (--resetCount > 0) { + if (!reuse) { + lastStamped = null; } + chunked.items = chunked.preppedItems.slice(); } - }; - - chunked.$.repeater.reuseChunkedInstances = reuse; - chunked.items = chunked.preppedItems.slice(); - + } + // Final rendering at exact item count + assert.equal(stamped.length, chunked.items.length, 'final count wrong'); + assert.isAtLeast(frameCount, 5, 'should have taken at least 5 frames to render'); + }); }); }); + test('changing to/from initialCount=0', async () => { + // Render all + chunked.items = chunked.preppedItems.slice(); + let stamped = await waitUntilRendered(chunked.preppedItems.length); + assert.equal(stamped.length, chunked.preppedItems.length); + // Clear the list + chunked.items = []; + stamped = await waitUntilRendered(0); + assert.equal(stamped.length, 0); + // Disable chunking + chunked.$.repeater.initialCount = 0; + // Render all + chunked.items = chunked.preppedItems.slice(); + stamped = await waitUntilRendered(chunked.preppedItems.length); + assert.equal(stamped.length, chunked.preppedItems.length); + // Clear the list + chunked.items = []; + stamped = await waitUntilRendered(0); + assert.equal(stamped.length, 0); + // Re-enable chunking + chunked.$.repeater.initialCount = 10; + // Render all; the initial render should have the initial count, and then + // chunk until the end of the list + let frameCount = 0; + chunked.items = chunked.preppedItems.slice(); + while (stamped.length < chunked.items.length) { + stamped = await waitUntilRendered(10); + if (frameCount === 0) { + assert.equal(stamped.length, 10); + } + frameCount++; + } + assert.equal(stamped.length, chunked.preppedItems.length); + assert.isAtLeast(frameCount, 2); + }); + }); suite('misc', function() { @@ -4233,7 +4283,6 @@

x-repeat-limit

test('paths update on observed properties', function() { let simple = fixture('simple'); flush(); - //debugger; var stamped = simple.root.querySelectorAll('*:not(template):not(dom-repeat)'); assert.equal(stamped[0].itemaProp, 'prop-1'); simple.$.repeat.observe = 'prop';