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.

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

fix: non-reference clone of transferred listeners #1080

Merged
merged 2 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 14 additions & 5 deletions src/lifecycleEvents.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import { Logger } from './logger/logger';

// Data of any type can be passed to the callback. Can be cast to any type that is given in emit().
// eslint-disable-next-line @typescript-eslint/no-explicit-any
type callback = (data: any) => Promise<void>;
export type callback = (data: any) => Promise<void>;
type ListenerMap = Map<string, callback>;
export type UniqueListenerMap = Map<string, ListenerMap>;

declare const global: {
salesforceCoreLifecycle?: Lifecycle;
Expand Down Expand Up @@ -49,7 +51,7 @@ export class Lifecycle {

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

/**
Expand Down Expand Up @@ -89,8 +91,11 @@ 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 }, oldInstance.uniqueListeners);
// object spread and the clone fn keep them from being references
global.salesforceCoreLifecycle = new Lifecycle(
{ ...oldInstance.listeners },
cloneUniqueListeners(oldInstance.uniqueListeners)
);
// clean up any listeners on the old version
Object.keys(oldInstance.listeners).map((eventName) => {
oldInstance.removeAllListeners(eventName);
Expand Down Expand Up @@ -176,7 +181,7 @@ export class Lifecycle {
if (uniqueListenerIdentifier) {
if (!this.uniqueListeners.has(eventName)) {
// nobody is listening to the event yet
this.uniqueListeners.set(eventName, new Map<string, callback>([[uniqueListenerIdentifier, cb]]));
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);
Expand Down Expand Up @@ -231,3 +236,7 @@ export class Lifecycle {
}
}
}

const cloneListeners: (listeners: ListenerMap) => ListenerMap = (listeners) => new Map(Array.from(listeners.entries()));
export const cloneUniqueListeners = (uniqueListeners: UniqueListenerMap): UniqueListenerMap =>
new Map(Array.from(uniqueListeners.entries()).map(([key, value]) => [key, cloneListeners(value)]));
28 changes: 27 additions & 1 deletion test/unit/lifecycleEventsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { Duration, sleep } from '@salesforce/kit/lib/duration';
import { spyMethod } from '@salesforce/ts-sinon';
import * as chai from 'chai';
import { Lifecycle } from '../../src/lifecycleEvents';
import { Lifecycle, callback, cloneUniqueListeners } from '../../src/lifecycleEvents';
import { TestContext } from '../../src/testSetup';
import { Logger } from '../../src/logger/logger';

Expand Down Expand Up @@ -257,3 +257,29 @@ describe('lifecycleEvents', () => {
lifecycle2.removeAllListeners('test7');
});
});

describe('listener map cloning', () => {
const cb = (): Promise<void> => Promise.resolve();
it('clones map, breaking event name reference', () => {
const map1 = new Map<string, Map<string, callback>>();
map1.set('evt', new Map([['uniqueId', cb]]));

const map2 = cloneUniqueListeners(map1);
chai.expect(map2).to.deep.equal(map1);
map1.delete('evt');
chai.expect(map1.has('evt')).to.be.false;
chai.expect(map2.has('evt')).to.be.true;
});
it('clones map, breaking uniqueId reference', () => {
const map1 = new Map<string, Map<string, callback>>();
map1.set('evt', new Map([['uniqueId', cb]]));

const map2 = cloneUniqueListeners(map1);
chai.expect(map2).to.deep.equal(map1);
map2.get('evt')?.set('uniqueId2', cb);
chai.expect(map1.has('evt')).to.be.true;
chai.expect(map2.has('evt')).to.be.true;
chai.expect(map1.get('evt')?.has('uniqueId2')).to.be.false;
chai.expect(map2.get('evt')?.has('uniqueId2')).to.be.true;
});
});
Loading