fix PreferenceChangeEvent<T> type

Despite precisely knowing for a given `preferenceName` what its
associated type for `newValue` and `oldValue` should be, the current
typing wasn't allowing TypeScript to properly infer the type.

With this commit, we can do things like:

    declare const event: PreferenceChangeEvent<{
        a: string
	b: number
    }>
    if (event.preferenceName === 'a') {
        event.newValue // type is `string`
    }

The type is more complex, but it now represents how it is meant to be
used rather than getting tsserver lost like before.

This commit also fixes places where the typing was broken and makes use
of the new information.

Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com>

examples/api-samples/src/browser/file-watching/sample-file-watching-contribution.ts

@@ -69,7 +69,7 @@ class SampleFileWatchingContribution implements FrontendApplicationContribution
         this.verbose = this.fileWatchingPreferences['sample.file-watching.verbose'];
         this.fileWatchingPreferences.onPreferenceChanged(e => {
             if (e.preferenceName === 'sample.file-watching.verbose') {
-                this.verbose = e.newValue!;
+                this.verbose = e.newValue;
             }
         });
     }

packages/core/src/browser/preferences/preference-proxy.ts

@@ -21,12 +21,61 @@ import { PreferenceService } from './preference-service';
 import { PreferenceSchema, OverridePreferenceName } from './preference-contribution';
 import { PreferenceScope } from './preference-scope';
 
-export interface PreferenceChangeEvent<T> {
-    readonly preferenceName: keyof T;
-    readonly newValue?: T[keyof T];
-    readonly oldValue?: T[keyof T];
+/**
+ * It is worth explaining the type for `PreferenceChangeEvent`:
+ *
+ * // Given T:
+ * type T = { a: string, b: number }
+ *
+ * // We construct a new type such as:
+ * type U = {
+ *     a: {
+ *         preferenceName: 'a'
+ *         newValue: string
+ *         oldValue?: string
+ *     }
+ *     b: {
+ *        preferenceName: 'b'
+ *        newValue: number
+ *        oldValue?: number
+ *     }
+ * }
+ *
+ * // Then we get the union of all values of U by selecting by `keyof T`:
+ * type V = U[keyof T]
+ *
+ * // Implementation:
+ * type PreferenceChangeEvent<T> = {
+ *     // Create a mapping where each key is a key from T,
+ *     // -? normalizes optional typings to avoid getting
+ *     // `undefined` as part of the final union:
+ *     [K in keyof T]-?: {
+ *         // In this object, K will take the value of each
+ *         // independent key from T:
+ *         preferenceName: K
+ *         newValue: T[K]
+ *         oldValue?: T[K]
+ *     // Finally we create the union by doing so:
+ *     }[keyof T]
+ * }
+ */
+
+/**
+ * Union of all possible key/value pairs for a type `T`
+ */
+export type PreferenceChangeEvent<T> = {
     affects(resourceUri?: string, overrideIdentifier?: string): boolean;
-}
+} & {
+    [K in keyof T]-?: {
+        readonly preferenceName: K;
+        readonly newValue: T[K];
+        /**
+         * Undefined if the preference is set for the first time.
+         */
+        // TODO: Use the default value instead of undefined?
+        readonly oldValue?: T[K];
+    }
+}[keyof T];
 
 export interface PreferenceEventEmitter<T> {
     readonly onPreferenceChanged: Event<PreferenceChangeEvent<T>>;

packages/git/src/browser/diff/git-diff-widget.tsx

@@ -16,7 +16,7 @@
 
 import { inject, injectable, postConstruct } from 'inversify';
 import {
-    BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, Message, MessageLoop, PreferenceChangeEvent
+    BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, Message, MessageLoop
 } from '@theia/core/lib/browser';
 import { EditorManager, DiffNavigatorProvider } from '@theia/editor/lib/browser';
 import { GitDiffTreeModel } from './git-diff-tree-model';
@@ -25,7 +25,7 @@ import { GitDiffHeaderWidget } from './git-diff-header-widget';
 import { ScmService } from '@theia/scm/lib/browser/scm-service';
 import { GitRepositoryProvider } from '../git-repository-provider';
 import { ScmTreeWidget } from '@theia/scm/lib/browser/scm-tree-widget';
-import { ScmPreferences, ScmConfiguration } from '@theia/scm/lib/browser/scm-preferences';
+import { ScmPreferences } from '@theia/scm/lib/browser/scm-preferences';
 
 /* eslint-disable @typescript-eslint/no-explicit-any */
 
@@ -76,9 +76,9 @@ export class GitDiffWidget extends BaseWidget implements StatefulWidget {
         this.containerLayout.addWidget(this.resourceWidget);
 
         this.updateViewMode(this.scmPreferences.get('scm.defaultViewMode'));
-        this.toDispose.push(this.scmPreferences.onPreferenceChanged((e: PreferenceChangeEvent<ScmConfiguration>) => {
+        this.toDispose.push(this.scmPreferences.onPreferenceChanged(e => {
             if (e.preferenceName === 'scm.defaultViewMode') {
-                this.updateViewMode(e.newValue!);
+                this.updateViewMode(e.newValue);
             }
         }));
     }

packages/git/src/browser/history/git-commit-detail-widget.tsx

@@ -19,14 +19,14 @@
 import { Message } from '@phosphor/messaging';
 import { injectable, inject, postConstruct } from 'inversify';
 import {
-    BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, MessageLoop, PreferenceChangeEvent
+    BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, MessageLoop
 } from '@theia/core/lib/browser';
 import { GitCommitDetailWidgetOptions } from './git-commit-detail-widget-options';
 import { GitCommitDetailHeaderWidget } from './git-commit-detail-header-widget';
 import { ScmService } from '@theia/scm/lib/browser/scm-service';
 import { GitDiffTreeModel } from '../diff/git-diff-tree-model';
 import { ScmTreeWidget } from '@theia/scm/lib/browser/scm-tree-widget';
-import { ScmPreferences, ScmConfiguration } from '@theia/scm/lib/browser/scm-preferences';
+import { ScmPreferences } from '@theia/scm/lib/browser/scm-preferences';
 
 @injectable()
 export class GitCommitDetailWidget extends BaseWidget implements StatefulWidget {
@@ -76,9 +76,9 @@ export class GitCommitDetailWidget extends BaseWidget implements StatefulWidget
         this.containerLayout.addWidget(this.resourceWidget);
 
         this.updateViewMode(this.scmPreferences.get('scm.defaultViewMode'));
-        this.toDispose.push(this.scmPreferences.onPreferenceChanged((e: PreferenceChangeEvent<ScmConfiguration>) => {
+        this.toDispose.push(this.scmPreferences.onPreferenceChanged(e => {
             if (e.preferenceName === 'scm.defaultViewMode') {
-                this.updateViewMode(e.newValue!);
+                this.updateViewMode(e.newValue);
             }
         }));
 

packages/markers/src/browser/problem/problem-decorator.ts

@@ -25,8 +25,7 @@ import { TreeDecorator, TreeDecoration } from '@theia/core/lib/browser/tree/tree
 import { FileStatNode } from '@theia/filesystem/lib/browser';
 import { Marker } from '../../common/marker';
 import { ProblemManager } from './problem-manager';
-import { ProblemPreferences, ProblemConfiguration } from './problem-preferences';
-import { PreferenceChangeEvent } from '@theia/core/lib/browser';
+import { ProblemPreferences } from './problem-preferences';
 import { ProblemUtils } from './problem-utils';
 
 @injectable()
@@ -46,10 +45,9 @@ export class ProblemDecorator implements TreeDecorator {
 
     @postConstruct()
     protected init(): void {
-        this.problemPreferences.onPreferenceChanged((event: PreferenceChangeEvent<ProblemConfiguration>) => {
-            const { preferenceName } = event;
-            if (preferenceName === 'problems.decorations.enabled') {
-                this.fireDidChangeDecorations((tree: Tree) => this.collectDecorators(tree));
+        this.problemPreferences.onPreferenceChanged(event => {
+            if (event.preferenceName === 'problems.decorations.enabled') {
+                this.fireDidChangeDecorations(tree => this.collectDecorators(tree));
             }
         });
     }

packages/monaco/src/browser/textmate/monaco-textmate-service.ts

@@ -103,7 +103,7 @@ export class MonacoTextmateService implements FrontendApplicationContribution {
         this.tokenizerOption.lineLimit = this.preferences['editor.maxTokenizationLineLength'];
         this.preferences.onPreferenceChanged(e => {
             if (e.preferenceName === 'editor.maxTokenizationLineLength') {
-                this.tokenizerOption.lineLimit = this.preferences['editor.maxTokenizationLineLength'];
+                this.tokenizerOption.lineLimit = e.newValue;
             }
         });
 

packages/output/src/common/output-channel.ts

@@ -25,7 +25,6 @@ import { MonacoTextModelService, IReference } from '@theia/monaco/lib/browser/mo
 import { OutputUri } from './output-uri';
 import { OutputResource } from '../browser/output-resource';
 import { OutputPreferences } from './output-preferences';
-import { OutputConfigSchema } from './output-preferences';
 
 @injectable()
 export class OutputChannelManager implements Disposable, ResourceResolver {
@@ -221,9 +220,9 @@ export class OutputChannel implements Disposable {
         this._maxLineNumber = this.preferences['output.maxChannelHistory'];
         this.toDispose.push(resource);
         this.toDispose.push(Disposable.create(() => this.decorationIds.clear()));
-        this.toDispose.push(this.preferences.onPreferenceChanged(({ preferenceName, newValue }) => {
-            if (preferenceName === 'output.maxChannelHistory') {
-                const maxLineNumber = newValue ? newValue : OutputConfigSchema.properties['output.maxChannelHistory'].default;
+        this.toDispose.push(this.preferences.onPreferenceChanged(event => {
+            if (event.preferenceName === 'output.maxChannelHistory') {
+                const maxLineNumber = event.newValue;
                 if (this.maxLineNumber !== maxLineNumber) {
                     this.maxLineNumber = maxLineNumber;
                 }

packages/preview/src/browser/preview-widget.ts

@@ -83,7 +83,7 @@ export class PreviewWidget extends BaseWidget implements Navigatable {
         this.scrollBeyondLastLine = !!this.editorPreferences['editor.scrollBeyondLastLine'];
         this.toDispose.push(this.editorPreferences.onPreferenceChanged(e => {
             if (e.preferenceName === 'editor.scrollBeyondLastLine') {
-                this.scrollBeyondLastLine = !!e.newValue;
+                this.scrollBeyondLastLine = e.newValue;
                 this.forceUpdate();
             }
         }));

packages/scm/src/browser/scm-widget.tsx

@@ -20,14 +20,14 @@ import { Message } from '@phosphor/messaging';
 import { injectable, inject, postConstruct } from 'inversify';
 import { DisposableCollection } from '@theia/core/lib/common/disposable';
 import {
-    BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, MessageLoop, PreferenceChangeEvent, CompositeTreeNode, SelectableTreeNode,
+    BaseWidget, Widget, StatefulWidget, Panel, PanelLayout, MessageLoop, CompositeTreeNode, SelectableTreeNode,
 } from '@theia/core/lib/browser';
 import { ScmCommitWidget } from './scm-commit-widget';
 import { ScmAmendWidget } from './scm-amend-widget';
 import { ScmNoRepositoryWidget } from './scm-no-repository-widget';
 import { ScmService } from './scm-service';
 import { ScmTreeWidget } from './scm-tree-widget';
-import { ScmPreferences, ScmConfiguration } from './scm-preferences';
+import { ScmPreferences } from './scm-preferences';
 
 @injectable()
 export class ScmWidget extends BaseWidget implements StatefulWidget {
@@ -78,9 +78,9 @@ export class ScmWidget extends BaseWidget implements StatefulWidget {
         this.refresh();
         this.toDispose.push(this.scmService.onDidChangeSelectedRepository(() => this.refresh()));
         this.updateViewMode(this.scmPreferences.get('scm.defaultViewMode'));
-        this.toDispose.push(this.scmPreferences.onPreferenceChanged((e: PreferenceChangeEvent<ScmConfiguration>) => {
+        this.toDispose.push(this.scmPreferences.onPreferenceChanged(e => {
             if (e.preferenceName === 'scm.defaultViewMode') {
-                this.updateViewMode(e.newValue!);
+                this.updateViewMode(e.newValue);
             }
         }));
 

packages/terminal/src/browser/terminal-preferences.ts

@@ -151,6 +151,7 @@ export const TerminalConfigSchema: PreferenceSchema = {
 export interface TerminalConfiguration {
     'terminal.enableCopy': boolean
     'terminal.enablePaste': boolean
+    // xterm compatible, see https://xtermjs.org/docs/api/terminal/interfaces/iterminaloptions/
     'terminal.integrated.fontFamily': string
     'terminal.integrated.fontSize': number
     'terminal.integrated.fontWeight': FontWeight
@@ -165,9 +166,9 @@ export interface TerminalConfiguration {
     'terminal.integrated.cursorBlinking': boolean,
     'terminal.integrated.cursorStyle': CursorStyleVSCode,
     'terminal.integrated.cursorWidth': number,
-    'terminal.integrated.shell.windows': string | undefined,
-    'terminal.integrated.shell.osx': string | undefined,
-    'terminal.integrated.shell.linux': string | undefined,
+    'terminal.integrated.shell.windows': string | null | undefined,
+    'terminal.integrated.shell.osx': string | null | undefined,
+    'terminal.integrated.shell.linux': string | null | undefined,
     'terminal.integrated.shellArgs.windows': string[],
     'terminal.integrated.shellArgs.osx': string[],
     'terminal.integrated.shellArgs.linux': string[],

packages/terminal/src/browser/terminal-widget-impl.ts

@@ -148,18 +148,16 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget
             const lastSeparator = change.preferenceName.lastIndexOf('.');
             if (lastSeparator > 0) {
                 let preferenceName = change.preferenceName.substr(lastSeparator + 1);
-                let preferenceValue = this.preferences[change.preferenceName];
+                let preferenceValue = change.newValue;
 
                 if (preferenceName === 'rendererType') {
-                    const newRendererType: string = this.preferences[change.preferenceName] as string;
+                    const newRendererType = preferenceValue as string;
                     if (newRendererType !== this.getTerminalRendererType(newRendererType)) {
-                        // given terminal renderer type is not supported or invalid
+                        // Given terminal renderer type is not supported or invalid
                         preferenceValue = DEFAULT_TERMINAL_RENDERER_TYPE;
                     }
-                }
-
-                // Convert the terminal preference into a valid `xterm` option.
-                if (preferenceName === 'cursorBlinking') {
+                } else if (preferenceName === 'cursorBlinking') {
+                    // Convert the terminal preference into a valid `xterm` option
                     preferenceName = 'cursorBlink';
                 } else if (preferenceName === 'cursorStyle') {
                     preferenceValue = this.getCursorStyle();
@@ -635,9 +633,9 @@ export class TerminalWidgetImpl extends TerminalWidget implements StatefulWidget
     protected get shellPreferences(): IShellTerminalPreferences {
         return {
             shell: {
-                Windows: this.preferences['terminal.integrated.shell.windows'],
-                Linux: this.preferences['terminal.integrated.shell.linux'],
-                OSX: this.preferences['terminal.integrated.shell.osx'],
+                Windows: this.preferences['terminal.integrated.shell.windows'] ?? undefined,
+                Linux: this.preferences['terminal.integrated.shell.linux'] ?? undefined,
+                OSX: this.preferences['terminal.integrated.shell.osx'] ?? undefined,
             },
             shellArgs: {
                 Windows: this.preferences['terminal.integrated.shellArgs.windows'],

packages/workspace/src/browser/workspace-service.ts

@@ -95,8 +95,8 @@ export class WorkspaceService implements FrontendApplicationContribution {
                 this.updateWorkspace();
             }
         });
-        this.fsPreferences.onPreferenceChanged(e => {
-            if (e.preferenceName === 'files.watcherExclude') {
+        this.fsPreferences.onPreferenceChanged(event => {
+            if (event.preferenceName === 'files.watcherExclude') {
                 this.refreshRootWatchers();
             }
         });