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.7.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/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));

test/CollectionSignal.test.ts

@@ -8,6 +8,7 @@ import {
   newTeslaModel3,
   newTeslaModelX,
   usedChevyCamero,
+  usedTeslaModel3,
 } from './shared/data';
 
 describe('Collection signal tests', () => {
@@ -115,5 +116,23 @@ describe('Collection signal tests', () => {
     test('Should trigger no update signal in view', () => {
       expect(updateViewSignal).toHaveBeenCalledTimes(0);
     });
+
+    describe('When unregister add signal', () => {
+      beforeEach(() => {
+        changeViewSignal.mockClear();
+        changeSignal.mockClear();
+        carCollection.unregisterObserver(changeSignal);
+        usedCarCollection.unregisterObserver(changeViewSignal);
+        carCollection.add(usedTeslaModel3);
+      });
+
+      test("should not trigger collection's change signal", () => {
+        expect(changeSignal).toHaveBeenCalledTimes(0);
+      });
+
+      test("should not trigger view's change signal", () => {
+        expect(changeViewSignal).toHaveBeenCalledTimes(0);
+      });
+    });
   });
 });

test/collectionview.test.ts

@@ -1,29 +1,48 @@
+import { CollectionViewBase } from '../src';
 import {
   CarCollection,
   UsedCarCollectionView,
   UsedGasCarCollectionView,
 } from './shared/collections';
 import {
+  ICar,
   allCars,
   usedChevyCamero,
   usedChevyEquinox,
   usedTeslaModel3,
   usedTeslaModelX,
 } from './shared/data';
 
+/**
+ * View with out any filter or sort set
+ */
+class DefaultView extends CollectionViewBase<ICar, CarCollection> {
+  constructor(source: CarCollection) {
+    super(source);
+  }
+}
+
 describe('collection view tests', () => {
   let cars: CarCollection;
   let usedCars: UsedCarCollectionView;
   let usedGasCars: UsedGasCarCollectionView;
+  let defaultView: DefaultView;
 
   beforeEach(() => {
     cars = new CarCollection();
     cars.addRange(allCars);
 
+    defaultView = new DefaultView(cars);
     usedCars = new UsedCarCollectionView(cars);
     usedGasCars = new UsedGasCarCollectionView(usedCars);
   });
 
+  describe('DefaultView', () => {
+    it('defaultView has same number of items as collection', () => {
+      expect(defaultView.count).toEqual(cars.count);
+    });
+  });
+
   // Tests against index which is only based on one value
   describe('OneLevelIndex', () => {
     it('usedCars.byMake(Tesla) should return all used Tesla cars', () => {
@@ -69,5 +88,13 @@ describe('collection view tests', () => {
         new Set([usedTeslaModelX])
       );
     });
+
+    it('exist should return false with the removed car', () => {
+      expect(usedCars.exists(usedTeslaModel3)).toEqual(false);
+    });
+
+    it('exist should return true with cars not removed', () => {
+      expect(usedCars.exists(usedTeslaModelX)).toEqual(true);
+    });
   });
 });