Skip to content

Commit 4bb6ce0

Browse files
Refactor CallbackRegistry to improve item tracking and usage
- Introduce `ItemInfo` type to track item registration, usage, and promise details - Enhance timeout handling with more informative warnings for unused items - Improve `get`, `has`, and `getAll` methods to better track item usage - Optimize `getOrWaitForItem` to handle existing promises and registration states
1 parent bdc72ed commit 4bb6ce0

File tree

1 file changed

+76
-30
lines changed

1 file changed

+76
-30
lines changed

node_package/src/CallbackRegistry.ts

Lines changed: 76 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,33 +2,47 @@ import { ItemRegistrationCallback } from "./types";
22
import { onPageLoaded, onPageUnloaded } from "./pageLifecycle";
33
import { getContextAndRailsContext } from "./context";
44

5-
export default class CallbackRegistry<T> {
6-
private readonly registryType: string;
7-
private registeredItems = new Map<string, T>();
8-
private callbacks = new Map<string, Array<{
5+
/**
6+
* Represents information about a registered item including its value,
7+
* promise details for pending registrations, and usage status
8+
*/
9+
type ItemInfo<T> = {
10+
item: T | null;
11+
waitingPromiseInfo?: {
912
resolve: ItemRegistrationCallback<T>;
1013
reject: (error: Error) => void;
11-
}>>();
14+
promise: Promise<T>;
15+
},
16+
isUsed: boolean;
17+
}
18+
19+
export default class CallbackRegistry<T> {
20+
private readonly registryType: string;
21+
private registeredItems = new Map<string, ItemInfo<T>>();
1222

13-
private timoutEventsInitialized = false;
23+
private timeoutEventsInitialized = false;
1424
private timedout = false;
1525

1626
constructor(registryType: string) {
1727
this.registryType = registryType;
1828
}
1929

2030
private initializeTimeoutEvents() {
21-
if (!this.timoutEventsInitialized) {
22-
this.timoutEventsInitialized = true;
31+
if (!this.timeoutEventsInitialized) {
32+
this.timeoutEventsInitialized = true;
2333
}
2434

2535
let timeoutId: NodeJS.Timeout;
2636
const triggerTimeout = () => {
2737
this.timedout = true;
28-
this.callbacks.forEach((itemCallbacks, itemName) => {
29-
itemCallbacks.forEach((callback) => {
30-
callback.reject(this.createNotFoundError(itemName));
31-
});
38+
this.registeredItems.forEach((itemInfo, itemName) => {
39+
if (itemInfo.waitingPromiseInfo) {
40+
itemInfo.waitingPromiseInfo.reject(this.createNotFoundError(itemName));
41+
// eslint-disable-next-line no-param-reassign
42+
itemInfo.waitingPromiseInfo = undefined;
43+
} else if (!itemInfo.isUsed) {
44+
console.warn(`Warning: ${this.registryType} '${itemName}' was registered but never used. This may indicate unused code that can be removed.`);
45+
}
3246
});
3347
};
3448

@@ -40,54 +54,86 @@ export default class CallbackRegistry<T> {
4054
});
4155

4256
onPageUnloaded(() => {
43-
this.callbacks.clear();
57+
this.registeredItems.forEach((itemInfo) => {
58+
// eslint-disable-next-line no-param-reassign
59+
itemInfo.waitingPromiseInfo = undefined;
60+
});
4461
this.timedout = false;
4562
clearTimeout(timeoutId);
4663
});
4764
}
4865

4966
set(name: string, item: T): void {
50-
this.registeredItems.set(name, item);
51-
52-
const callbacks = this.callbacks.get(name) || [];
53-
callbacks.forEach(callback => callback.resolve(item));
54-
this.callbacks.delete(name);
67+
const { waitingPromiseInfo } = this.registeredItems.get(name) ?? {};
68+
this.registeredItems.set(name, { item, isUsed: !!waitingPromiseInfo });
69+
70+
if (waitingPromiseInfo) {
71+
waitingPromiseInfo.resolve(item);
72+
}
5573
}
5674

5775
get(name: string): T {
58-
const item = this.registeredItems.get(name);
59-
if (item !== undefined) return item;
60-
61-
throw this.createNotFoundError(name);
76+
const itemInfo = this.registeredItems.get(name);
77+
if (!itemInfo?.item) {
78+
throw this.createNotFoundError(name);
79+
}
80+
itemInfo.isUsed = true;
81+
return itemInfo.item;
6282
}
6383

6484
has(name: string): boolean {
65-
return this.registeredItems.has(name);
85+
return !!this.registeredItems.get(name)?.item;
6686
}
6787

6888
clear(): void {
6989
this.registeredItems.clear();
7090
}
7191

7292
getAll(): Map<string, T> {
73-
return this.registeredItems;
93+
const components = new Map<string, T>();
94+
this.registeredItems.forEach((itemInfo, name) => {
95+
if (!itemInfo.item) return;
96+
components.set(name, itemInfo.item);
97+
});
98+
return components;
7499
}
75100

76101
getOrWaitForItem(name: string): Promise<T> {
77102
this.initializeTimeoutEvents();
78-
return new Promise((resolve, reject) => {
103+
const existingInfo = this.registeredItems.get(name);
104+
105+
// Return existing promise if there's already a waiting promise
106+
if (existingInfo?.waitingPromiseInfo) {
107+
return existingInfo.waitingPromiseInfo.promise;
108+
}
109+
110+
let waitingPromiseInfo: ItemInfo<T>['waitingPromiseInfo'];
111+
const getItemPromise = new Promise<T>((resolve, reject) => {
79112
try {
80-
resolve(this.get(name));
113+
const item = this.get(name);
114+
resolve(item);
81115
} catch(error) {
82116
if (this.timedout) {
83-
throw error;
117+
reject(error);
118+
return;
84119
}
85120

86-
const itemCallbacks = this.callbacks.get(name) || [];
87-
itemCallbacks.push({ resolve, reject });
88-
this.callbacks.set(name, itemCallbacks);
121+
this.registeredItems.set(name, {
122+
item: null,
123+
waitingPromiseInfo: waitingPromiseInfo = {
124+
resolve,
125+
reject,
126+
promise: getItemPromise,
127+
},
128+
isUsed: true,
129+
});
89130
}
90131
});
132+
if (waitingPromiseInfo) {
133+
waitingPromiseInfo.promise = getItemPromise;
134+
}
135+
136+
return getItemPromise;
91137
}
92138

93139
private createNotFoundError(itemName: string): Error {

0 commit comments

Comments
 (0)