Skip to content

Commit

Permalink
[labs/virtualizer] Fix virtualize directive update to match lit-virtu…
Browse files Browse the repository at this point in the history
…alizer update. Added test helpers, fixes and scenarios to cover. (#3133)

* Added @open-wc/testing and created some helpers for cleaner tests.
* Added regression test to ensure that virtualize directive will renders when non-item data changes.
* Added eventually() helper to try and reduce redundancy in expect/until pairing.
* Restructured the regression test to prove the observed behavior in #3052
* Fixed virtualize directive to render instead of return noChange to match lit-virtualizer behavior.
* Lock selenium webdriver types because of bad WebSocket import.
  • Loading branch information
usergenic committed Jul 20, 2022
1 parent 359860d commit 36db238
Show file tree
Hide file tree
Showing 8 changed files with 301 additions and 56 deletions.
5 changes: 5 additions & 0 deletions .changeset/yellow-dots-fix.md
@@ -0,0 +1,5 @@
---
'@lit-labs/virtualizer': patch
---

The virtualize directive will now correctly re-render children when data stored outside the items array has changed.
20 changes: 20 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions packages/labs/virtualizer/package.json
Expand Up @@ -101,6 +101,9 @@
"/LitVirtualizer.{d.ts,d.ts.map,js,js.map}"
],
"devDependencies": {
"@esm-bundle/chai": "^4.3.4-fix.0",
"@open-wc/testing": "^3.1.6",
"@types/selenium-webdriver": "=4.0.19",
"@web/dev-server-esbuild": "^0.3.0",
"tachometer": "^0.7.0"
},
Expand Down
65 changes: 54 additions & 11 deletions packages/labs/virtualizer/src/test/helpers.ts
Expand Up @@ -18,32 +18,75 @@ export function getCallerFromStack() {
}

/**
* Use this to await a condition (given as an anonymous function that returns
* a boolean value) to be met. We will stop waiting after the timeout is
* exceeded, after which time we will reject the promise.
* Strips tags, squeezes whitespace and trims a string, to make
* text content comparisons of HTML fragments easier. This helper
* takes an undefined value to make it convenient to use with
* optional chaining operators, i.e. justText(x?.y?.z)
*/
export function justText(html: string | undefined): string {
if (html === undefined) {
return '';
}
return squeeze(stripTags(html, ' ')).trim();
}

/**
* Transforms any amount of whitespace in a string into a single
* space character.
*/
export async function until(cond: () => Boolean, timeout = 1000) {
export function squeeze(text: string): string {
return text.replace(/\s+/gm, ' ').trim();
}

/**
* Removes all tags and comments from a string, by naively
* stripping out everything between < and > characters.
*/
export function stripTags(html: string, replaceWith?: string): string {
return html.replace(
/<[^>]+>/gm,
replaceWith === undefined ? ' ' : replaceWith
);
}

/**
* A promise which will resolve to true or false depending on whether
* the condition function returns true within the given timeout. The
* intended usage of this function in a test would look like:
*/
export async function eventually(cond: () => boolean, timeout = 1000) {
const start = new Date().getTime();
const caller = getCallerFromStack();
return new Promise((resolve, reject) => {
return new Promise((resolve, _reject) => {
check();
function check() {
if (cond()) {
return resolve(true);
}
const now = new Date().getTime();
if (now - start > timeout) {
return reject(
new Error(
`Condition not met within ${timeout}ms: "${cond.toString()}"\n${caller}`
)
);
return resolve(false);
}
setTimeout(check, 0);
}
});
}

/**
* Use this to await a condition (given as an anonymous function that returns
* a boolean value) to be met. We will stop waiting after the timeout is
* exceeded, after which time we will reject the promise.
*/
export async function until(cond: () => boolean, timeout = 1000) {
const caller = getCallerFromStack();
if (await eventually(cond, timeout)) {
return true;
} else {
throw new Error(
`Condition not met within ${timeout}ms: "${cond.toString()}"\n${caller}`
);
}
}

/**
* This solution was inspired to address the issue described in the following links:
* https://stackoverflow.com/questions/49384120/resizeobserver-loop-limit-exceeded
Expand Down
@@ -0,0 +1,148 @@
/**
* @license
* Copyright 2022 Google LLC
* SPDX-License-Identifier: BSD-3-Clause
*/

import {ignoreBenignErrors, justText, until} from '../helpers.js';
import {LitVirtualizer} from '../../lit-virtualizer.js';
import {virtualize} from '../../virtualize.js';
import {css, LitElement} from 'lit';
import {customElement, property} from 'lit/decorators.js';
import {expect, html, fixture} from '@open-wc/testing';

abstract class TestElement extends LitElement {
static styles = css`
:host {
display: block;
height: 200px;
}
`;

@property({type: Object, attribute: false})
public selected: Set<number> = new Set();

@property({type: Array, attribute: false})
public items: Array<number> = [];
}

@customElement('using-lit-virtualizer')
class UsingLitVirtualizer extends TestElement {
render() {
return html` <lit-virtualizer
scroller
.items=${this.items}
.renderItem=${(n: number) =>
html`<div>${n}${this.selected.has(n) ? ' selected' : ''}</div>`}
></lit-virtualizer>`;
}
}

@customElement('using-virtualize-directive')
class UsingVirtualizeDirective extends TestElement {
render() {
return html` <div>
${virtualize({
scroller: true,
items: this.items,
renderItem: (n) =>
html`<div>${n}${this.selected.has(n) ? ' selected' : ''}</div>`,
})}
</div>`;
}
}

describe('test fixture classes', () => {
it('are registered as custom elements', () => {
expect(customElements.get('this-is-not-a-custom-element')).to.be.undefined;
expect(customElements.get('lit-virtualizer')).to.eq(LitVirtualizer);
expect(customElements.get('using-lit-virtualizer')).to.eq(
UsingLitVirtualizer
);
expect(customElements.get('using-virtualize-directive')).to.eq(
UsingVirtualizeDirective
);
});
});

describe('lit-virtualizer and virtualize directive', () => {
ignoreBenignErrors(beforeEach, afterEach);

/**
* Regression test to cover the difference in behavior which led
* to this issue: https://github.com/lit/lit/issues/3052
*/
it('both render changes based on non-item data changes', async () => {
const items: Array<number> = Array.from(Array(100).keys());
const selected = new Set([2, 5]);

const example = await fixture(html`
<div>
<using-lit-virtualizer></using-lit-virtualizer>
<using-virtualize-directive></using-virtualize-directive>
</div>
`);
await until(
() =>
example.querySelector('using-lit-virtualizer') instanceof
UsingLitVirtualizer
);
await until(
() =>
example.querySelector('using-virtualize-directive') instanceof
UsingVirtualizeDirective
);

const ulv: UsingLitVirtualizer = example.querySelector(
'using-lit-virtualizer'
)!;

ulv.items = items;
ulv.selected = selected;

await until(() =>
justText(ulv.shadowRoot?.innerHTML).includes('2 selected')
);

expect(justText(ulv.shadowRoot?.innerHTML)).to.include('2 selected');
expect(justText(ulv.shadowRoot?.innerHTML)).to.include('5 selected');

const uvd: UsingVirtualizeDirective = example.querySelector(
'using-virtualize-directive'
)!;

uvd.items = items;
uvd.selected = selected;

await until(() =>
justText(uvd.shadowRoot?.innerHTML).includes('2 selected')
);

expect(justText(uvd.shadowRoot?.innerHTML)).includes('2 selected');
expect(justText(uvd.shadowRoot?.innerHTML)).includes('5 selected');

const newSelected = new Set([1, 3]);

ulv.selected = newSelected;

await until(() =>
justText(ulv.shadowRoot?.innerHTML).includes('1 selected')
);

expect(justText(ulv.shadowRoot!.innerHTML)).to.include('1 selected');
expect(justText(ulv.shadowRoot!.innerHTML)).to.include('3 selected');
expect(justText(ulv.shadowRoot!.innerHTML)).not.to.include('2 selected');
expect(justText(ulv.shadowRoot!.innerHTML)).not.to.include('5 selected');

uvd.selected = newSelected;

await until(() =>
justText(uvd.shadowRoot?.innerHTML).includes('1 selected')
);

expect(justText(uvd.shadowRoot!.innerHTML)).to.include('1 selected');
expect(justText(uvd.shadowRoot!.innerHTML)).to.include('3 selected');
expect(justText(uvd.shadowRoot!.innerHTML)).not.to.include('2 selected');
expect(justText(uvd.shadowRoot!.innerHTML)).not.to.include('5 selected');
});
});

0 comments on commit 36db238

Please sign in to comment.