Skip to content

Commit

Permalink
When re-enabling, ensure __limit is at a good starting point and add …
Browse files Browse the repository at this point in the history
…a test for that.

Also:
* Ensure `__itemsArrayChanged` is cleared after every render.
* Enqueue `__continueChunkingAfterRaf` before notifying renderedItemCount for safety
  • Loading branch information
kevinpschaaf committed Mar 3, 2020
1 parent b503db1 commit 1d96db3
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 41 deletions.
69 changes: 32 additions & 37 deletions lib/elements/dom-repeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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', {
Expand Down Expand Up @@ -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() {
Expand Down
23 changes: 19 additions & 4 deletions test/unit/dom-repeat.html
Original file line number Diff line number Diff line change
Expand Up @@ -3948,7 +3948,6 @@ <h4>x-repeat-limit</h4>
suite('chunked rendering', function() {

let chunked;
let repeat;
let resolve;
let targetCount;
const handleChange = () => {
Expand All @@ -3964,7 +3963,6 @@ <h4>x-repeat-limit</h4>
chunked = document.createElement('x-repeat-chunked');
chunked.addEventListener('dom-change', handleChange);
document.body.appendChild(chunked);
repeat = chunked.$.repeat;
});
teardown(() => {
chunked.removeEventListener('dom-change', handleChange);
Expand Down Expand Up @@ -4193,7 +4191,7 @@ <h4>x-repeat-limit</h4>

});

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);
Expand All @@ -4219,14 +4217,31 @@ <h4>x-repeat-limit</h4>
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);
}
frameCount++;
}
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);
});

});
Expand Down

0 comments on commit 1d96db3

Please sign in to comment.