Fix core grid bugs and rendering performance (#78)

Filtering: OR-only expression trees matched every record because the
empty AND set was vacuously true; nullish cell values crashed string
conditions. Fix the match logic and coerce nullish operands once in
normalizeCase.

Events: cancelable sorting/filtering fired after state was already
mutated. Hand listeners a candidate copy and commit it onto the
state-held object only when the event passes, preserving both object
identity and the listener-can-modify contract. Emit sorted/filtered
after the data view updates.

Pipeline: concurrent async runs could resolve out of order. Sequence
them with an epoch token; stale results are discarded and a rejected
hook keeps the previous data state. Expose the documented but missing
public columns setter and stop columnReducer from dropping sibling
columns inside containers.

Navigation: state was module-level and shared across grid instances;
hidden columns were reachable and a nullish active node produced NaN
row indices. Move state per instance and clamp over visible columns.

Performance: key row rendering by column field so cells are reused,
share a single Intl.Collator, memoize dot-path segments, and drive
the scrollbar offset from a ResizeObserver coalesced into rAF instead
of per-render measurement.

Tests: 178 -> 227, coverage 98%.

Author: rkaraivanov

src/components/cell.ts

@@ -77,7 +77,7 @@ export default class IgcGridLiteCell<T extends object> extends LitElement {
   }
 
   protected override update(props: PropertyValues<this>): void {
-    if (props.has('adoptRootStyles')) {
+    if (props.has('adoptRootStyles') || props.has('cellTemplate')) {
       this._adoptedStylesController.shouldAdoptStyles(this._shouldAdoptStyles);
     }
 

src/components/column.ts

@@ -7,6 +7,7 @@ import { GRID_COLUMN_TAG } from '../internal/tags.js';
 import type {
   BaseColumnConfiguration,
   ColumnSortConfiguration,
+  DataType,
   IgcCellContext,
   IgcHeaderContext,
   Keys,
@@ -44,7 +45,7 @@ export class IgcGridLiteColumn<T extends object = any>
 
   /** The data type of the column's values. */
   @property({ attribute: 'data-type' })
-  public dataType?: 'number' | 'string' | 'boolean' = 'string';
+  public dataType?: DataType = 'string';
 
   /** The header text of the column. */
   @property()

src/components/filter-row.ts

@@ -23,6 +23,9 @@ import type { FilterExpression, FilterOperation, OperandKeys } from '../operatio
 import { styles } from '../styles/filter-row/filter-row.css.js';
 import { all } from '../styles/themes/filtering-row-themes.js';
 
+/** Number of filter expressions shown as chips before collapsing into a single counted chip. */
+const MAX_PREVIEW_CHIPS = 3;
+
 type ExpressionChipProps<T> = {
   expression: FilterExpression<T>;
   selected: boolean;
@@ -135,12 +138,19 @@ export default class IgcFilterRow<T extends object> extends LitElement {
     const key = event.detail.value as OperandKeys<PropertyType<T, typeof this.column.field>>;
 
     // XXX: Types
-    this.expression.condition = (getFilterOperandsFor(this.column) as any)[key] as FilterOperation<
+    const condition = (getFilterOperandsFor(this.column) as any)[key] as FilterOperation<
       PropertyType<T, keyof T>
     >;
 
-    if (this.input.value || this.expression.condition.unary) {
-      this.filterController.filterWithEvent(this.expression, 'modify');
+    if (this.input.value || condition.unary) {
+      this.filterController.filterWithEvent(
+        { ...this.expression, condition },
+        'modify',
+        this.expression
+      );
+    } else {
+      // Nothing to filter by yet - the condition is only staged in the UI.
+      this.expression.condition = condition;
     }
 
     this.requestUpdate();
@@ -156,9 +166,11 @@ export default class IgcFilterRow<T extends object> extends LitElement {
       : 'add';
 
     if (shouldUpdate) {
-      this.expression.searchTerm = value as any;
-
-      this.filterController.filterWithEvent(this.expression, type);
+      this.filterController.filterWithEvent(
+        { ...this.expression, searchTerm: value as any },
+        type,
+        this.expression
+      );
     } else {
       this.#removeExpression(this.expression);
     }
@@ -206,8 +218,9 @@ export default class IgcFilterRow<T extends object> extends LitElement {
     return async (e: Event) => {
       e.stopPropagation();
 
-      expression.criteria = expression.criteria === 'and' ? 'or' : 'and';
-      this.filterController.filterWithEvent(expression, 'modify');
+      const criteria = expression.criteria === 'and' ? 'or' : 'and';
+
+      this.filterController.filterWithEvent({ ...expression, criteria }, 'modify', expression);
       this.requestUpdate();
     };
   }
@@ -385,8 +398,8 @@ export default class IgcFilterRow<T extends object> extends LitElement {
   protected renderFilterState(column: ColumnConfiguration<T>) {
     const state = this.filterController.get(column.field);
 
-    const partial = state && state.length < 3;
-    const hidden = state && state.length >= 3;
+    const partial = state && state.length < MAX_PREVIEW_CHIPS;
+    const hidden = state && state.length >= MAX_PREVIEW_CHIPS;
 
     const open = () => {
       this.column = column;

src/components/grid.ts

@@ -40,11 +40,16 @@ import IgcGridLiteHeaderRow from './header-row.js';
 import IgcGridLiteRow from './row.js';
 import IgcVirtualizer from './virtualizer.js';
 
-/** Column reducer matching either direct column element or one nested in container */
+/** Column reducer matching either a direct column element or every one nested in a container */
 function columnReducer<T extends Element>(acc: T[], el: T): T[] {
   const tag = IgcGridLiteColumn.tagName;
-  const column = el.matches(tag) ? el : el.querySelector(tag);
-  if (column) acc.push(column as unknown as T);
+
+  if (el.matches(tag)) {
+    acc.push(el);
+    return acc;
+  }
+
+  acc.push(...(Array.from(el.querySelectorAll(tag)) as unknown as T[]));
   return acc;
 }
 
@@ -193,6 +198,23 @@ export class IgcGridLite<T extends object = any> extends EventEmitterBase<IgcGri
   @state()
   protected _dataState: T[] = [];
 
+  /** Monotonic token identifying the most recently started pipeline run. */
+  private _pipelineEpoch = 0;
+
+  private _pipelineTask: Promise<void> = Promise.resolve();
+
+  /**
+   * Resolves when the most recently started pipeline run has settled and its result
+   * is rendered. Consumed by the sort/filter controllers so that `sorted`/`filtered`
+   * are emitted against an up to date {@link IgcGridLite.dataView}.
+   *
+   * @internal
+   */
+  public get _pipelineComplete(): Promise<void> {
+    // The `updateComplete` hop lets a just-requested PIPELINE update install its task first.
+    return this.updateComplete.then(() => this._pipelineTask);
+  }
+
   @property({ type: Boolean, reflect: true, attribute: 'adopt-root-styles' })
   public adoptRootStyles = false;
 
@@ -258,7 +280,7 @@ export class IgcGridLite<T extends object = any> extends EventEmitterBase<IgcGri
    */
   @property({ attribute: false })
   public get sortingExpressions(): SortingExpression<T>[] {
-    return Array.from(this._stateController.sorting.state.values());
+    return Array.from(this._stateController.sorting.state.values(), (expr) => ({ ...expr }));
   }
 
   /**
@@ -278,9 +300,25 @@ export class IgcGridLite<T extends object = any> extends EventEmitterBase<IgcGri
    */
   @property({ attribute: false })
   public get filterExpressions(): FilterExpression<T>[] {
-    return this._stateController.filtering.state.values.flatMap((each) => each.all);
+    return this._stateController.filtering.state.values.flatMap((each) =>
+      each.all.map((expr) => ({ ...expr }))
+    );
+  }
+
+  /**
+   * Sets the column configuration for the grid.
+   *
+   * @remarks
+   * Passing an empty collection resets the columns, which - with `autoGenerate` -
+   * lets the next data source binding re-generate them.
+   */
+  public set columns(configuration: ColumnConfiguration<T>[]) {
+    this._stateController.setColumnConfiguration(asArray(configuration));
   }
 
+  /**
+   * Returns the column configuration of the grid.
+   */
   public get columns(): ColumnConfiguration<T>[] {
     return this._stateController.columns.map((col) => ({ ...col }));
   }
@@ -323,9 +361,35 @@ export class IgcGridLite<T extends object = any> extends EventEmitterBase<IgcGri
     }
   }
 
+  /**
+   * NOTE: The `PIPELINE` sentinel resolves to this method's own name, which is what makes
+   * the `@watch` comparison (`undefined !== <method>`) fire on every requested update.
+   * Renaming the method silently disables the pipeline.
+   */
   @watch(PIPELINE)
-  protected async pipeline() {
-    this._dataState = await this._dataController.apply([...this.data], this._stateController);
+  protected pipeline(): void {
+    this._pipelineTask = this._runPipeline();
+  }
+
+  /** Runs the data operations, discarding a result superseded by a newer run. */
+  private async _runPipeline(): Promise<void> {
+    const epoch = ++this._pipelineEpoch;
+
+    try {
+      const state = await this._dataController.apply([...this.data], this._stateController);
+
+      if (epoch !== this._pipelineEpoch) {
+        return;
+      }
+
+      this._dataState = state;
+    } catch (e) {
+      // A failing hook must not blank the grid - keep the previous data state and report.
+      // biome-ignore lint/suspicious/noConsole: the pipeline hooks are user code; swallowing their errors hides bugs
+      console.error(e);
+    }
+
+    await this.updateComplete;
   }
 
   constructor() {
@@ -371,10 +435,10 @@ export class IgcGridLite<T extends object = any> extends EventEmitterBase<IgcGri
    * Performs a filter operation in the grid based on the passed expression(s).
    */
   public filter(config: FilterExpression<T> | FilterExpression<T>[]): void {
-    const expressions = asArray(config).filter((expr) => {
-      const column = this.getColumn(expr.key);
-      return column !== undefined;
-    });
+    // Copies - the caller's expression objects must not be rewritten.
+    const expressions = asArray(config)
+      .filter((expr) => this.getColumn(expr.key) !== undefined)
+      .map((expr) => ({ ...expr }));
 
     for (const expr of expressions) {
       if (!isString(expr.condition)) {

src/components/header-row.ts

@@ -47,12 +47,14 @@ export default class IgcGridLiteHeaderRow<T extends object> extends LitElement {
     this._state?.filtering.setActiveColumn(header?.column);
   }
 
-  protected override shouldUpdate(props: PropertyValues<this>): boolean {
+  protected override willUpdate(props: PropertyValues<this>): void {
+    // Column configuration is passed by reference, so the headers have to be
+    // refreshed explicitly whenever the row itself updates.
     for (const header of this.headers) {
       header.requestUpdate();
     }
 
-    return super.shouldUpdate(props);
+    super.willUpdate(props);
   }
 
   protected override render() {
@@ -62,7 +64,7 @@ export default class IgcGridLiteHeaderRow<T extends object> extends LitElement {
     return html`
       ${repeat(
         columns,
-        (column) => column,
+        (column) => column.field,
         (column) => html`
           <igc-grid-lite-header
             part=${partMap({ filtered: column.field === filterRow?.column?.field })}

src/components/header.ts

@@ -67,7 +67,7 @@ export default class IgcGridLiteHeader<T extends object> extends LitElement {
   }
 
   protected override update(props: PropertyValues<this>): void {
-    if (props.has('adoptRootStyles')) {
+    if (props.has('adoptRootStyles') || props.has('column')) {
       this._adoptedStylesController.shouldAdoptStyles(
         this.adoptRootStyles && this.column.headerTemplate != null
       );

src/components/row.ts

@@ -51,7 +51,7 @@ export default class IgcGridLiteRow<T extends object> extends LitElement {
     return html`
       ${repeat(
         columns,
-        (column) => column,
+        (column) => column.field,
         (column) => html`
           <igc-grid-lite-cell
             part="cell"

src/controllers/dom.ts

@@ -1,14 +1,37 @@
 import type { ReactiveController } from 'lit';
 import type { StyleInfo } from 'lit/directives/style-map.js';
+import type IgcVirtualizer from '../components/virtualizer.js';
 import { registerGridIcons } from '../internal/icon-registry.js';
 import type { GridHost } from '../internal/types.js';
 import { applyColumnWidths } from '../internal/utils.js';
 import type { StateController } from './state.js';
 
+const SCROLLBAR_OFFSET_VAR = '--scrollbar-offset';
+const VISIBILITY_CHANGED = 'visibilityChanged';
+
 class GridDOMController<T extends object> implements ReactiveController {
   protected readonly _host: GridHost<T>;
   protected readonly _state: StateController<T>;
 
+  /** The virtualizer currently being tracked for scrollbar changes. */
+  #observed?: IgcVirtualizer;
+  #resizeObserver?: ResizeObserver;
+
+  /** Last written offset in pixels. -1 marks "never measured". */
+  #scrollOffset = -1;
+  #pendingFrame?: number;
+
+  #onScrollbarChange = (): void => {
+    if (this.#pendingFrame !== undefined) {
+      return;
+    }
+
+    this.#pendingFrame = requestAnimationFrame(() => {
+      this.#pendingFrame = undefined;
+      this.#applyScrollOffset();
+    });
+  };
+
   constructor(host: GridHost<T>, state: StateController<T>) {
     this._host = host;
     this._state = state;
@@ -21,27 +44,60 @@ class GridDOMController<T extends object> implements ReactiveController {
     registerGridIcons();
     this.setGridColumnSizes();
 
+    // The virtualizer is part of the host template - wait for it before observing.
     this._host.updateComplete.then(() => {
-      this._state.virtualizer?.addEventListener(
-        'visibilityChanged',
-        () => {
-          this.setScrollOffset();
-        },
-        { once: true }
-      );
+      this.#observeVirtualizer();
     });
   }
 
+  public hostDisconnected(): void {
+    if (this.#pendingFrame !== undefined) {
+      cancelAnimationFrame(this.#pendingFrame);
+      this.#pendingFrame = undefined;
+    }
+
+    this.#resizeObserver?.disconnect();
+    this.#resizeObserver = undefined;
+
+    this.#observed?.removeEventListener(VISIBILITY_CHANGED, this.#onScrollbarChange);
+    this.#observed = undefined;
+  }
+
   public hostUpdate(): void {
-    this.setScrollOffset();
     this.setGridColumnSizes();
   }
 
-  public setScrollOffset(): void {
-    const size = this._state.virtualizer
-      ? this._state.virtualizer.offsetWidth - this._state.virtualizer.clientWidth
-      : 0;
-    this._host.style.setProperty('--scrollbar-offset', `${size}px`);
+  /**
+   * Watches the virtualizer for anything which can toggle its scrollbar: a new
+   * visible range, or a content box resize - the scrollbar itself shrinks the
+   * content box. Keeping the measurement here avoids a forced layout on every
+   * host update.
+   */
+  #observeVirtualizer(): void {
+    const virtualizer = this._state.virtualizer;
+
+    if (!virtualizer || virtualizer === this.#observed) {
+      return;
+    }
+
+    this.#observed = virtualizer;
+    virtualizer.addEventListener(VISIBILITY_CHANGED, this.#onScrollbarChange);
+
+    this.#resizeObserver = new ResizeObserver(this.#onScrollbarChange);
+    this.#resizeObserver.observe(virtualizer);
+  }
+
+  /** Writes the scrollbar offset CSS variable, but only when the measurement changed. */
+  #applyScrollOffset(): void {
+    const virtualizer = this.#observed;
+    const offset = virtualizer ? virtualizer.offsetWidth - virtualizer.clientWidth : 0;
+
+    if (offset === this.#scrollOffset) {
+      return;
+    }
+
+    this.#scrollOffset = offset;
+    this._host.style.setProperty(SCROLLBAR_OFFSET_VAR, `${offset}px`);
   }
 
   protected setGridColumnSizes(): void {