-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(google-maps): maintain subscriptions across event targets (#20897)
This came up during the review of #20873. Currently the class that manages events for the Google Maps objects assumes that once a target is assigned, it'll either be maintained or eventually removed which means that the fix from #20873 will cause any existing event listeners to be dropped. These changes add some code which will preserve the listeners. Furthermore, I've added a dedicated testing setup for the `MapEventManager` since there's a fair bit of logic encapsulated in it and we've only been testing it by proxy through the various Google Maps components. (cherry picked from commit cc503c4)
- Loading branch information
1 parent
8638456
commit 38dc511
Showing
2 changed files
with
194 additions
and
19 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,170 @@ | ||
import {NgZone} from '@angular/core'; | ||
import {MapEventManager} from './map-event-manager'; | ||
|
||
describe('MapEventManager', () => { | ||
let dummyZone: NgZone; | ||
let manager: MapEventManager; | ||
let target: TestEventTarget; | ||
|
||
beforeEach(() => { | ||
dummyZone = { | ||
run: jasmine.createSpy('NgZone.run').and.callFake((callback: () => void) => callback()) | ||
} as unknown as NgZone; | ||
target = new TestEventTarget(); | ||
manager = new MapEventManager(dummyZone); | ||
}); | ||
|
||
afterEach(() => { | ||
manager.destroy(); | ||
}); | ||
|
||
it('should register a listener when subscribing to an event', () => { | ||
expect(target.addListener).not.toHaveBeenCalled(); | ||
|
||
manager.setTarget(target); | ||
const stream = manager.getLazyEmitter('click'); | ||
|
||
expect(target.addListener).not.toHaveBeenCalled(); | ||
expect(target.events.get('click')).toBeFalsy(); | ||
|
||
const subscription = stream.subscribe(); | ||
expect(target.addListener).toHaveBeenCalledTimes(1); | ||
expect(target.events.get('click')?.size).toBe(1); | ||
subscription.unsubscribe(); | ||
}); | ||
|
||
it('should register a listener if the subscription happened before there was a target', () => { | ||
const stream = manager.getLazyEmitter('click'); | ||
const subscription = stream.subscribe(); | ||
|
||
expect(target.addListener).not.toHaveBeenCalled(); | ||
expect(target.events.get('click')).toBeFalsy(); | ||
|
||
manager.setTarget(target); | ||
|
||
expect(target.addListener).toHaveBeenCalledTimes(1); | ||
expect(target.events.get('click')?.size).toBe(1); | ||
subscription.unsubscribe(); | ||
}); | ||
|
||
it('should remove the listener when unsubscribing', () => { | ||
const stream = manager.getLazyEmitter('click'); | ||
const subscription = stream.subscribe(); | ||
manager.setTarget(target); | ||
expect(target.events.get('click')?.size).toBe(1); | ||
|
||
subscription.unsubscribe(); | ||
expect(target.events.get('click')?.size).toBe(0); | ||
}); | ||
|
||
it('should remove the listener when the manager is destroyed', () => { | ||
const stream = manager.getLazyEmitter('click'); | ||
stream.subscribe(); | ||
manager.setTarget(target); | ||
expect(target.events.get('click')?.size).toBe(1); | ||
|
||
manager.destroy(); | ||
expect(target.events.get('click')?.size).toBe(0); | ||
}); | ||
|
||
it('should remove the listener when the target is changed', () => { | ||
const stream = manager.getLazyEmitter('click'); | ||
stream.subscribe(); | ||
manager.setTarget(target); | ||
expect(target.events.get('click')?.size).toBe(1); | ||
|
||
manager.setTarget(undefined); | ||
expect(target.events.get('click')?.size).toBe(0); | ||
}); | ||
|
||
it('should trigger the subscription to an event', () => { | ||
const spy = jasmine.createSpy('subscription'); | ||
const stream = manager.getLazyEmitter('click'); | ||
stream.subscribe(spy); | ||
manager.setTarget(target); | ||
expect(spy).not.toHaveBeenCalled(); | ||
|
||
target.triggerListeners('click'); | ||
expect(spy).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
it('should be able to register multiple listeners to the same event', () => { | ||
const firstSpy = jasmine.createSpy('subscription one'); | ||
const secondSpy = jasmine.createSpy('subscription two'); | ||
const stream = manager.getLazyEmitter('click'); | ||
manager.setTarget(target); | ||
stream.subscribe(firstSpy); | ||
stream.subscribe(secondSpy); | ||
expect(firstSpy).not.toHaveBeenCalled(); | ||
expect(secondSpy).not.toHaveBeenCalled(); | ||
expect(target.events.get('click')?.size).toBe(2); | ||
|
||
target.triggerListeners('click'); | ||
expect(firstSpy).toHaveBeenCalledTimes(1); | ||
expect(secondSpy).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
it('should run listeners inside the NgZone', () => { | ||
const spy = jasmine.createSpy('subscription'); | ||
const stream = manager.getLazyEmitter('click'); | ||
stream.subscribe(spy); | ||
manager.setTarget(target); | ||
expect(dummyZone.run).not.toHaveBeenCalled(); | ||
|
||
target.triggerListeners('click'); | ||
expect(dummyZone.run).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
it('should maintain subscriptions when swapping out targets', () => { | ||
const spy = jasmine.createSpy('subscription'); | ||
const stream = manager.getLazyEmitter('click'); | ||
stream.subscribe(spy); | ||
manager.setTarget(target); | ||
expect(spy).not.toHaveBeenCalled(); | ||
|
||
target.triggerListeners('click'); | ||
expect(spy).toHaveBeenCalledTimes(1); | ||
|
||
const alternateTarget = new TestEventTarget(); | ||
manager.setTarget(alternateTarget); | ||
|
||
expect(spy).toHaveBeenCalledTimes(1); | ||
expect(target.events.get('click')?.size).toBe(0); | ||
expect(alternateTarget.events.get('click')?.size).toBe(1); | ||
|
||
alternateTarget.triggerListeners('click'); | ||
expect(spy).toHaveBeenCalledTimes(2); | ||
|
||
manager.setTarget(undefined); | ||
expect(alternateTarget.events.get('click')?.size).toBe(0); | ||
|
||
alternateTarget.triggerListeners('click'); | ||
expect(spy).toHaveBeenCalledTimes(2); | ||
}); | ||
|
||
}); | ||
|
||
/** Imitates a Google Maps event target and keeps track of the registered events. */ | ||
class TestEventTarget { | ||
events = new Map<string, Set<() => void>>(); | ||
|
||
addListener = jasmine.createSpy('addListener').and.callFake( | ||
(name: string, listener: () => void) => { | ||
if (!this.events.has(name)) { | ||
this.events.set(name, new Set()); | ||
} | ||
this.events.get(name)!.add(listener); | ||
|
||
return {remove: () => this.events.get(name)!.delete(listener)}; | ||
}); | ||
|
||
triggerListeners(name: string) { | ||
const listeners = this.events.get(name); | ||
|
||
if (!listeners) { | ||
throw Error(`No listeners registered for "${name}" event.`); | ||
} | ||
|
||
listeners.forEach(listener => listener()); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters