GLSP-1050 Introduce Event API (#261)

* GLSP-1050 Introduce Event API - Introduce/reuse Event API from `vscode-jsonrpc` - Rework notifier services to use new API - Remove manual registration methods from notifier services - Adapt test cases for `SelectionService` and remove test cases that were testing generic listener functionality that is already provided (and therefore assumed tested) by `vscode-jsonrpc` Also: - Align prefix of injectable interfaces to consistently use the `I` prefix Part of eclipse-glsp/glsp#1050 * Address review feedback
eclipse-glsp · Jul 10, 2023 · f420894 · f420894
1 parent f12aad0
commit f420894
Show file tree

Hide file tree

Showing 18 changed files with 292 additions and 190 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -6,7 +6,7 @@
       "request": "launch",
       "name": "Run current test (client)",
       "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
-      "args": ["--config", "${workspaceRoot}/.mocharc", "${file}"],
+      "args": ["--config", "${workspaceRoot}/.mocharc", "--timeout", "0", "${file}"],
       "console": "integratedTerminal",
       "internalConsoleOptions": "neverOpen",
       "env": {
@@ -18,7 +18,7 @@
       "request": "launch",
       "name": "Run current test (protocol)",
       "program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
-      "args": ["--config", "${workspaceRoot}/.mocharc", "${file}"],
+      "args": ["--config", "${workspaceRoot}/.mocharc", "--timeout", "0", "${file}"],
       "console": "integratedTerminal",
       "internalConsoleOptions": "neverOpen",
       "env": {
@@ -33,6 +33,8 @@
       "args": [
         "--config",
         "${workspaceRoot}/.mocharc",
+        "--timeout",
+        "0",
         "${workspaceFolder}/packages/client/src/**/*.spec.ts"
       ],
       "env": {
@@ -49,6 +51,8 @@
       "args": [
         "--config",
         "${workspaceRoot}/.mocharc",
+        "--timeout",
+        "0",
         "${workspaceFolder}/packages/protocol/src/**/*.spec.ts"
       ],
       "env": {

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,7 +25,7 @@
 -   [protocol] Renamed `UndoOperation` and `RedoOperation` to `UndoAction` and `RedoAction` to match operation specification [#216](https://github.com/eclipse-glsp/glsp-client/pull/216)
 -   [protocol] Remove dependency to `vscode-ws-jsonrpc`. The protocol package now directly offers functions to create a websocket rpc connections [#215](https://github.com/eclipse-glsp/glsp-client/pull/215)
 -   [protocol] The `elementIds` property of `LayoutOperation` is now optional. If `undefined` the entire model will be layouted. [#232](https://github.com/eclipse-glsp/glsp-client/pull/232)
--   [api] Refactored base API [#259](https://github.com/eclipse-glsp/glsp-client/pull/#259)
+-   [API] Refactored base API [#259](https://github.com/eclipse-glsp/glsp-client/pull/#259)
     -   Removed the `TYPES.SelectionService` service identifier. Please directly use the `SelectionService` class as service identifier instead.
     -   The `SelectionService` binding is now part of the `defaultGLSPModule`. This means the `SelectionService` remains available even if the `selectModule` is not configured.
     -   `RootModelChangeListener`s are no longer tied to the `FeedbackawareUpdateModelCommand` instead they are managed by the `GLSPCommandStack`.
@@ -34,6 +34,10 @@
         -   `isRanked()` -> `Ranked.is()`
         -   `getRank()` -> `Ranked.getRank()`
         -   `DEFAULT_RANK` -> `Ranked.DEFAULT_RANK`
+-   [API] Introduced Event API to replace the old listener/notifier pattern [#261](https://github.com/eclipse-glsp/glsp-client/pull/#261)
+    -   Reworked `SelectionService`, `GlspCommandStack` & `EditorContextService` to make use of this new API
+    -   Removed explicit (de)registration methods for listeners. Use the corresponding event property (e.g. `SelectionService.onSelectionChanged`) instead.
+    -   Aligned naming of injectable interfaces & service identifiers to consistently use the `I` prefix.
 
 ## [v1.0.0 - 30/06/2022](https://github.com/eclipse-glsp/glsp-client/releases/tag/v1.0.0)
 

diff --git a/packages/client/src/base/command-stack.ts b/packages/client/src/base/command-stack.ts
@@ -13,22 +13,50 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { injectable, multiInject, optional } from 'inversify';
-import { CommandStack, ICommand, SModelRoot, SetModelCommand, TYPES, UpdateModelCommand, distinctAdd, remove } from '~glsp-sprotty';
+import { injectable, multiInject, optional, preDestroy } from 'inversify';
+import {
+    CommandStack,
+    Disposable,
+    DisposableCollection,
+    Emitter,
+    Event,
+    ICommand,
+    SModelRoot,
+    SetModelCommand,
+    TYPES,
+    UpdateModelCommand
+} from '~glsp-sprotty';
 
 /**
  * A hook to listen for model root changes. Will be called after a server update
  * has been processed
  */
-export interface SModelRootListener {
+export interface ISModelRootListener {
     modelRootChanged(root: Readonly<SModelRoot>): void;
 }
 
 @injectable()
-export class GLSPCommandStack extends CommandStack {
-    @multiInject(TYPES.SModelRootListener)
+export class GLSPCommandStack extends CommandStack implements Disposable {
+    @multiInject(TYPES.ISModelRootListener)
     @optional()
-    protected modelRootListeners: SModelRootListener[] = [];
+    protected modelRootListeners: ISModelRootListener[] = [];
+    protected toDispose = new DisposableCollection();
+
+    protected override initialize(): void {
+        super.initialize();
+        this.toDispose.push(this.onModelRootChangedEmitter);
+        this.modelRootListeners.forEach(listener => this.onModelRootChanged(root => listener.modelRootChanged(root)));
+    }
+
+    @preDestroy()
+    dispose(): void {
+        this.toDispose.dispose();
+    }
+
+    protected onModelRootChangedEmitter = new Emitter<Readonly<SModelRoot>>();
+    get onModelRootChanged(): Event<Readonly<SModelRoot>> {
+        return this.onModelRootChangedEmitter.event;
+    }
 
     override undo(): Promise<SModelRoot> {
         this.logger.warn(
@@ -55,14 +83,6 @@ export class GLSPCommandStack extends CommandStack {
     }
 
     protected notifyListeners(root: Readonly<SModelRoot>): void {
-        this.modelRootListeners.forEach(listener => listener.modelRootChanged(root));
-    }
-
-    register(listener: SModelRootListener): void {
-        distinctAdd(this.modelRootListeners, listener);
-    }
-
-    deregister(listener: SModelRootListener): void {
-        remove(this.modelRootListeners, listener);
+        this.onModelRootChangedEmitter.fire(root);
     }
 }
diff --git a/packages/client/src/base/di.config.ts b/packages/client/src/base/di.config.ts
@@ -70,7 +70,7 @@ const defaultGLSPModule = new ContainerModule((bind, _unbind, isBound, rebind) =
 
     bindAsService(context, TYPES.MouseListener, SelectionClearingMouseListener);
 
-    bindOrRebind(context, TYPES.ICommandStack).to(GLSPCommandStack);
+    bindOrRebind(context, TYPES.ICommandStack).to(GLSPCommandStack).inSingletonScope();
     bind(GLSPToolManager).toSelf().inSingletonScope();
     bindOrRebind(context, TYPES.IToolManager).toService(GLSPToolManager);
     bind(GLSPActionDispatcher).toSelf().inSingletonScope();
@@ -83,6 +83,7 @@ const defaultGLSPModule = new ContainerModule((bind, _unbind, isBound, rebind) =
     bindOrRebind(context, TYPES.ViewRegistry).to(GLSPViewRegistry).inSingletonScope();
 
     bind(SelectionService).toSelf().inSingletonScope();
+    bind(TYPES.ISModelRootListener).toService(SelectionService);
 });
 
 export default defaultGLSPModule;

diff --git a/packages/client/src/base/editor-context-service.ts b/packages/client/src/base/editor-context-service.ts
@@ -13,26 +13,29 @@
  *
  * SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
  ********************************************************************************/
-import { inject, injectable, multiInject, optional } from 'inversify';
+import { inject, injectable, multiInject, optional, postConstruct, preDestroy } from 'inversify';
 import {
     Action,
     Args,
+    Disposable,
+    DisposableCollection,
     EditMode,
     EditorContext,
+    Emitter,
+    Event,
     IActionHandler,
     ModelSource,
     MousePositionTracker,
     SModelElement,
     SModelRoot,
     SetEditModeAction,
     TYPES,
-    distinctAdd,
-    remove
+    ValueChange
 } from '~glsp-sprotty';
 import { SelectionService } from './selection-service';
 import { isSourceUriAware } from './source-uri-aware';
 
-export interface EditModeListener {
+export interface IEditModeListener {
     editModeChanged(newValue: string, oldValue: string): void;
 }
 
@@ -49,7 +52,7 @@ export interface EditModeListener {
  *    position, etc.).
  */
 @injectable()
-export class EditorContextService implements IActionHandler {
+export class EditorContextService implements IActionHandler, Disposable {
     @inject(SelectionService)
     protected selectionService: SelectionService;
 
@@ -58,19 +61,31 @@ export class EditorContextService implements IActionHandler {
 
     @multiInject(TYPES.IEditModeListener)
     @optional()
-    protected editModeListeners: EditModeListener[] = [];
+    protected editModeListeners: IEditModeListener[] = [];
 
     @inject(TYPES.ModelSourceProvider)
     protected modelSourceProvider: () => Promise<ModelSource>;
 
     protected _editMode: string;
 
-    register(editModeListener: EditModeListener): void {
-        distinctAdd(this.editModeListeners, editModeListener);
+    protected onEditModeChangedEmitter = new Emitter<ValueChange<string>>();
+    get onEditModeChanged(): Event<ValueChange<string>> {
+        return this.onEditModeChangedEmitter.event;
     }
 
-    deregister(editModeListener: EditModeListener): void {
-        remove(this.editModeListeners, editModeListener);
+    protected toDispose = new DisposableCollection();
+
+    @postConstruct()
+    protected initialize(): void {
+        this.toDispose.push(this.onEditModeChangedEmitter);
+        this.editModeListeners.forEach(listener =>
+            this.onEditModeChanged(change => listener.editModeChanged(change.newValue, change.oldValue))
+        );
+    }
+
+    @preDestroy()
+    dispose(): void {
+        this.toDispose.dispose();
     }
 
     get(args?: Args): EditorContext {
@@ -98,7 +113,7 @@ export class EditorContextService implements IActionHandler {
     }
 
     protected notifyEditModeListeners(oldValue: string): void {
-        this.editModeListeners.forEach(listener => listener.editModeChanged(oldValue, this.editMode));
+        this.onEditModeChangedEmitter.fire({ newValue: this.editMode, oldValue });
     }
 
     async getSourceUri(): Promise<string | undefined> {