Improve plugin-view toolbar lifecycle

CHANGELOG.md

@@ -5,7 +5,9 @@
 
 <a name="breaking_changes_1.22.0">[Breaking Changes:](#breaking_changes_1.22.0)</a>
 - [core] Removed `MarkdownRenderer` class [#10589](https://github.com/eclipse-theia/theia/pull/10589)
-- [plugin] Removed deprecated fields `id` and `label` from `theia.Command`
+- [core] `ContextKeyService` is now an interface. Extenders should extend `ContextKeyServiceDummyImpl` [#10546](https://github.com/eclipse-theia/theia/pull/10546)
+- [plugin] Removed deprecated fields `id` and `label` from `theia.Command` [#10512](https://github.com/eclipse-theia/theia/pull/10512)
+- [plugin-ext] `ViewContextKeyService#with` method removed. Use `ContextKeyService#with` instead. `PluginViewWidget` and `PluginTreeWidget` inject the `ContextKeyService` rather than `ViewContextKeyService`. [#10546](https://github.com/eclipse-theia/theia/pull/10546)
 
 
 ## v1.21.0 - 12/16/2021

packages/core/src/browser/context-key-service.ts

@@ -15,7 +15,8 @@
  ********************************************************************************/
 
 import { injectable } from 'inversify';
-import { Emitter } from '../common/event';
+import { Disposable } from '../common';
+import { Emitter, Event } from '../common/event';
 
 export interface ContextKey<T> {
     set(value: T | undefined): void;
@@ -36,8 +37,44 @@ export interface ContextKeyChangeEvent {
     affects(keys: Set<string>): boolean;
 }
 
+export const ContextKeyService = Symbol('ContextKeyService');
+export interface ContextKeyService extends Disposable {
+    readonly onDidChange: Event<ContextKeyChangeEvent>;
+
+    createKey<T>(key: string, defaultValue: T | undefined): ContextKey<T>;
+
+    /**
+     * Whether the expression is satisfied. If `context` provided, the service will attempt to retrieve a context object associated with that element.
+     */
+    match(expression: string, context?: HTMLElement): boolean;
+
+    /**
+     * @returns a Set of the keys used by the given `expression` or `undefined` if none are used or the expression cannot be parsed.
+     */
+    parseKeys(expression: string): Set<string> | undefined;
+
+    /**
+     * Creates a temporary context that will use the `values` passed in when evaluating `callback`
+     * `callback` must be synchronous.
+     */
+    with<T>(values: Record<string, unknown>, callback: () => T): T;
+
+    /**
+     * Creates a child service with a separate context scoped to the HTML element passed in.
+     * Useful for e.g. setting the {view} context value for particular widgets.
+     */
+    createScoped(target?: HTMLElement): ScopedValueStore;
+
+    /**
+     * Set or modify a value in the service's context.
+     */
+    setContext(key: string, value: unknown): void;
+}
+
+export type ScopedValueStore = Omit<ContextKeyService, 'onDidChange' | 'match' | 'parseKeys' | 'with'>;
+
 @injectable()
-export class ContextKeyService {
+export class ContextKeyServiceDummyImpl implements ContextKeyService {
 
     protected readonly onDidChangeEmitter = new Emitter<ContextKeyChangeEvent>();
     readonly onDidChange = this.onDidChangeEmitter.event;
@@ -62,4 +99,25 @@ export class ContextKeyService {
         return new Set<string>();
     }
 
+    /**
+     * Details should be implemented by an extension, e.g. by the monaco extension.
+     * Callback must be synchronous.
+     */
+    with<T>(values: Record<string, unknown>, callback: () => T): T {
+        return callback();
+    }
+
+    /**
+     * Details should implemented by an extension, e.g. by the monaco extension.
+     */
+    createScoped(target?: HTMLElement): ContextKeyService {
+        return this;
+    }
+
+    /**
+     * Details should be implemented by an extension, e.g. by the monaco extension.
+     */
+    setContext(key: string, value: unknown): void { }
+
+    dispose(): void { }
 }

packages/core/src/browser/frontend-application-module.ts

@@ -62,7 +62,7 @@ import { FrontendApplicationStateService } from './frontend-application-state';
 import { JsonSchemaStore, JsonSchemaContribution, DefaultJsonSchemaContribution } from './json-schema-store';
 import { TabBarToolbarRegistry, TabBarToolbarContribution, TabBarToolbarFactory, TabBarToolbar } from './shell/tab-bar-toolbar';
 import { bindCorePreferences } from './core-preferences';
-import { ContextKeyService } from './context-key-service';
+import { ContextKeyService, ContextKeyServiceDummyImpl } from './context-key-service';
 import { ResourceContextKey } from './resource-context-key';
 import { KeyboardLayoutService } from './keyboard/keyboard-layout-service';
 import { MimeService } from './mime-service';
@@ -221,7 +221,7 @@ export const frontendApplicationModule = new ContainerModule((bind, unbind, isBo
     bind(CommandService).toService(CommandRegistry);
     bindContributionProvider(bind, CommandContribution);
 
-    bind(ContextKeyService).toSelf().inSingletonScope();
+    bind(ContextKeyService).to(ContextKeyServiceDummyImpl).inSingletonScope();
 
     bind(MenuModelRegistry).toSelf().inSingletonScope();
     bindContributionProvider(bind, MenuContribution);

packages/core/src/browser/keybinding.spec.ts

@@ -30,7 +30,7 @@ import { LabelParser } from './label-parser';
 import { MockLogger } from '../common/test/mock-logger';
 import { StatusBar, StatusBarImpl } from './status-bar/status-bar';
 import { FrontendApplicationStateService } from './frontend-application-state';
-import { ContextKeyService } from './context-key-service';
+import { ContextKeyService, ContextKeyServiceDummyImpl } from './context-key-service';
 import { CorePreferences } from './core-preferences';
 import * as os from '../common/os';
 import * as chai from 'chai';
@@ -87,7 +87,7 @@ before(async () => {
         bind(StatusBar).toService(StatusBarImpl);
         bind(CommandService).toService(CommandRegistry);
         bind(LabelParser).toSelf().inSingletonScope();
-        bind(ContextKeyService).toSelf().inSingletonScope();
+        bind(ContextKeyService).to(ContextKeyServiceDummyImpl).inSingletonScope();
         bind(FrontendApplicationStateService).toSelf().inSingletonScope();
         bind(CorePreferences).toConstantValue(<CorePreferences>{});
         bindPreferenceService(bind);

packages/debug/src/browser/console/debug-console-contribution.tsx

@@ -160,7 +160,7 @@ export class DebugConsoleContribution extends AbstractViewContribution<ConsoleWi
 
     static bindContribution(bind: interfaces.Bind): void {
         bind(InDebugReplContextKey).toDynamicValue(({ container }) =>
-            container.get(ContextKeyService).createKey('inDebugRepl', false)
+            container.get<ContextKeyService>(ContextKeyService).createKey('inDebugRepl', false)
         ).inSingletonScope();
         bind(DebugConsoleSession).toSelf().inRequestScope();
         bind(DebugConsoleSessionFactory).toFactory(context => (session: DebugSession) => {

packages/debug/src/browser/debug-frontend-module.ts

@@ -66,7 +66,7 @@ export default new ContainerModule((bind: interfaces.Bind) => {
     bindContributionProvider(bind, DebugContribution);
 
     bind(DebugCallStackItemTypeKey).toDynamicValue(({ container }) =>
-        container.get(ContextKeyService).createKey('callStackItemType', undefined)
+        container.get<ContextKeyService>(ContextKeyService).createKey('callStackItemType', undefined)
     ).inSingletonScope();
 
     bindContributionProvider(bind, DebugSessionContribution);

packages/git/src/browser/git-repository-provider.spec.ts

@@ -36,7 +36,7 @@ import * as chai from 'chai';
 import { GitCommitMessageValidator } from './git-commit-message-validator';
 import { ScmService } from '@theia/scm/lib/browser/scm-service';
 import { ScmContextKeyService } from '@theia/scm/lib/browser/scm-context-key-service';
-import { ContextKeyService } from '@theia/core/lib/browser/context-key-service';
+import { ContextKeyService, ContextKeyServiceDummyImpl } from '@theia/core/lib/browser/context-key-service';
 import { GitScmProvider } from './git-scm-provider';
 import { createGitScmProviderFactory } from './git-frontend-module';
 import { EditorManager } from '@theia/editor/lib/browser';
@@ -98,7 +98,7 @@ describe('GitRepositoryProvider', () => {
         testContainer.bind(ScmService).toSelf().inSingletonScope();
         testContainer.bind(GitScmProvider.Factory).toFactory(createGitScmProviderFactory);
         testContainer.bind(ScmContextKeyService).toSelf().inSingletonScope();
-        testContainer.bind(ContextKeyService).toSelf().inSingletonScope();
+        testContainer.bind(ContextKeyService).to(ContextKeyServiceDummyImpl).inSingletonScope();
         testContainer.bind(GitCommitMessageValidator).toSelf().inSingletonScope();
         testContainer.bind(EditorManager).toConstantValue(<EditorManager>{});
         testContainer.bind(GitErrorHandler).toConstantValue(<GitErrorHandler>{});

packages/monaco/src/browser/monaco-context-key-service.ts

@@ -15,18 +15,21 @@
  ********************************************************************************/
 
 import { injectable, inject, postConstruct } from '@theia/core/shared/inversify';
-import { ContextKeyService, ContextKey } from '@theia/core/lib/browser/context-key-service';
+import { ContextKeyService, ContextKey, ContextKeyChangeEvent, ScopedValueStore } from '@theia/core/lib/browser/context-key-service';
+import { Emitter } from '@theia/core';
 
 @injectable()
-export class MonacoContextKeyService extends ContextKeyService {
+export class MonacoContextKeyService implements ContextKeyService {
+    protected readonly onDidChangeEmitter = new Emitter<ContextKeyChangeEvent>();
+    readonly onDidChange = this.onDidChangeEmitter.event;
 
     @inject(monaco.contextKeyService.ContextKeyService)
     protected readonly contextKeyService: monaco.contextKeyService.ContextKeyService;
 
     @postConstruct()
     protected init(): void {
         this.contextKeyService.onDidChangeContext(e =>
-            this.fireDidChange({
+            this.onDidChangeEmitter.fire({
                 affects: keys => e.affectsSome(keys)
             })
         );
@@ -36,16 +39,28 @@ export class MonacoContextKeyService extends ContextKeyService {
         return this.contextKeyService.createKey(key, defaultValue);
     }
 
-    activeContext?: HTMLElement;
+    activeContext?: HTMLElement | monaco.contextKeyService.IContext;
 
     match(expression: string, context?: HTMLElement): boolean {
-        const ctx = context || this.activeContext || (window.document.activeElement instanceof HTMLElement ? window.document.activeElement : undefined);
         const parsed = this.parse(expression);
+        const ctx = this.identifyContext(context);
         if (!ctx) {
             return this.contextKeyService.contextMatchesRules(parsed);
         }
-        const keyContext = this.contextKeyService.getContext(ctx);
-        return monaco.keybindings.KeybindingResolver.contextMatchesRules(keyContext, parsed);
+        return monaco.keybindings.KeybindingResolver.contextMatchesRules(ctx, parsed);
+    }
+
+    protected identifyContext(callersContext?: HTMLElement | monaco.contextKeyService.IContext): monaco.contextKeyService.IContext | undefined {
+        if (callersContext && 'getValue' in callersContext) {
+            return callersContext;
+        } else if (this.activeContext && 'getValue' in this.activeContext) {
+            return this.activeContext;
+        }
+        const browserContext = callersContext ?? this.activeContext ?? (document.activeElement instanceof HTMLElement ? document.activeElement : undefined);
+        if (browserContext) {
+            return this.contextKeyService.getContext(browserContext);
+        }
+        return undefined;
     }
 
     protected readonly expressions = new Map<string, monaco.contextkey.ContextKeyExpression>();
@@ -65,4 +80,33 @@ export class MonacoContextKeyService extends ContextKeyService {
         return expr ? new Set<string>(expr.keys()) : expr;
     }
 
+    with<T>(values: Record<string, unknown>, callback: () => T): T {
+        const oldActive = this.activeContext;
+        const id = this.contextKeyService.createChildContext();
+        const child = this.contextKeyService.getContextValuesContainer(id);
+        for (const [key, value] of Object.entries(values)) {
+            child.setValue(key, value);
+        }
+        this.activeContext = child;
+        try {
+            return callback();
+        } finally {
+            this.activeContext = oldActive;
+            this.contextKeyService.disposeContext(id);
+        }
+    }
+
+    createScoped(target?: HTMLElement): ScopedValueStore {
+        return this.contextKeyService.createScoped(target);
+    }
+
+    setContext(key: string, value: unknown): void {
+        this.contextKeyService.setContext(key, value);
+    }
+
+    dispose(): void {
+        this.activeContext = undefined;
+        this.onDidChangeEmitter.dispose();
+        this.contextKeyService.dispose();
+    }
 }

packages/monaco/src/browser/monaco-frontend-module.ts

@@ -169,8 +169,10 @@ export function createMonacoConfigurationService(container: interfaces.Container
     const _configuration = service._configuration;
 
     _configuration.getValue = (section, overrides) => {
-        const overrideIdentifier = overrides && 'overrideIdentifier' in overrides && overrides['overrideIdentifier'] as string || undefined;
-        const resourceUri = overrides && 'resource' in overrides && !!overrides['resource'] && overrides['resource'].toString();
+        const overrideIdentifier: string | undefined = (overrides && 'overrideIdentifier' in overrides && typeof overrides.overrideIdentifier === 'string')
+            ? overrides['overrideIdentifier']
+            : undefined;
+        const resourceUri: string | undefined = (overrides && 'resource' in overrides && !!overrides['resource']) ? overrides['resource'].toString() : undefined;
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
         const proxy = createPreferenceProxy<{ [key: string]: any }>(preferences, preferenceSchemaProvider.getCombinedSchema(), {
             resourceUri, overrideIdentifier, style: 'both'

packages/monaco/src/typings/monaco/index.d.ts

@@ -2147,7 +2147,7 @@ declare module monaco.contextKeyService {
 
         createScoped(target?: HTMLElement): IContextKeyService;
         createOverlay(overlay: Iterable<[string, any]>): IContextKeyService;
-        getContext(target?: HTMLElement): IContext;
+        getContext(target: HTMLElement | null): IContext;
 
         updateParent(parentContextKeyService: IContextKeyService): void;
     }
@@ -2157,26 +2157,51 @@ declare module monaco.contextKeyService {
         getValue<T>(key: string): T | undefined;
     }
 
+    // https://github.com/theia-ide/vscode/blob/e930e4240ee604757efbd7fd621b77b75568f95d/src/vs/platform/contextkey/browser/contextKeyService.ts#L19
+    export class Context implements IContext {
+        constructor(id: number, parent: Context | null);
+        setValue(key: string, value: any): boolean;
+        removeValue(key: string): boolean;
+        getValue<T>(key: string): T | undefined;
+        updateParent(parent: Context): void;
+        collectAllValues(): Record<string, any>;
+    }
+
     // https://github.com/theia-ide/vscode/blob/standalone/0.23.x/src/vs/platform/contextkey/common/contextkey.ts#L1333
     export interface IContextKeyChangeEvent {
         affectsSome(keys: Set<string>): boolean;
     }
 
-    // https://github.com/theia-ide/vscode/blob/standalone/0.23.x/src/vs/platform/contextkey/browser/contextKeyService.ts#L352
-    export class ContextKeyService implements IContextKeyService {
-        constructor(configurationService: monaco.services.IConfigurationService);
+    // https://github.com/theia-ide/vscode/blob/e930e4240ee604757efbd7fd621b77b75568f95d/src/vs/platform/contextkey/browser/contextKeyService.ts#L247
+    export abstract class AbstractContextKeyService implements IContextKeyService {
+        constructor(myContextId: number);
+        get contextId(): number;
         onDidChangeContext: monaco.IEvent<IContextKeyChangeEvent>;
-        bufferChangeEvents(callback: Function): void;
-
         createKey<T>(key: string, defaultValue: T | undefined): IContextKey<T>;
+        bufferChangeEvents(callback: Function): void;
+        createScoped(target?: HTMLElement): AbstractContextKeyService;
+        createOverlay(overlay: Iterable<[string, any]>): IContextKeyService;
         contextMatchesRules(rules: monaco.contextkey.ContextKeyExpression | undefined): boolean;
         getContextKeyValue<T>(key: string): T | undefined;
+        setContext(key: string, value: any): void;
+        removeContext(key: string): void;
+        getContext(target: HTMLElement | null): IContext;
 
-        createScoped(target?: HTMLElement): IContextKeyService;
-        createOverlay(overlay: Iterable<[string, any]>): IContextKeyService;
-        getContext(target?: HTMLElement): IContext;
+        abstract dispose(): void;
+        abstract getContextValuesContainer(contextId: number): Context;
+        abstract createChildContext(parentContextId?: number): number;
+        abstract disposeContext(contextId: number): void;
+        abstract updateParent(): void;
+    }
 
-        updateParent(parentContextKeyService: IContextKeyService): void;
+    // https://github.com/theia-ide/vscode/blob/standalone/0.23.x/src/vs/platform/contextkey/browser/contextKeyService.ts#L352
+    export class ContextKeyService extends AbstractContextKeyService {
+        constructor(configurationService: monaco.services.IConfigurationService);
+        dispose(): void;
+        getContextValuesContainer(contextId: number): Context;
+        createChildContext(parentContextId?: number): number;
+        disposeContext(contextId: number): void;
+        updateParent(): void;
     }
 }
 

packages/plugin-ext/src/main/browser/menus/menus-contribution-handler.ts

@@ -242,14 +242,10 @@ export class MenusContributionPointHandler {
     }
 
     protected registerViewTitleAction(location: string, action: Menu): Disposable {
-        return this.registerTitleAction(location, { ...action, when: undefined }, {
+        return this.registerTitleAction(location, action, {
             execute: widget => widget instanceof PluginViewWidget && this.commands.executeCommand(action.command!),
-            isEnabled: widget => widget instanceof PluginViewWidget &&
-                this.viewContextKeys.with({ view: widget.options.viewId }, () =>
-                    this.commands.isEnabled(action.command!) && this.viewContextKeys.match(action.when)),
-            isVisible: widget => widget instanceof PluginViewWidget &&
-                this.viewContextKeys.with({ view: widget.options.viewId }, () =>
-                    this.commands.isVisible(action.command!) && this.viewContextKeys.match(action.when))
+            isEnabled: widget => widget instanceof PluginViewWidget && this.commands.isEnabled(action.command!),
+            isVisible: widget => widget instanceof PluginViewWidget && this.commands.isVisible(action.command!),
         });
     }