diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts index 513e159f7aa..859ad2b55d7 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.spec.ts @@ -211,32 +211,40 @@ describe("VaultPopupItemsService", () => { }); }); - it("should update cipher list when cipherService.ciphers$ emits", async () => { + it("should not re-emit autoFillCiphers$ when cipherService.ciphers$ emits but the resulting list is unchanged", async () => { const tracker = new ObservableTracker(service.autoFillCiphers$); + // Initial emission await tracker.expectEmission(); + const initial = tracker.emissions[0]; + // Trigger a ciphers$ emission that doesn't actually change the final list ciphersSubject.next({}); - await tracker.expectEmission(); + // Ensure we do NOT get a second emission + await expect(tracker.pauseUntilReceived(2)).rejects.toThrow("Timeout exceeded"); + expect(tracker.emissions.length).toBe(1); - // Should only emit twice - expect(tracker.emissions.length).toBe(2); - await expect(tracker.pauseUntilReceived(3)).rejects.toThrow("Timeout exceeded"); + // Sanity check: still the same array of ciphers + expect(tracker.emissions[0]).toBe(initial); }); - it("should update cipher list when cipherService.localData$ emits", async () => { + it("should not re-emit autoFillCiphers$ when cipherService.localData$ emits but the resulting list is unchanged", async () => { const tracker = new ObservableTracker(service.autoFillCiphers$); + // Initial emission await tracker.expectEmission(); + const initial = tracker.emissions[0]; + // Trigger a localData$ emission that doesn't actually change the final list localDataSubject.next({}); - await tracker.expectEmission(); + // Ensure we do NOT get a second emission + await expect(tracker.pauseUntilReceived(2)).rejects.toThrow("Timeout exceeded"); + expect(tracker.emissions.length).toBe(1); - // Should only emit twice - expect(tracker.emissions.length).toBe(2); - await expect(tracker.pauseUntilReceived(3)).rejects.toThrow("Timeout exceeded"); + // Sanity check: still the same array of ciphers + expect(tracker.emissions[0]).toBe(initial); }); it("should not emit cipher list if syncService.getLastSync returns null", async () => { @@ -468,13 +476,9 @@ describe("VaultPopupItemsService", () => { describe("loading$", () => { let tracked: ObservableTracker; - let trackedCiphers: ObservableTracker; beforeEach(() => { // Start tracking loading$ emissions tracked = new ObservableTracker(service.loading$); - - // Track remainingCiphers$ to make cipher observables active - trackedCiphers = new ObservableTracker(service.remainingCiphers$); }); it("should initialize with true first", async () => { @@ -482,22 +486,28 @@ describe("VaultPopupItemsService", () => { }); it("should emit false once ciphers are available", async () => { - expect(tracked.emissions.length).toBe(2); + await tracked.pauseUntilReceived(2); + expect(tracked.emissions[0]).toBe(true); expect(tracked.emissions[1]).toBe(false); }); - it("should cycle when cipherService.ciphers$ emits", async () => { - // Restart tracking - tracked = new ObservableTracker(service.loading$); + it("should cycle when cipherService.ciphers$ emits even if cipher lists don't re-emit", async () => { + await tracked.pauseUntilReceived(2); + + const before = [...tracked.emissions]; + ciphersSubject.next({}); - await trackedCiphers.pauseUntilReceived(2); + await tracked.pauseUntilReceived(before.length + 2); - expect(tracked.emissions.length).toBe(3); - expect(tracked.emissions[0]).toBe(false); - expect(tracked.emissions[1]).toBe(true); - expect(tracked.emissions[2]).toBe(false); + const after = tracked.emissions; + + expect(after.length).toBeGreaterThan(before.length); + + const tail = after.slice(before.length); + expect(tail[0]).toBe(true); + expect(tail[1]).toBe(false); }); }); diff --git a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts index 321d7936806..d00c07218cb 100644 --- a/apps/browser/src/vault/popup/services/vault-popup-items.service.ts +++ b/apps/browser/src/vault/popup/services/vault-popup-items.service.ts @@ -219,6 +219,7 @@ export class VaultPopupItemsService { return this.cipherService.filterCiphersForUrl(ciphers, tab.url, otherTypes); }), map((ciphers) => ciphers.sort(this.sortCiphersForAutofill.bind(this))), + distinctUntilChanged(ciphersEqualByIdAndOrder), shareReplay({ refCount: false, bufferSize: 1 }), ); @@ -231,6 +232,7 @@ export class VaultPopupItemsService { map(([autoFillCiphers, ciphers]) => ciphers.filter((cipher) => cipher.favorite && !autoFillCiphers.includes(cipher)), ), + distinctUntilChanged(ciphersEqualByIdAndOrder), shareReplay({ refCount: false, bufferSize: 1 }), ); @@ -250,6 +252,7 @@ export class VaultPopupItemsService { (cipher) => !autoFillCiphers.includes(cipher) && !favoriteCiphers.includes(cipher), ), ), + distinctUntilChanged(ciphersEqualByIdAndOrder), shareReplay({ refCount: false, bufferSize: 1 }), ); @@ -257,8 +260,10 @@ export class VaultPopupItemsService { * Observable that indicates whether the service is currently loading ciphers. */ loading$: Observable = merge( + // Turn loading on whenever we start processing ciphers this._ciphersLoading$.pipe(map(() => true)), - this.remainingCiphers$.pipe(map(() => false)), + // Turn loading off whenever the decrypted cipher list finishes updating + this._allDecryptedCiphers$.pipe(map(() => false)), ).pipe(startWith(true), distinctUntilChanged(), shareReplay({ refCount: false, bufferSize: 1 })); /** Observable that indicates whether there is search text present. @@ -398,3 +403,25 @@ export class VaultPopupItemsService { const waitUntilSync = (syncService: SyncService): MonoTypeOperatorFunction => { return waitUntil(syncService.activeUserLastSync$().pipe(filter((lastSync) => lastSync != null))); }; + +const ciphersEqualByIdAndOrder = (a: PopupCipherViewLike[], b: PopupCipherViewLike[]): boolean => { + if (a === b) { + return true; + } + + if (!a || !b) { + return false; + } + + if (a.length !== b.length) { + return false; + } + + for (let i = 0; i < a.length; i++) { + if (a[i]?.id !== b[i]?.id) { + return false; + } + } + + return true; +};