From c5d60ae62e4bbcacdd3a496d17fbd3ce0cfbafba Mon Sep 17 00:00:00 2001 From: Shane McLaughlin Date: Wed, 27 Sep 2023 15:02:18 -0500 Subject: [PATCH] feat: unique listener names for lifecycle events (#941) --- src/lifecycleEvents.ts | 30 ++++++++--- test/unit/lifecycleEventsTest.ts | 88 ++++++++++++++++++++++++++++++-- 2 files changed, 108 insertions(+), 10 deletions(-) diff --git a/src/lifecycleEvents.ts b/src/lifecycleEvents.ts index 362d09703f..64f5a82176 100644 --- a/src/lifecycleEvents.ts +++ b/src/lifecycleEvents.ts @@ -47,7 +47,10 @@ export class Lifecycle { public static readonly warningEventName = 'warning'; private logger?: Logger; - private constructor(private readonly listeners: Dictionary = {}) {} + private constructor( + private readonly listeners: Dictionary = {}, + private readonly uniqueListeners: Map> = new Map>() + ) {} /** * return the package.json version of the sfdx-core library. @@ -87,7 +90,7 @@ export class Lifecycle { const oldInstance = global.salesforceCoreLifecycle; // use the newer version and transfer any listeners from the old version // object spread keeps them from being references - global.salesforceCoreLifecycle = new Lifecycle({ ...oldInstance.listeners }); + global.salesforceCoreLifecycle = new Lifecycle({ ...oldInstance.listeners }, oldInstance.uniqueListeners); // clean up any listeners on the old version Object.keys(oldInstance.listeners).map((eventName) => { oldInstance.removeAllListeners(eventName); @@ -111,6 +114,7 @@ export class Lifecycle { */ public removeAllListeners(eventName: string): void { this.listeners[eventName] = []; + this.uniqueListeners.delete(eventName); } /** @@ -119,7 +123,9 @@ export class Lifecycle { * @param eventName The name of the event to get listeners of */ public getListeners(eventName: string): callback[] { - const listeners = this.listeners[eventName]; + const listeners = this.listeners[eventName]?.concat( + Array.from((this.uniqueListeners.get(eventName) ?? []).values()) ?? [] + ); if (listeners) { return listeners; } else { @@ -150,8 +156,9 @@ export class Lifecycle { * * @param eventName The name of the event that is being listened for * @param cb The callback function to run when the event is emitted + * @param uniqueListenerIdentifier A unique identifier for the listener. If a listener with the same identifier is already registered, a new one will not be added */ - public on(eventName: string, cb: (data: T) => Promise): void { + public on(eventName: string, cb: (data: T) => Promise, uniqueListenerIdentifier?: string): void { const listeners = this.getListeners(eventName); if (listeners.length !== 0) { if (!this.logger) { @@ -165,8 +172,19 @@ export class Lifecycle { } listeners will fire.` ); } - listeners.push(cb); - this.listeners[eventName] = listeners; + + if (uniqueListenerIdentifier) { + if (!this.uniqueListeners.has(eventName)) { + // nobody is listening to the event yet + this.uniqueListeners.set(eventName, new Map([[uniqueListenerIdentifier, cb]])); + } else if (!this.uniqueListeners.get(eventName)?.has(uniqueListenerIdentifier)) { + // the unique listener identifier is not already registered + this.uniqueListeners.get(eventName)?.set(uniqueListenerIdentifier, cb); + } + } else { + listeners.push(cb); + this.listeners[eventName] = listeners; + } } /** diff --git a/test/unit/lifecycleEventsTest.ts b/test/unit/lifecycleEventsTest.ts index d5fa48c666..d84a4a9121 100644 --- a/test/unit/lifecycleEventsTest.ts +++ b/test/unit/lifecycleEventsTest.ts @@ -69,6 +69,45 @@ describe('lifecycleEvents', () => { Lifecycle.getInstance().removeAllListeners('telemetry'); }); + it("uniqueListeners adds listeners only when they don't already exist", async () => { + Lifecycle.getInstance().on( + 'test1', + async (result) => { + // @ts-expect-error: called is a sinon spy property + fake.bar('test1', result); + }, + 'id000' + ); + chai.expect(Lifecycle.getInstance().getListeners('test1').length).to.equal(1); + Lifecycle.getInstance().on( + 'test1', + async (result) => { + // @ts-expect-error: called is a sinon spy property + fake.bar('test1', result); + }, + 'id001' + ); + // both of these should be ignored + chai.expect(Lifecycle.getInstance().getListeners('test1').length).to.equal(2); + Lifecycle.getInstance().on( + 'test1', + async (result) => { + // @ts-expect-error: called is a sinon spy property + fake.bar('test1', result); + }, + 'id000' + ); + chai.expect(Lifecycle.getInstance().getListeners('test1').length).to.equal(2); + Lifecycle.getInstance().on( + 'test1', + async (result) => { + // @ts-expect-error: called is a sinon spy property + fake.bar('test1', result); + }, + 'id001' + ); + Lifecycle.getInstance().removeAllListeners('test1'); + }); it('successful event registration and emitting causes the callback to be called', async () => { Lifecycle.getInstance().on('test1', async (result) => { // @ts-expect-error: called is a sinon spy property @@ -131,13 +170,24 @@ describe('lifecycleEvents', () => { // @ts-expect-error: called is a sinon spy property fake.bar('test5', result); }); + Lifecycle.getInstance().on( + 'test5', + async (result) => { + // @ts-expect-error: called is a sinon spy property + fake.bar('test5', result); + }, + 'id000' + ); await Lifecycle.getInstance().emit('test5', 'Success'); - chai.expect(fakeSpy.callCount).to.be.equal(1); + chai.expect(fakeSpy.callCount).to.be.equal(2); chai.expect(fakeSpy.args[0][1]).to.be.equal('Success'); - + chai.expect(fakeSpy.args[1][1]).to.be.equal('Success'); + loggerSpy.resetHistory(); + fakeSpy.resetHistory(); Lifecycle.getInstance().removeAllListeners('test5'); await Lifecycle.getInstance().emit('test5', 'Failure: Listener Removed'); - chai.expect(fakeSpy.callCount).to.be.equal(1); + chai.expect(fakeSpy.callCount).to.be.equal(0); + chai.expect(loggerSpy.callCount).to.be.equal(1); chai .expect(loggerSpy.args[0][0]) @@ -146,7 +196,7 @@ describe('lifecycleEvents', () => { ); }); - it('getListeners works', async () => { + it('getListeners works, including uniqueListeners', async () => { const x = async (result: Record) => { // eslint-disable-next-line @typescript-eslint/no-unsafe-argument fake.bar('test6', result); @@ -159,6 +209,36 @@ describe('lifecycleEvents', () => { Lifecycle.getInstance().removeAllListeners('test6'); }); + it('getListeners works (unique Listeners)', async () => { + const x = async (result: Record) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + fake.bar('test6', result); + }; + Lifecycle.getInstance().on('test6', x, 'id000'); + + chai.expect(Lifecycle.getInstance().getListeners('test6')).to.have.lengthOf(1); + chai.expect(Lifecycle.getInstance().getListeners('test6')[0]).to.deep.equal(x); + + chai.expect(Lifecycle.getInstance().getListeners('undefinedKey').length).to.be.equal(0); + Lifecycle.getInstance().removeAllListeners('test6'); + }); + + it('getListeners works (mixed unique and non-unique)', async () => { + const x = async (result: Record) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-argument + fake.bar('test6', result); + }; + Lifecycle.getInstance().on('test6', x); + Lifecycle.getInstance().on('test6', x, 'id000'); + + chai.expect(Lifecycle.getInstance().getListeners('test6')).to.have.lengthOf(2); + chai.expect(Lifecycle.getInstance().getListeners('test6')[0]).to.deep.equal(x); + chai.expect(Lifecycle.getInstance().getListeners('test6')[1]).to.deep.equal(x); + + chai.expect(Lifecycle.getInstance().getListeners('undefinedKey').length).to.be.equal(0); + Lifecycle.getInstance().removeAllListeners('test6'); + }); + it('will use a newer version and transfer the listeners', () => { // the original const lifecycle = Lifecycle.getInstance();