Skip to content

Commit

Permalink
refactor(SignalManager): improve type safety and reduce boilerplate
Browse files Browse the repository at this point in the history 
- Replace individual fire* methods with strongly-typed emit interface
- Consolidate signal payload types into single Signals interface
- Add type-safe event handler registration with overloaded on/off methods
- Remove redundant method declarations in favor of TypeScript inference
- Maintain full backward compatibility with existing event system

The new implementation provides compile-time type checking for event
payloads while significantly reducing code volume and maintenance burden.

Signed-off-by: Will Yang <[email protected]>
  • Loading branch information
@williamsyang-work
williamsyang-work committed Oct 22, 2024
1 parent 6444d22 commit e381965
Show file tree
Hide file tree
Showing 20 changed files with 275 additions and 361 deletions.
10 changes: 4 additions & 6 deletions packages/base/src/experiment-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { OutputDescriptor } from 'tsp-typescript-client/lib/models/output-descri
import { Experiment } from 'tsp-typescript-client/lib/models/experiment';
import { TraceManager } from './trace-manager';
import { TspClientResponse } from 'tsp-typescript-client/lib/protocol/tsp-client-response';
import { signalManager, Signals } from './signals/signal-manager';
import { signalManager } from './signals/signal-manager';

export class ExperimentManager {
private fOpenExperiments: Map<string, Experiment> = new Map();
Expand All @@ -15,9 +15,7 @@ export class ExperimentManager {
constructor(tspClient: ITspClient, traceManager: TraceManager) {
this.fTspClient = tspClient;
this.fTraceManager = traceManager;
signalManager().on(Signals.EXPERIMENT_DELETED, (experiment: Experiment) =>
this.onExperimentDeleted(experiment)
);
signalManager().on('EXPERIMENT_DELETED', (experiment: Experiment) => this.onExperimentDeleted(experiment));
}

/**
Expand Down Expand Up @@ -99,7 +97,7 @@ export class ExperimentManager {
const experiment = experimentResponse.getModel();
if (experimentResponse.isOk() && experiment) {
this.addExperiment(experiment);
signalManager().fireExperimentOpenedSignal(experiment);
signalManager().emit('EXPERIMENT_OPENED', experiment);
return experiment;
}
// TODO Handle any other experiment open errors
Expand Down Expand Up @@ -131,7 +129,7 @@ export class ExperimentManager {
await this.fTspClient.deleteExperiment(experimentUUID);
const deletedExperiment = this.removeExperiment(experimentUUID);
if (deletedExperiment) {
signalManager().fireExperimentDeletedSignal(deletedExperiment);
signalManager().emit('EXPERIMENT_DELETED', deletedExperiment);
}
}
}
Expand Down
282 changes: 104 additions & 178 deletions packages/base/src/signals/signal-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,189 +10,115 @@ import { ContextMenuItemClickedSignalPayload } from './context-menu-item-clicked
import { RowSelectionsChangedSignalPayload } from './row-selections-changed-signal-payload';
import { ItemPropertiesSignalPayload } from './item-properties-signal-payload';

export declare interface SignalManager {
fireTraceOpenedSignal(trace: Trace): void;
fireTraceDeletedSignal(trace: Trace): void;
fireExperimentExperimentSignal(experiment: Experiment): void;
fireExperimentClosedSignal(experiment: Experiment): void;
fireExperimentDeletedSignal(experiment: Experiment): void;
fireExperimentSelectedSignal(experiment: Experiment | undefined): void;
fireExperimentUpdatedSignal(experiment: Experiment): void;
fireOpenedTracesChangedSignal(payload: OpenedTracesUpdatedSignalPayload): void;
fireOutputAddedSignal(payload: OutputAddedSignalPayload): void;
fireItemPropertiesSignalUpdated(payload: ItemPropertiesSignalPayload): void;
fireThemeChangedSignal(theme: string): void;
// TODO - Refactor or remove this signal. Similar signal to fireRequestSelectionRangeChange
fireSelectionChangedSignal(payload: { [key: string]: string }): void;
fireRowSelectionsChanged(payload: RowSelectionsChangedSignalPayload): void;
fireCloseTraceViewerTabSignal(traceUUID: string): void;
fireTraceViewerTabActivatedSignal(experiment: Experiment): void;
fireUpdateZoomSignal(hasZoomedIn: boolean): void;
fireResetZoomSignal(): void;
fireMarkerCategoriesFetchedSignal(): void;
fireMarkerSetsFetchedSignal(): void;
fireMarkerCategoryClosedSignal(payload: { traceViewerId: string; markerCategory: string }): void;
fireTraceServerStartedSignal(): void;
fireUndoSignal(): void;
fireRedoSignal(): void;
fireOutputDataChanged(outputs: OutputDescriptor[]): void;
fireOpenOverviewOutputSignal(traceId: string): void;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
firePinView(output: OutputDescriptor, payload?: any): void;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
fireUnPinView(output: OutputDescriptor, payload?: any): void;
fireOverviewOutputSelectedSignal(payload: { traceId: string; outputDescriptor: OutputDescriptor }): void;
fireSaveAsCsv(payload: { traceId: string; data: string }): void;
fireSelectionRangeUpdated(payload: TimeRangeUpdatePayload): void;
fireViewRangeUpdated(payload: TimeRangeUpdatePayload): void;
fireRequestSelectionRangeChange(payload: TimeRangeUpdatePayload): void;
fireContributeContextMenu(payload: ContextMenuContributedSignalPayload): void;
fireContextMenuItemClicked(payload: ContextMenuItemClickedSignalPayload): void;
export interface Signals {
TRACE_OPENED: Trace;
TRACE_DELETED: { trace: Trace };
EXPERIMENT_OPENED: Experiment;
EXPERIMENT_CLOSED: Experiment;
EXPERIMENT_DELETED: Experiment;
EXPERIMENT_SELECTED: Experiment | undefined;
EXPERIMENT_UPDATED: Experiment;
OPENED_TRACES_UPDATED: OpenedTracesUpdatedSignalPayload;
AVAILABLE_OUTPUTS_CHANGED: void;
OUTPUT_ADDED: OutputAddedSignalPayload;
ITEM_PROPERTIES_UPDATED: ItemPropertiesSignalPayload;
THEME_CHANGED: string;
SELECTION_CHANGED: { [key: string]: string };
ROW_SELECTIONS_CHANGED: RowSelectionsChangedSignalPayload;
CLOSE_TRACEVIEWERTAB: string;
TRACEVIEWERTAB_ACTIVATED: Experiment;
UPDATE_ZOOM: boolean;
RESET_ZOOM: void;
MARKER_CATEGORIES_FETCHED: void;
MARKERSETS_FETCHED: void;
MARKER_CATEGORY_CLOSED: [traceViewerId: string, markerCategory: string];
TRACE_SERVER_STARTED: void;
UNDO: void;
REDO: void;
PIN_VIEW: [output: OutputDescriptor, extra?: any];
UNPIN_VIEW: [output: OutputDescriptor, extra?: any];
OPEN_OVERVIEW_OUTPUT: string;
OVERVIEW_OUTPUT_SELECTED: [traceId: string, outputDescriptor: OutputDescriptor];
SAVE_AS_CSV: [traceId: string, data: string];
VIEW_RANGE_UPDATED: TimeRangeUpdatePayload;
SELECTION_RANGE_UPDATED: TimeRangeUpdatePayload;
REQUEST_SELECTION_RANGE_CHANGE: TimeRangeUpdatePayload;
OUTPUT_DATA_CHANGED: [descriptors: OutputDescriptor[], trigger: boolean];
CONTRIBUTE_CONTEXT_MENU: ContextMenuContributedSignalPayload;
CONTEXT_MENU_ITEM_CLICKED: ContextMenuItemClickedSignalPayload;
}

export const Signals = {
TRACE_OPENED: 'trace opened',
TRACE_DELETED: 'trace deleted',
EXPERIMENT_OPENED: 'experiment opened',
EXPERIMENT_CLOSED: 'experiment closed',
EXPERIMENT_DELETED: 'experiment deleted',
EXPERIMENT_SELECTED: 'experiment selected',
EXPERIMENT_UPDATED: 'experiment updated',
OPENED_TRACES_UPDATED: 'opened traces updated',
AVAILABLE_OUTPUTS_CHANGED: 'available outputs changed',
OUTPUT_ADDED: 'output added',
ITEM_PROPERTIES_UPDATED: 'item properties updated',
THEME_CHANGED: 'theme changed',
SELECTION_CHANGED: 'selection changed',
ROW_SELECTIONS_CHANGED: 'rows selected changed',
CLOSE_TRACEVIEWERTAB: 'tab closed',
TRACEVIEWERTAB_ACTIVATED: 'widget activated',
UPDATE_ZOOM: 'update zoom',
RESET_ZOOM: 'reset zoom',
UNDO: 'undo',
REDO: 'redo',
MARKER_CATEGORIES_FETCHED: 'marker categories fetched',
MARKERSETS_FETCHED: 'markersets fetched',
MARKER_CATEGORY_CLOSED: 'marker category closed',
TRACE_SERVER_STARTED: 'trace server started',
PIN_VIEW: 'view pinned',
UNPIN_VIEW: 'view unpinned',
OPEN_OVERVIEW_OUTPUT: 'open overview output',
OVERVIEW_OUTPUT_SELECTED: 'overview output selected',
SAVE_AS_CSV: 'save as csv',
VIEW_RANGE_UPDATED: 'view range updated',
SELECTION_RANGE_UPDATED: 'selection range updated',
REQUEST_SELECTION_RANGE_CHANGE: 'change selection range',
OUTPUT_DATA_CHANGED: 'output data changed',
CONTRIBUTE_CONTEXT_MENU: 'contribute context menu',
CONTEXT_MENU_ITEM_CLICKED: 'context menu item clicked'
};
type SignalType = keyof Signals;
type SignalArgs<T> = T extends [void]
? []
: T extends [any, ...any[]]
? T // Tuple types for multi-arg events
: [T]; // Everything else (including arrays) as single argument

export class SignalManager extends EventEmitter implements SignalManager {
fireTraceOpenedSignal(trace: Trace): void {
this.emit(Signals.TRACE_OPENED, trace);
}
fireTraceDeletedSignal(trace: Trace): void {
this.emit(Signals.TRACE_DELETED, { trace });
}
fireExperimentOpenedSignal(experiment: Experiment): void {
this.emit(Signals.EXPERIMENT_OPENED, experiment);
}
fireExperimentClosedSignal(experiment: Experiment): void {
this.emit(Signals.EXPERIMENT_CLOSED, experiment);
}
fireExperimentDeletedSignal(experiment: Experiment): void {
this.emit(Signals.EXPERIMENT_DELETED, experiment);
}
fireExperimentSelectedSignal(experiment: Experiment | undefined): void {
this.emit(Signals.EXPERIMENT_SELECTED, experiment);
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
fireRowSelectionsChanged(payload: RowSelectionsChangedSignalPayload): void {
this.emit(Signals.ROW_SELECTIONS_CHANGED, payload);
}
fireExperimentUpdatedSignal(experiment: Experiment): void {
this.emit(Signals.EXPERIMENT_UPDATED, experiment);
}
fireOpenedTracesChangedSignal(payload: OpenedTracesUpdatedSignalPayload): void {
this.emit(Signals.OPENED_TRACES_UPDATED, payload);
}
fireOutputAddedSignal(payload: OutputAddedSignalPayload): void {
this.emit(Signals.OUTPUT_ADDED, payload);
}
fireItemPropertiesSignalUpdated(payload: ItemPropertiesSignalPayload): void {
this.emit(Signals.ITEM_PROPERTIES_UPDATED, payload);
}
fireThemeChangedSignal(theme: string): void {
this.emit(Signals.THEME_CHANGED, theme);
}
fireSelectionChangedSignal(payload: { [key: string]: string }): void {
this.emit(Signals.SELECTION_CHANGED, payload);
}
fireCloseTraceViewerTabSignal(traceUUID: string): void {
this.emit(Signals.CLOSE_TRACEVIEWERTAB, traceUUID);
}
fireTraceViewerTabActivatedSignal(experiment: Experiment): void {
this.emit(Signals.TRACEVIEWERTAB_ACTIVATED, experiment);
}
fireUpdateZoomSignal(hasZoomedIn: boolean): void {
this.emit(Signals.UPDATE_ZOOM, hasZoomedIn);
}
fireResetZoomSignal(): void {
this.emit(Signals.RESET_ZOOM);
export class SignalManager extends EventEmitter {
/**
* Registers an event handler for a specific signal type.
* Provides type-safe event registration with correct payload types for each signal.
*
* @template K - The signal type (key of Signals interface)
* @param event - The event name to listen for
* @param listener - The callback function to execute when the event occurs
* Type of arguments is automatically inferred from Signals interface
* @returns The signal manager instance for chaining
*
* @example
* // Single argument event
* signalManager().on('THEME_CHANGED', (theme: string) => {
* console.log(`Theme changed to: ${theme}`);
* });
*
* // Tuple argument event
* signalManager().on('PIN_VIEW', (output: OutputDescriptor, extra?: any) => {
* console.log(`Pinning view for output: ${output.id}`);
* });
*/
on<K extends SignalType>(event: K, listener: (...args: SignalArgs<Signals[K]>) => void): this {
return super.on(event, listener as any);
}
fireMarkerCategoriesFetchedSignal(): void {
this.emit(Signals.MARKER_CATEGORIES_FETCHED);
}
fireMarkerSetsFetchedSignal(): void {
this.emit(Signals.MARKERSETS_FETCHED);
}
fireMarkerCategoryClosedSignal(payload: { traceViewerId: string; markerCategory: string }): void {
this.emit(Signals.MARKER_CATEGORY_CLOSED, payload);
}
fireTraceServerStartedSignal(): void {
this.emit(Signals.TRACE_SERVER_STARTED);
}
fireUndoSignal(): void {
this.emit(Signals.UNDO);
}
fireRedoSignal(): void {
this.emit(Signals.REDO);
}
fireOutputDataChanged(outputs: OutputDescriptor[]): void {
this.emit(Signals.OUTPUT_DATA_CHANGED, outputs);
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
firePinView(output: OutputDescriptor, payload?: any): void {
this.emit(Signals.PIN_VIEW, output, payload);
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
fireUnPinView(output: OutputDescriptor, payload?: any): void {
this.emit(Signals.UNPIN_VIEW, output, payload);
}
fireOpenOverviewOutputSignal(traceId: string): void {
this.emit(Signals.OPEN_OVERVIEW_OUTPUT, traceId);
}
fireOverviewOutputSelectedSignal(payload: { traceId: string; outputDescriptor: OutputDescriptor }): void {
this.emit(Signals.OVERVIEW_OUTPUT_SELECTED, payload);
}
fireSaveAsCsv(payload: { traceId: string; data: string }): void {
this.emit(Signals.SAVE_AS_CSV, payload);
}
fireViewRangeUpdated(payload: TimeRangeUpdatePayload): void {
this.emit(Signals.VIEW_RANGE_UPDATED, payload);
}
fireSelectionRangeUpdated(payload: TimeRangeUpdatePayload): void {
this.emit(Signals.SELECTION_RANGE_UPDATED, payload);
}
fireRequestSelectionRangeChange(payload: TimeRangeUpdatePayload): void {
this.emit(Signals.REQUEST_SELECTION_RANGE_CHANGE, payload);
}
fireContributeContextMenu(payload: ContextMenuContributedSignalPayload): void {
this.emit(Signals.CONTRIBUTE_CONTEXT_MENU, payload);

/**
* Removes an event handler for a specific signal type.
* Ensures type safety by requiring the listener signature to match the signal type.
*
* @template K - The signal type (key of Signals interface)
* @param event - The event name to remove the listener from
* @param listener - The callback function to remove
* @returns The signal manager instance for chaining
*
* @example
* const themeHandler = (theme: string) => console.log(theme);
* signalManager().off('THEME_CHANGED', themeHandler);
*/
off<K extends SignalType>(event: K, listener: (...args: SignalArgs<Signals[K]>) => void): this {
return super.off(event, listener as any);
}
fireContextMenuItemClicked(payload: ContextMenuItemClickedSignalPayload): void {
this.emit(Signals.CONTEXT_MENU_ITEM_CLICKED, payload);

/**
* Emits a signal with type-safe arguments based on the signal type.
* Arguments are automatically validated against the Signals interface.
*
* @template K - The signal type (key of Signals interface)
* @param event - The event name to emit
* @param args - The arguments to pass to listeners, type checked against Signals interface
* @returns true if the event had listeners, false otherwise
*
* @example
* // Single argument emission
* signalManager().emit('THEME_CHANGED', 'dark');
*
* // Tuple argument emission
* signalManager().emit('MARKER_CATEGORY_CLOSED', 'viewer1', 'category1');
*
* // Void event emission
* signalManager().emit('RESET_ZOOM');
*/
emit<K extends SignalType>(event: K, ...args: SignalArgs<Signals[K]>): boolean {
return super.emit(event, ...args);
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/base/src/trace-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ export class TraceManager {
const trace = traceResponse.getModel();
if (traceResponse.isOk() && trace) {
this.addTrace(trace);
signalManager().fireTraceOpenedSignal(trace);
signalManager().emit('TRACE_OPENED', trace);
return trace;
}
// TODO Handle trace open errors
Expand Down Expand Up @@ -123,7 +123,7 @@ export class TraceManager {
if (deleteResponse.getStatusCode() !== 409) {
const deletedTrace = this.removeTrace(traceUUID);
if (deletedTrace) {
signalManager().fireTraceDeletedSignal(deletedTrace);
signalManager().emit('TRACE_DELETED', { trace: deletedTrace });
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions packages/react-components/src/components/abstract-output-component.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ export abstract class AbstractOutputComponent<

private closeComponent() {
if (this.props.pinned) {
signalManager().fireUnPinView(this.props.outputDescriptor);
signalManager().emit('UNPIN_VIEW', this.props.outputDescriptor);
}
this.props.onOutputRemove(this.props.outputDescriptor.id);
}
Expand Down Expand Up @@ -271,15 +271,15 @@ export abstract class AbstractOutputComponent<

// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
protected pinView(payload?: any): void {
signalManager().firePinView(this.props.outputDescriptor, payload);
signalManager().emit('PIN_VIEW', this.props.outputDescriptor, payload);
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
protected unPinView(payload?: any): void {
if (payload) {
signalManager().fireUnPinView(this.props.outputDescriptor, payload);
signalManager().emit('UNPIN_VIEW', this.props.outputDescriptor, payload);
} else {
signalManager().fireUnPinView(this.props.outputDescriptor);
signalManager().emit('UNPIN_VIEW', this.props.outputDescriptor);
}
}

Expand Down
5 changes: 1 addition & 4 deletions packages/react-components/src/components/datatree-output-component.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,7 @@ export class DataTreeOutputComponent extends AbstractOutputComponent<AbstractOut
csvArray.push(row.join(','));
}
const tableString = csvArray.join('\n');
signalManager().fireSaveAsCsv({
traceId: this.props.traceId,
data: tableString
});
signalManager().emit('SAVE_AS_CSV', this.props.traceId, tableString);
}
}
}
Expand Down
Loading

0 comments on commit e381965

Please sign in to comment.