[labs/virtualizer] Simplify initialization, more complete fix for #3481…

… (#3624) * add test for immediate layout change * consolidate state events * replace event-based communication between layout and virtualizer * no longer auto-load ResizeObserver polyfill; init now entirely sync except for default layout * add fix for immediate layout change * restore separate message for visibility change only, to avoid perf regression * update package-lock.json * fix typo in README * Update root package-lock and remove virtualizer specific package-lock * Updated the range changed events expectation to accept more than one event to adjust to inconsistent behavior in different runs/environments. * Updated section about the forked ResizeObserver polyfill in virtualizer README. --------- Co-authored-by: Augustine Kim <augustinekim@google.com> Co-authored-by: Brendan Baldwin <brendan@usergenic.com>
lit · Feb 2, 2023 · e51ff22 · e51ff22
1 parent 61ce814
commit e51ff22
Show file tree

Hide file tree

Showing 20 changed files with 4,705 additions and 3,496 deletions.
diff --git a/.changeset/thin-tips-camp.md b/.changeset/thin-tips-camp.md
@@ -0,0 +1,5 @@
+---
+'@lit-labs/virtualizer': patch
+---
+
+Additional fix for [#3481: Error when immediately re-rendering](https://github.com/lit/lit/issues/3481); initialization code significantly simplified
diff --git a/.changeset/two-carrots-crash.md b/.changeset/two-carrots-crash.md
@@ -0,0 +1,5 @@
+---
+'@lit-labs/virtualizer': major
+---
+
+ResizeObserver polyfill is no longer automatically loaded. If you target older browsers without native ResizeObserver support, see the docs for guidance on manual polyfill loading.
diff --git a/package-lock.json b/package-lock.json
diff --git a/packages/labs/virtualizer/README.md b/packages/labs/virtualizer/README.md
@@ -285,6 +285,65 @@ export class MyItems extends LitElement {
 }
 ```
 
+## ResizeObserver dependency
+
+Virtualizer depends on the standard [`ResizeObserver`]() API, which is supported in all modern browsers. In case your browser support matrix includes older browsers that don't implement `ResizeObserver`, the Virtualizer package includes a `ResizeObserver` polyfill that is known to be compatible with Virtualizer. This is a forked version of Denis Rul's `resize-observer-polyfill` package, which we modified to extend its observations into shadow roots.
+
+### Using the default loader
+
+The package also includes a simple mechanism for loading the `ResizeObserver` polyfill. This mechanism uses feature detection to determine whether the polyfill is required; if it is, then the polyfill is automatically loaded and provided directly to Virtualizer.
+
+You need to invoke the loader—which is asynchronous—and await its return before running any code that would cause a virtualizer to be instantiated. The simplest way to do this is to load virtualizer itself with a dynamic `import()` statement. For example:
+
+```js
+import {loadPolyfillIfNeeded} from '@lit-labs/virtualizer/polyfillLoaders/ResizeObserver.js';
+
+async function load() {
+  await loadPolyfillIfNeeded();
+  await import('@lit-labs/virtualizer');
+}
+```
+
+### Writing a custom loader
+
+In case you want to make the `ResizeObserver` polyfill available for use outside of Virtualizer or have other specialized polyfill-loading requirements, you can alternatively import the polyfill directly and write your own custom code for loading and exposing it.
+
+If you choose this option, there are two ways you can make the polyfill available for Virtualizer to use:
+
+- Expose the polyfill via `window.ResizeObserver` so that Virtualizer can access it just as it would the native implementation.
+- Pass the polyfill constructor to Virtualizer's `provideResizeObserver()` function.
+
+Either way, you should do so before instantiating any virtualizers, just as when you use the provided loader.
+
+Here's how you would import the polyfill and the `provideResizeObserver()` function statically:
+
+```js
+import ResizeObserverPolyfill from '@lit-labs/virtualizer/polyfills/resize-observer-polyfill/ResizeObserver.js';
+import {provideResizeObserver} from '@lit-labs/virtualizer/polyfillLoaders/ResizeObserver.js';
+```
+
+More typically, though, you'd import them dynamically:
+
+```js
+async function myCustomLoader() {
+  // Write whatever custom logic you need, using feature detection, etc...
+
+  // If you need to load the polyfill, do it like this:
+  const ResizeObserverPolyfill = await import(
+    '@lit-labs/virtualizer/polyfills/resize-observer-polyfill/ResizeObserver.js'
+  ).default;
+
+  // To install the polyfill globally, do something like this:
+  window.ResizeObserver = ResizeObserverPolyfill;
+
+  // Or to provide it to Virtualizer, do something like this:
+  const {provideResizeObserver} = await import(
+    '@lit-labs/virtualizer/polyfillLoaders/ResizeObserver.js'
+  );
+  provideResizeObserver(ResizeObserverPolyfill);
+}
+```
+
 ## API Reference
 
 ### `items` property

diff --git a/packages/labs/virtualizer/package.json b/packages/labs/virtualizer/package.json
@@ -29,6 +29,12 @@
     "./LitVirtualizer.js": {
       "default": "./LitVirtualizer.js"
     },
+    "./polyfillLoaders/ResizeObserver.js": {
+      "default": "./polyfillLoaders/ResizeObserver.js"
+    },
+    "./polyfills/resize-observer-polyfill/ResizeObserver.js": {
+      "default": "./polyfillLoaders/ResizeObserver.js"
+    },
     "./virtualize.js": {
       "default": "./virtualize.js"
     }
@@ -121,7 +127,6 @@
     "tachometer": "^0.7.0"
   },
   "dependencies": {
-    "event-target-shim": "^6.0.2",
     "lit": "^2.5.0",
     "tslib": "^2.0.3"
   }

diff --git a/packages/labs/virtualizer/src/Virtualizer.ts b/packages/labs/virtualizer/src/Virtualizer.ts
@@ -4,7 +4,6 @@
  * SPDX-License-Identifier: BSD-3-Clause
  */
 
-import getResizeObserver from './polyfillLoaders/ResizeObserver.js';
 import {
   ItemBox,
   Margins,
@@ -14,11 +13,13 @@ import {
   Layout,
   LayoutConstructor,
   LayoutSpecifier,
+  StateChangedMessage,
   Size,
   InternalRange,
   MeasureChildFunction,
   ScrollToCoordinates,
   BaseLayoutConfig,
+  LayoutHostMessage,
 } from './layouts/shared/Layout.js';
 import {
   RangeChangedEvent,
@@ -27,6 +28,22 @@ import {
 } from './events.js';
 import {ScrollerController} from './ScrollerController.js';
 
+// Virtualizer depends on `ResizeObserver`, which is supported in
+// all modern browsers. For developers whose browser support
+// matrix includes older browsers, we include a compatible
+// polyfill in the package; this bit of module state facilitates
+// a simple mechanism (see ./polyfillLoaders/ResizeObserver.js.)
+// for loading the polyfill.
+let _ResizeObserver: typeof ResizeObserver | undefined = window?.ResizeObserver;
+
+/**
+ * Call this function to provide a `ResizeObserver` polyfill for Virtualizer to use.
+ * @param Ctor Constructor for a `ResizeObserver` polyfill (recommend using the one provided with the Virtualizer package)
+ */
+export function provideResizeObserver(Ctor: typeof ResizeObserver) {
+  _ResizeObserver = Ctor;
+}
+
 export const virtualizerRef = Symbol('virtualizerRef');
 const SIZER_ATTRIBUTE = 'virtualizer-sizer';
 
@@ -206,6 +223,13 @@ export class Virtualizer {
   private _layoutCompleteRejecter: Function | null = null;
   private _pendingLayoutComplete: number | null = null;
 
+  /**
+   * Layout initialization is async because we dynamically load
+   * the default layout if none is specified. This state is to track
+   * whether init is complete.
+   */
+  private _layoutInitialized: Promise<void> | null = null;
+
   constructor(config: VirtualizerConfig) {
     if (!config) {
       throw new Error(
@@ -235,18 +259,22 @@ export class Virtualizer {
     // If no layout is specified, we make an empty
     // layout config, which will result in the default
     // layout with default parameters
-    this._initLayout(config.layout || ({} as BaseLayoutConfig));
+    const layoutConfig = config.layout || ({} as BaseLayoutConfig);
+    // Save the promise returned by `_initLayout` as a state
+    // variable we can check before updating layout config
+    this._layoutInitialized = this._initLayout(layoutConfig);
   }
 
-  private async _initObservers() {
+  private _initObservers() {
     this._mutationObserver = new MutationObserver(
       this._finishDOMUpdate.bind(this)
     );
-    const ResizeObserver = await getResizeObserver();
-    this._hostElementRO = new ResizeObserver(() =>
+    this._hostElementRO = new _ResizeObserver!(() =>
       this._hostElementSizeChanged()
     );
-    this._childrenRO = new ResizeObserver(this._childrenSizeChanged.bind(this));
+    this._childrenRO = new _ResizeObserver!(
+      this._childrenSizeChanged.bind(this)
+    );
   }
 
   _initHostElement(config: VirtualizerConfig) {
@@ -255,8 +283,8 @@ export class Virtualizer {
     hostElement[virtualizerRef] = this;
   }
 
-  async connected() {
-    await this._initObservers();
+  connected() {
+    this._initObservers();
     const includeSelf = this._isScroller;
     this._clippingAncestors = getClippingAncestors(
       this._hostElement!,
@@ -303,10 +331,10 @@ export class Virtualizer {
     );
     this._scrollEventListeners = [];
     this._clippingAncestors = [];
-    this._scrollerController = this._scrollerController?.detach(this) || null;
-    this._mutationObserver?.disconnect();
-    this._hostElementRO?.disconnect();
-    this._childrenRO?.disconnect();
+    this._scrollerController = this._scrollerController!.detach(this) || null;
+    this._mutationObserver!.disconnect();
+    this._hostElementRO!.disconnect();
+    this._childrenRO!.disconnect();
     this._rejectLayoutCompletePromise('disconnected');
   }
 
@@ -356,16 +384,28 @@ export class Virtualizer {
     return this._sizer;
   }
 
-  updateLayoutConfig(layoutConfig: LayoutConfigValue) {
+  async updateLayoutConfig(layoutConfig: LayoutConfigValue) {
+    // If layout initialization hasn't finished yet, we wait
+    // for it to finish so we can check whether the new config
+    // is compatible with the existing layout before proceeding.
+    await this._layoutInitialized;
     const Ctor =
       ((layoutConfig as LayoutSpecifier).type as LayoutConstructor) ||
+      // The new config is compatible with the current layout,
+      // so we update the config and return true to indicate
+      // a successful update
       DefaultLayoutConstructor;
     if (typeof Ctor === 'function' && this._layout instanceof Ctor) {
       const config = {...(layoutConfig as LayoutSpecifier)} as {
         type?: LayoutConstructor;
       };
       delete config.type;
       this._layout.config = config as BaseLayoutConfig;
+      // The new config requires a different layout altogether, but
+      // to limit implementation complexity we don't support dynamically
+      // changing the layout of an existing virtualizer instance.
+      // Returning false here lets the caller know that they should
+      // instead make a new virtualizer instance with the desired layout.
       return true;
     }
     return false;
@@ -396,7 +436,10 @@ export class Virtualizer {
         .FlowLayout as unknown as LayoutConstructor;
     }
 
-    this._layout = new Ctor(config);
+    this._layout = new Ctor(
+      (message: LayoutHostMessage) => this._handleLayoutMessage(message),
+      config
+    );
 
     if (
       this._layout.measureChildren &&
@@ -407,14 +450,11 @@ export class Virtualizer {
       }
       this._measureCallback = this._layout.updateItemSizes.bind(this._layout);
     }
-    this._layout.addEventListener('scrollsizechange', this);
-    this._layout.addEventListener('scrollerrorchange', this);
-    this._layout.addEventListener('itempositionchange', this);
-    this._layout.addEventListener('rangechange', this);
-    this._layout.addEventListener('unpinned', this);
+
     if (this._layout.listenForChildLoadEvents) {
       this._hostElement!.addEventListener('load', this._loadListener, true);
     }
+
     this._schedule(this._updateLayout);
   }
 
@@ -480,7 +520,11 @@ export class Virtualizer {
     }
   }
 
-  async _updateDOM() {
+  async _updateDOM(state: StateChangedMessage) {
+    this._scrollSize = state.scrollSize;
+    this._adjustRange(state.range);
+    this._childrenPos = state.childPositions;
+    this._scrollError = state.scrollError || null;
     const {_rangeChanged, _itemsChanged} = this;
     if (this._visibilityChanged) {
       this._notifyVisibility();
@@ -549,30 +593,19 @@ export class Virtualizer {
           this._handleScrollEvent();
         }
         break;
-      case 'scrollsizechange':
-        this._scrollSize = event.detail;
-        this._schedule(this._updateDOM);
-        break;
-      case 'scrollerrorchange':
-        this._scrollError = event.detail;
-        this._schedule(this._updateDOM);
-        break;
-      case 'itempositionchange':
-        this._childrenPos = event.detail;
-        this._schedule(this._updateDOM);
-        break;
-      case 'rangechange':
-        this._adjustRange(event.detail);
-        this._schedule(this._updateDOM);
-        break;
-      case 'unpinned':
-        this._hostElement!.dispatchEvent(new UnpinnedEvent());
-        break;
       default:
         console.warn('event not handled', event);
     }
   }
 
+  _handleLayoutMessage(message: LayoutHostMessage) {
+    if (message.type === 'stateChanged') {
+      this._updateDOM(message);
+    } else if (message.type === 'unpinned') {
+      this._hostElement!.dispatchEvent(new UnpinnedEvent());
+    }
+  }
+
   get _children(): Array<HTMLElement> {
     const arr = [];
     let next = this._hostElement!.firstElementChild as HTMLElement;

diff --git a/packages/labs/virtualizer/src/layouts/flexWrap.ts b/packages/labs/virtualizer/src/layouts/flexWrap.ts
@@ -13,6 +13,7 @@ import {
 import {
   ChildMeasurements,
   ItemBox,
+  LayoutHostSink,
   MeasureChildFunction,
   Positions,
   Size,
@@ -23,7 +24,10 @@ interface FlexWrapLayoutConfig extends SizeGapPaddingBaseLayoutConfig {
 }
 
 type FlexWrapLayoutSpecifier = FlexWrapLayoutConfig & {
-  type: new (config?: FlexWrapLayoutConfig) => FlexWrapLayout;
+  type: new (
+    hostSink: LayoutHostSink,
+    config?: FlexWrapLayoutConfig
+  ) => FlexWrapLayout;
 };
 
 type FlexWrapLayoutSpecifierFactory = (

diff --git a/packages/labs/virtualizer/src/layouts/flow.ts b/packages/labs/virtualizer/src/layouts/flow.ts
@@ -15,6 +15,7 @@ import {
   offsetAxis,
   ChildMeasurements,
   BaseLayoutConfig,
+  LayoutHostSink,
 } from './shared/Layout.js';
 
 type ItemBounds = {
@@ -24,7 +25,7 @@ type ItemBounds = {
 
 type FlowLayoutConstructor = {
   prototype: FlowLayout;
-  new (config?: BaseLayoutConfig): FlowLayout;
+  new (hostSink: LayoutHostSink, config?: BaseLayoutConfig): FlowLayout;
 };
 
 type FlowLayoutSpecifier = BaseLayoutConfig & {
@@ -485,36 +486,9 @@ export class FlowLayout extends BaseLayout<BaseLayoutConfig> {
     return 0;
   }
 
-  // TODO: Can this be made to inherit from base, with proper hooks?
-  _reflow() {
-    const {_first, _last, _scrollSize, _firstVisible, _lastVisible} = this;
-
-    this._updateScrollSize();
-    this._setPositionFromPin();
-    this._getActiveItems();
-    this._updateVisibleIndices();
-
-    if (this._scrollSize !== _scrollSize) {
-      this._emitScrollSize();
-    }
-
-    if (
-      this._first !== _first ||
-      this._last !== _last ||
-      this._firstVisible !== _firstVisible ||
-      this._lastVisible !== _lastVisible
-    ) {
-      this._emitRange();
-    }
-
-    if (!(this._first === -1 && this._last === -1)) {
-      this._emitChildPositions();
-    }
-
-    if (this._scrollError !== 0) {
-      this._emitScrollError();
-    }
-
+  override _reflow() {
+    const {_first, _last} = this;
+    super._reflow();
     if (
       (this._first === -1 && this._last == -1) ||
       (this._first === _first && this._last === _last)