fix: PrimaryKeyCollection not include idIndex when buildIndex is called

Summary:

Test Plan:

jest.config.js

@@ -5,5 +5,6 @@ module.exports = {
   collectCoverage: true,
   verbose: true,
   coverageReporters: ['cobertura', 'html', 'lcov'],
+  coveragePathIgnorePatterns: ['index.ts$'],
   testTimeout: 500,
 };

package.json

@@ -1,6 +1,6 @@
 {
   "name": "indexed-collection",
-  "version": "1.6.0",
+  "version": "1.8.0",
   "description": "A zero-dependency library of classes that make filtering, sorting and observing changes to arrays easier and more efficient.",
   "license": "MIT",
   "keywords": [

src/collections/CollectionViewBase.ts

@@ -1,23 +1,30 @@
+import { ICollectionAndView } from '../core/ICollectionAndView';
+import { ICollectionViewOption } from '../core/ICollectionViewOption';
 import { IReadonlyCollection } from '../core/IReadonlyCollection';
 import { Optional } from '../core/Optional';
-import { CollectionChangeSignal } from '../signals/CollectionChangeSignal';
-import { SignalObserver } from '../signals/SignalObserver';
-import { ICollectionViewOption } from '../core/ICollectionViewOption';
 import {
-  defaultSort,
   defaultCollectionViewOption,
+  defaultSort,
 } from '../core/defaultCollectionViewOption';
-import { ICollectionAndView } from '../core/ICollectionAndView';
+import { CollectionAddSignal } from '../signals/CollectionAddSignal';
+import { CollectionChangeSignal } from '../signals/CollectionChangeSignal';
+import { CollectionRemoveSignal } from '../signals/CollectionRemoveSignal';
+import { CollectionUpdateSignal } from '../signals/CollectionUpdateSignal';
+import { SignalObserver } from '../signals/SignalObserver';
+import { filterCollectionChangeDetail } from './util';
 
 /**
  * CollectionView is a view onto a collection of data.  Most common use case would be
  * having the collection reduced by filter and/or sorted according to various criteria
  * without modifying the underlying data.
  */
 export abstract class CollectionViewBase<
-  T,
-  SourceCollectionT extends ICollectionAndView<T>
-> extends SignalObserver implements IReadonlyCollection<T> {
+    T,
+    SourceCollectionT extends ICollectionAndView<T>
+  >
+  extends SignalObserver
+  implements IReadonlyCollection<T>
+{
   private readonly _source: SourceCollectionT;
   private readonly _option: ICollectionViewOption<T>;
   private _cachedItems: T[] = [];
@@ -66,13 +73,35 @@ export abstract class CollectionViewBase<
     return this.filter(item) ? item : undefined;
   }
 
-  protected source_onChange(): void {
+  protected source_onChange(signal: CollectionChangeSignal<T>): void {
     this.rebuildCache();
-    this.notifyChange();
+    this.notifyChange(signal);
   }
 
-  public notifyChange(): void {
-    this.notifyObservers(new CollectionChangeSignal(this));
+  public notifyChange(signal: CollectionChangeSignal<T>): void {
+    const changes = filterCollectionChangeDetail(signal.detail, this.filter);
+
+    const addedCount = changes.added.length;
+    const removedCount = changes.removed.length;
+    const updatedCount = changes.updated.length;
+
+    if (addedCount === 0 && removedCount === 0 && updatedCount === 0) {
+      return;
+    }
+
+    this.notifyObservers(new CollectionChangeSignal(this, changes));
+
+    if (addedCount > 0) {
+      this.notifyObservers(new CollectionAddSignal(this, changes.added));
+    }
+
+    if (removedCount > 0) {
+      this.notifyObservers(new CollectionRemoveSignal(this, changes.removed));
+    }
+
+    if (updatedCount > 0) {
+      this.notifyObservers(new CollectionUpdateSignal(this, changes.updated));
+    }
   }
 
   get count(): number {

src/collections/IndexedCollectionBase.ts

@@ -1,4 +1,5 @@
 import { CollectionNature } from '../core/CollectionNature';
+import { ICollectionChangeDetail } from '../core/ICollectionChangeDetail';
 import { ICollectionOption } from '../core/ICollectionOption';
 import { IIndex } from '../core/IIndex';
 import { IMutableCollection } from '../core/IMutableCollection';
@@ -7,8 +8,12 @@ import { defaultCollectionOption } from '../core/defaultCollectionOption';
 import { IInternalList } from '../core/internals/IInternalList';
 import { InternalList } from '../core/internals/InternalList';
 import { InternalSetList } from '../core/internals/InternalSetList';
+import { CollectionAddSignal } from '../signals/CollectionAddSignal';
 import { CollectionChangeSignal } from '../signals/CollectionChangeSignal';
+import { CollectionRemoveSignal } from '../signals/CollectionRemoveSignal';
+import { CollectionUpdateSignal } from '../signals/CollectionUpdateSignal';
 import { SignalObserver } from '../signals/SignalObserver';
+import { mergeCollectionChangeDetail } from './util';
 
 export abstract class IndexedCollectionBase<T>
   extends SignalObserver
@@ -20,6 +25,7 @@ export abstract class IndexedCollectionBase<T>
 
   private _pauseChangeSignal: boolean = false;
   private _hasPendingChangeSignal: boolean = false;
+  private _pendingChange: Partial<ICollectionChangeDetail<T>> = {};
 
   public readonly option: Readonly<ICollectionOption>;
 
@@ -83,17 +89,15 @@ export abstract class IndexedCollectionBase<T>
     for (const index of this.indexes) {
       index.index(item);
     }
-    this.notifyChange();
+    this.notifyChange({
+      added: [item],
+    });
     return true;
   }
 
   public addRange(
     items: readonly T[] | IReadonlyCollection<T> | ReadonlySet<T>
   ): boolean[] {
-    // const rawItems: readonly T[] = Array.isArray(items)
-    //   ? items
-    //   : (items as IReadonlyCollection<T>).items;
-
     let rawItems: Readonly<Iterable<T>>;
     if (Array.isArray(items)) {
       rawItems = items;
@@ -131,7 +135,9 @@ export abstract class IndexedCollectionBase<T>
     for (const index of this.indexes) {
       index.unIndex(item);
     }
-    this.notifyChange();
+    this.notifyChange({
+      removed: [item],
+    });
     return true;
   }
 
@@ -149,7 +155,9 @@ export abstract class IndexedCollectionBase<T>
     for (const index of this.indexes) {
       index.index(newItem);
     }
-    this.notifyChange();
+    this.notifyChange({
+      updated: [{ oldValue: oldItem, newValue: newItem }],
+    });
     return true;
   }
 
@@ -161,12 +169,30 @@ export abstract class IndexedCollectionBase<T>
     return this._allItemList.count;
   }
 
-  protected notifyChange(): void {
+  protected notifyChange(change: Partial<ICollectionChangeDetail<T>>): void {
     if (this._pauseChangeSignal) {
       this._hasPendingChangeSignal = true;
+      this._pendingChange = mergeCollectionChangeDetail(
+        this._pendingChange,
+        change
+      );
       return;
     }
-    this.notifyObservers(new CollectionChangeSignal(this));
+
+    const changes = mergeCollectionChangeDetail({}, change);
+    this.notifyObservers(new CollectionChangeSignal(this, changes));
+
+    if (changes.added.length > 0) {
+      this.notifyObservers(new CollectionAddSignal(this, changes.added));
+    }
+
+    if (changes.removed.length > 0) {
+      this.notifyObservers(new CollectionRemoveSignal(this, changes.removed));
+    }
+
+    if (changes.updated.length > 0) {
+      this.notifyObservers(new CollectionUpdateSignal(this, changes.updated));
+    }
   }
 
   /**
@@ -194,7 +220,8 @@ export abstract class IndexedCollectionBase<T>
       this._pauseChangeSignal = false;
       if (this._hasPendingChangeSignal) {
         this._hasPendingChangeSignal = false;
-        this.notifyChange();
+        this.notifyChange(this._pendingChange);
+        this._pendingChange = {};
       }
     }
   }

src/collections/PrimaryKeyCollection.ts

@@ -28,6 +28,21 @@ export class PrimaryKeyCollection<
     }
   }
 
+  protected override buildIndexes(
+    indexes: readonly IIndex<T>[],
+    autoReindex?: boolean
+  ): void {
+    const combinedIndex: IIndex<T>[] = [];
+    if (this.idIndex != null) {
+      // this.idIndex can be null during instantiation
+      combinedIndex.push(this.idIndex);
+    }
+    if (indexes != null && indexes.length > 0) {
+      combinedIndex.push(...indexes);
+    }
+    super.buildIndexes(combinedIndex, autoReindex);
+  }
+
   exists(item: T): boolean {
     const key = this.primaryKeyExtract(item);
     return Boolean(this.byPrimaryKey(key));

src/collections/util.ts

@@ -0,0 +1,41 @@
+import { ICollectionChangeDetail } from '../core/ICollectionChangeDetail';
+
+export function mergeCollectionChangeDetail<T>(
+  a: Partial<ICollectionChangeDetail<T>>,
+  b: Partial<ICollectionChangeDetail<T>>
+): ICollectionChangeDetail<T> {
+  const added = a.added
+    ? b.added
+      ? a.added.concat(b.added)
+      : a.added
+    : b.added;
+  const removed = a.removed
+    ? b.removed
+      ? a.removed.concat(b.removed)
+      : a.removed
+    : b.removed;
+  const updated = a.updated
+    ? b.updated
+      ? a.updated.concat(b.updated)
+      : a.updated
+    : b.updated;
+  return {
+    added: added ?? [],
+    removed: removed ?? [],
+    updated: updated ?? [],
+  };
+}
+
+export function filterCollectionChangeDetail<T>(
+  change: Readonly<ICollectionChangeDetail<T>>,
+  filter: (item: T) => boolean
+): ICollectionChangeDetail<T> {
+  const added = change.added.filter(filter);
+  const removed = change.removed.filter(filter);
+  const updated = change.updated.filter((item) => filter(item.newValue));
+  return {
+    added,
+    removed,
+    updated,
+  };
+}

src/core/ICollectionChangeDetail.ts

@@ -0,0 +1,10 @@
+export interface ICollectionUpdateLineItem<T> {
+  oldValue: T;
+  newValue: T;
+}
+
+export interface ICollectionChangeDetail<T> {
+  readonly added: T[];
+  readonly removed: T[];
+  readonly updated: ICollectionUpdateLineItem<T>[];
+}

src/signals/CollectionAddSignal.ts

@@ -0,0 +1,12 @@
+import { IReadonlyCollection } from '../core/IReadonlyCollection';
+import { Signal } from './Signal';
+
+export class CollectionAddSignal<T> extends Signal {
+  static readonly type = Symbol('COLLECTION_ADD');
+  constructor(
+    target: IReadonlyCollection<T>,
+    public readonly added: readonly T[]
+  ) {
+    super(CollectionAddSignal.type, target);
+  }
+}

src/signals/CollectionChangeSignal.ts

@@ -1,9 +1,13 @@
-import { Signal } from './Signal';
+import { ICollectionChangeDetail } from '../core/ICollectionChangeDetail';
 import { IReadonlyCollection } from '../core/IReadonlyCollection';
+import { Signal } from './Signal';
 
 export class CollectionChangeSignal<T> extends Signal {
   static readonly type = Symbol('COLLECTION_CHANGE');
-  constructor(target: IReadonlyCollection<T>) {
+  constructor(
+    target: IReadonlyCollection<T>,
+    public readonly detail: Readonly<ICollectionChangeDetail<T>>
+  ) {
     super(CollectionChangeSignal.type, target);
   }
 }

src/signals/CollectionRemoveSignal.ts

@@ -0,0 +1,12 @@
+import { IReadonlyCollection } from '../core/IReadonlyCollection';
+import { Signal } from './Signal';
+
+export class CollectionRemoveSignal<T> extends Signal {
+  static readonly type = Symbol('COLLECTION_REMOVE');
+  constructor(
+    target: IReadonlyCollection<T>,
+    public readonly removed: readonly T[]
+  ) {
+    super(CollectionRemoveSignal.type, target);
+  }
+}

src/signals/CollectionUpdateSignal.ts

@@ -0,0 +1,13 @@
+import { ICollectionUpdateLineItem } from '../core/ICollectionChangeDetail';
+import { IReadonlyCollection } from '../core/IReadonlyCollection';
+import { Signal } from './Signal';
+
+export class CollectionUpdateSignal<T> extends Signal {
+  static readonly type = Symbol('COLLECTION_UPDATE');
+  constructor(
+    target: IReadonlyCollection<T>,
+    public readonly updated: readonly Readonly<ICollectionUpdateLineItem<T>>[]
+  ) {
+    super(CollectionUpdateSignal.type, target);
+  }
+}

src/signals/index.ts

@@ -1,4 +1,7 @@
+export { CollectionAddSignal } from './CollectionAddSignal';
 export { CollectionChangeSignal } from './CollectionChangeSignal';
+export { CollectionRemoveSignal } from './CollectionRemoveSignal';
+export { CollectionUpdateSignal } from './CollectionUpdateSignal';
 export { SignalHandler } from './ISignalObserver';
 export { Signal, SignalType } from './Signal';
 export { SignalObserver } from './SignalObserver';