Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: unique listener names for lifecycle events #941

Merged
merged 1 commit into from
Sep 27, 2023
Merged

feat: unique listener names for lifecycle events #941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions src/lifecycleEvents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ export class Lifecycle {
public static readonly warningEventName = 'warning';
private logger?: Logger;

private constructor(private readonly listeners: Dictionary<callback[]> = {}) {}
private constructor(
private readonly listeners: Dictionary<callback[]> = {},
private readonly uniqueListeners: Map<string, Map<string, callback>> = new Map<string, Map<string, callback>>()
) {}

/**
* return the package.json version of the sfdx-core library.
Expand Down Expand Up @@ -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);
Expand All @@ -111,6 +114,7 @@ export class Lifecycle {
*/
public removeAllListeners(eventName: string): void {
this.listeners[eventName] = [];
this.uniqueListeners.delete(eventName);
}

/**
Expand All @@ -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 {
Expand Down Expand Up @@ -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<T = AnyJson>(eventName: string, cb: (data: T) => Promise<void>): void {
public on<T = AnyJson>(eventName: string, cb: (data: T) => Promise<void>, uniqueListenerIdentifier?: string): void {
const listeners = this.getListeners(eventName);
if (listeners.length !== 0) {
if (!this.logger) {
Expand All @@ -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<string, callback>([[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;
}
}

/**
Expand Down
88 changes: 84 additions & 4 deletions test/unit/lifecycleEventsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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])
Expand All @@ -146,7 +196,7 @@ describe('lifecycleEvents', () => {
);
});

it('getListeners works', async () => {
it('getListeners works, including uniqueListeners', async () => {
const x = async (result: Record<string, unknown>) => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
fake.bar('test6', result);
Expand All @@ -159,6 +209,36 @@ describe('lifecycleEvents', () => {
Lifecycle.getInstance().removeAllListeners('test6');
});

it('getListeners works (unique Listeners)', async () => {
const x = async (result: Record<string, unknown>) => {
// 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<string, unknown>) => {
// 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();
Expand Down