Skip to content

Commit

Permalink
fix PreferenceChangeEvent<T> type
Browse files Browse the repository at this point in the history
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:

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

It is worth explaining the new type for the `PreferenceChangeEvent`:

```ts
// 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]
}
```

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 <[email protected]>
  • Loading branch information
paul-marechal committed Feb 12, 2021
1 parent 6798167 commit 42bee66
Show file tree
Hide file tree
Showing 12 changed files with 39 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
});
}
Expand Down
17 changes: 12 additions & 5 deletions packages/core/src/browser/preferences/preference-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,19 @@ 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];
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>>;
Expand Down
6 changes: 3 additions & 3 deletions packages/git/src/browser/diff/git-diff-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 */

Expand Down Expand Up @@ -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);
}
}));
}
Expand Down
4 changes: 2 additions & 2 deletions packages/git/src/browser/history/git-commit-detail-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}));

Expand Down
7 changes: 3 additions & 4 deletions packages/markers/src/browser/problem/problem-decorator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,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));
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
});

Expand Down
6 changes: 3 additions & 3 deletions packages/output/src/common/output-channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,9 +221,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;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/preview/src/browser/preview-widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}));
Expand Down
4 changes: 2 additions & 2 deletions packages/scm/src/browser/scm-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}));

Expand Down
7 changes: 4 additions & 3 deletions packages/terminal/src/browser/terminal-preferences.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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[],
Expand Down
12 changes: 5 additions & 7 deletions packages/terminal/src/browser/terminal-widget-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions packages/workspace/src/browser/workspace-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
});
Expand Down

0 comments on commit 42bee66

Please sign in to comment.