From 1d96db3c387a5276d40a7021593ab3ca46e8e90b Mon Sep 17 00:00:00 2001 From: Kevin Schaaf Date: Mon, 2 Mar 2020 23:41:12 -0800 Subject: [PATCH] When re-enabling, ensure __limit is at a good starting point and add a test for that. Also: * Ensure `__itemsArrayChanged` is cleared after every render. * Enqueue `__continueChunkingAfterRaf` before notifying renderedItemCount for safety --- lib/elements/dom-repeat.js | 69 ++++++++++++++++++-------------------- test/unit/dom-repeat.html | 23 ++++++++++--- 2 files changed, 51 insertions(+), 41 deletions(-) diff --git a/lib/elements/dom-repeat.js b/lib/elements/dom-repeat.js index ef920fad84..6240f982d2 100644 --- a/lib/elements/dom-repeat.js +++ b/lib/elements/dom-repeat.js @@ -472,15 +472,10 @@ 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 + __initialCountChanged(initialCount) { + // When enabling chunking, start the limit at the current rendered count, + // otherwie ensure the list is unlimited + this.__limit = initialCount ? this.renderedItemCount : Infinity; } __handleObservedPaths(path) { @@ -554,18 +549,16 @@ export class DomRepeat extends domRepeatBase { const isntIdxToItemsIdx = this.__sortAndFilterItems(items); // If we're chunking, increase the limit if there are new instances to // create and schedule the next chunk - if (this.initialCount) { - this.__updateLimit(isntIdxToItemsIdx.length); - } + this.__updateLimit(isntIdxToItemsIdx.length); // Create, update, and/or remove instances this.__updateInstances(items, isntIdxToItemsIdx); - // Set rendered item count - this._setRenderedItemCount(this.__instances.length); // If we're chunking, schedule a rAF task to measure/continue chunking if (this.initialCount && (this.__shouldMeasureChunk || this.__shouldContinueChunking)) { this.__debounceRender(this.__continueChunkingAfterRaf); } + // Set rendered item count + this._setRenderedItemCount(this.__instances.length); // Notify users if (!suppressTemplateNotifications || this.notifyDomChange) { this.dispatchEvent(new CustomEvent('dom-change', { @@ -594,31 +587,33 @@ export class DomRepeat extends domRepeatBase { } __updateLimit(filteredItemCount) { - let newCount; - if (!this.__chunkCount || - (this.__itemsArrayChanged && !this.reuseChunkedInstances)) { - // Limit next render to the initial count - this.__limit = Math.min(filteredItemCount, this.initialCount); - // Subtract off any existing instances to determine the number of - // instances that will be created - newCount = Math.max(this.__limit - this.__instances.length, 0); - // Initialize the chunk size with how many items we're creating - this.__chunkCount = newCount || 1; - } else { - // The number of new instances that will be created is based on the - // existing instances, the new list size, and the maximum chunk size - newCount = Math.min( - Math.max(filteredItemCount - this.__instances.length, 0), - this.__chunkCount); - // Update the limit based on how many new items we're making, limited - // buy the total size of the list - this.__limit = Math.min(this.__limit + newCount, filteredItemCount); + if (this.initialCount) { + let newCount; + if (!this.__chunkCount || + (this.__itemsArrayChanged && !this.reuseChunkedInstances)) { + // Limit next render to the initial count + this.__limit = Math.min(filteredItemCount, this.initialCount); + // Subtract off any existing instances to determine the number of + // instances that will be created + newCount = Math.max(this.__limit - this.__instances.length, 0); + // Initialize the chunk size with how many items we're creating + this.__chunkCount = newCount || 1; + } else { + // The number of new instances that will be created is based on the + // existing instances, the new list size, and the maximum chunk size + newCount = Math.min( + Math.max(filteredItemCount - this.__instances.length, 0), + this.__chunkCount); + // Update the limit based on how many new items we're making, limited + // buy the total size of the list + this.__limit = Math.min(this.__limit + newCount, filteredItemCount); + } + // Record some state about chunking for use in `__continueChunking` + this.__shouldMeasureChunk = newCount === this.__chunkCount; + this.__shouldContinueChunking = this.__limit < filteredItemCount; + this.__renderStartTime = performance.now(); } this.__itemsArrayChanged = false; - // Record some state about chunking for use in `__continueChunking` - this.__shouldMeasureChunk = newCount === this.__chunkCount; - this.__shouldContinueChunking = this.__limit < filteredItemCount; - this.__renderStartTime = performance.now(); } __continueChunkingAfterRaf() { diff --git a/test/unit/dom-repeat.html b/test/unit/dom-repeat.html index fa466b8b95..e6edd8f92c 100644 --- a/test/unit/dom-repeat.html +++ b/test/unit/dom-repeat.html @@ -3948,7 +3948,6 @@

x-repeat-limit

suite('chunked rendering', function() { let chunked; - let repeat; let resolve; let targetCount; const handleChange = () => { @@ -3964,7 +3963,6 @@

x-repeat-limit

chunked = document.createElement('x-repeat-chunked'); chunked.addEventListener('dom-change', handleChange); document.body.appendChild(chunked); - repeat = chunked.$.repeat; }); teardown(() => { chunked.removeEventListener('dom-change', handleChange); @@ -4193,7 +4191,7 @@

x-repeat-limit

}); - test('changing to/from initialCount=0', async () => { + test.only('changing to/from initialCount=0', async () => { // Render all chunked.items = chunked.preppedItems.slice(); let stamped = await waitUntilRendered(chunked.preppedItems.length); @@ -4219,7 +4217,7 @@

x-repeat-limit

let frameCount = 0; chunked.items = chunked.preppedItems.slice(); while (stamped.length < chunked.items.length) { - stamped = await waitUntilRendered(10); + stamped = await waitUntilRendered(); if (frameCount === 0) { assert.equal(stamped.length, 10); } @@ -4227,6 +4225,23 @@

x-repeat-limit

} assert.equal(stamped.length, chunked.preppedItems.length); assert.isAtLeast(frameCount, 2); + // Disable chunking + chunked.$.repeater.initialCount = 0; + // Render some + chunked.items = chunked.preppedItems.slice(0, 20); + stamped = await waitUntilRendered(); + assert.equal(stamped.length, chunked.items.length); + // Re-enable chunking + chunked.$.repeater.initialCount = 10; + // Push remaining; these should chunk out + frameCount = 0; + chunked.push('items', ...chunked.preppedItems.slice(20)); + while (stamped.length < chunked.items.length) { + stamped = await waitUntilRendered(); + frameCount++; + } + assert.equal(stamped.length, chunked.items.length); + assert.isAtLeast(frameCount, 2); }); });