Skip to content

Commit ea5d706

Browse files
mjbvzmjbvz
authored
Fix dom on for TS's strict function mode (microsoft#37748)
**background** TS 2.6 introduces a new strict function checking mode. This can catch many common mistakes such as patterns like this: ```js function on(callback: (Base) => void) { ... } on((e: Sub) => {...}); ``` This does an implicit cast in the call to `on`. With strict function checking, this is now flagged as an error **Change** This change fixes dom.on so that it works with strict function checking. This done by making it generic so that the type of the event taken in the callback can be inferred from the callback itself.
1 parent 7bf5879 commit ea5d706

5 files changed

Lines changed: 21 additions & 21 deletions

File tree

‎src/vs/base/browser/builder.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -559,9 +559,9 @@ export class Builder implements IDisposable {
559559
/**
560560
* Registers listener on event types on the current element.
561561
*/
562-
public on(type: string, fn: (e: Event, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder;
563-
public on(typeArray: string[], fn: (e: Event, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder;
564-
public on(arg1: any, fn: (e: Event, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder {
562+
public on<E extends Event = Event>(type: string, fn: (e: E, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder;
563+
public on<E extends Event = Event>(typeArray: string[], fn: (e: E, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder;
564+
public on<E extends Event = Event>(arg1: any, fn: (e: E, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder {
565565

566566
// Event Type Array
567567
if (types.isArray(arg1)) {
@@ -575,7 +575,7 @@ export class Builder implements IDisposable {
575575
let type = arg1;
576576

577577
// Add Listener
578-
let unbind: IDisposable = DOM.addDisposableListener(this.currentElement, type, (e: Event) => {
578+
let unbind: IDisposable = DOM.addDisposableListener(this.currentElement, type, (e) => {
579579
fn(e, this, unbind); // Pass in Builder as Second Argument
580580
}, useCapture || false);
581581

@@ -641,9 +641,9 @@ export class Builder implements IDisposable {
641641
* Registers listener on event types on the current element and removes
642642
* them after first invocation.
643643
*/
644-
public once(type: string, fn: (e: Event, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder;
645-
public once(typesArray: string[], fn: (e: Event, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder;
646-
public once(arg1: any, fn: (e: Event, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder {
644+
public once<E extends Event = Event>(type: string, fn: (e: E, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder;
645+
public once<E extends Event = Event>(typesArray: string[], fn: (e: E, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder;
646+
public once<E extends Event = Event>(arg1: any, fn: (e: E, builder: Builder, unbind: IDisposable) => void, listenerToUnbindContainer?: IDisposable[], useCapture?: boolean): Builder {
647647

648648
// Event Type Array
649649
if (types.isArray(arg1)) {
@@ -657,7 +657,7 @@ export class Builder implements IDisposable {
657657
let type = arg1;
658658

659659
// Add Listener
660-
let unbind: IDisposable = DOM.addDisposableListener(this.currentElement, type, (e: Event) => {
660+
let unbind: IDisposable = DOM.addDisposableListener(this.currentElement, type, (e) => {
661661
fn(e, this, unbind); // Pass in Builder as Second Argument
662662
unbind.dispose();
663663
}, useCapture || false);

‎src/vs/base/browser/dom.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -413,18 +413,18 @@ class AnimationFrameQueueItem implements IDisposable {
413413
/**
414414
* Add a throttled listener. `handler` is fired at most every 16ms or with the next animation frame (if browser supports it).
415415
*/
416-
export interface IEventMerger<R> {
417-
(lastEvent: R, currentEvent: Event): R;
416+
export interface IEventMerger<R, E> {
417+
(lastEvent: R, currentEvent: E): R;
418418
}
419419

420420
const MINIMUM_TIME_MS = 16;
421-
const DEFAULT_EVENT_MERGER: IEventMerger<Event> = function (lastEvent: Event, currentEvent: Event) {
421+
const DEFAULT_EVENT_MERGER: IEventMerger<Event, Event> = function (lastEvent: Event, currentEvent: Event) {
422422
return currentEvent;
423423
};
424424

425-
class TimeoutThrottledDomListener<R> extends Disposable {
425+
class TimeoutThrottledDomListener<R, E extends Event> extends Disposable {
426426

427-
constructor(node: any, type: string, handler: (event: R) => void, eventMerger: IEventMerger<R> = <any>DEFAULT_EVENT_MERGER, minimumTimeMs: number = MINIMUM_TIME_MS) {
427+
constructor(node: any, type: string, handler: (event: R) => void, eventMerger: IEventMerger<R, E> = <any>DEFAULT_EVENT_MERGER, minimumTimeMs: number = MINIMUM_TIME_MS) {
428428
super();
429429

430430
let lastEvent: R = null;
@@ -452,8 +452,8 @@ class TimeoutThrottledDomListener<R> extends Disposable {
452452
}
453453
}
454454

455-
export function addDisposableThrottledListener<R>(node: any, type: string, handler: (event: R) => void, eventMerger?: IEventMerger<R>, minimumTimeMs?: number): IDisposable {
456-
return new TimeoutThrottledDomListener<R>(node, type, handler, eventMerger, minimumTimeMs);
455+
export function addDisposableThrottledListener<R, E extends Event = Event>(node: any, type: string, handler: (event: R) => void, eventMerger?: IEventMerger<R, E>, minimumTimeMs?: number): IDisposable {
456+
return new TimeoutThrottledDomListener<R, E>(node, type, handler, eventMerger, minimumTimeMs);
457457
}
458458

459459
export function getComputedStyle(el: HTMLElement): CSSStyleDeclaration {

‎src/vs/base/browser/globalMouseMoveMonitor.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ export class GlobalMouseMoveMonitor<R> extends Disposable {
9696
for (let i = 0; i < windowChain.length; i++) {
9797
this.hooks.push(dom.addDisposableThrottledListener(windowChain[i].window.document, 'mousemove',
9898
(data: R) => this.mouseMoveCallback(data),
99-
(lastEvent: R, currentEvent: MouseEvent) => this.mouseMoveEventMerger(lastEvent, currentEvent)
99+
(lastEvent: R, currentEvent) => this.mouseMoveEventMerger(lastEvent, currentEvent as MouseEvent)
100100
));
101101
this.hooks.push(dom.addDisposableListener(windowChain[i].window.document, 'mouseup', (e: MouseEvent) => this.stopMonitoring(true)));
102102
}

‎src/vs/editor/browser/editorDom.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,10 +135,10 @@ export class EditorMouseEventFactory {
135135
}
136136

137137
public onMouseMoveThrottled(target: HTMLElement, callback: (e: EditorMouseEvent) => void, merger: EditorMouseEventMerger, minimumTimeMs: number): IDisposable {
138-
let myMerger: dom.IEventMerger<EditorMouseEvent> = (lastEvent: EditorMouseEvent, currentEvent: MouseEvent): EditorMouseEvent => {
138+
let myMerger: dom.IEventMerger<EditorMouseEvent, MouseEvent> = (lastEvent: EditorMouseEvent, currentEvent: MouseEvent): EditorMouseEvent => {
139139
return merger(lastEvent, this._create(currentEvent));
140140
};
141-
return dom.addDisposableThrottledListener<EditorMouseEvent>(target, 'mousemove', callback, myMerger, minimumTimeMs);
141+
return dom.addDisposableThrottledListener<EditorMouseEvent, MouseEvent>(target, 'mousemove', callback, myMerger, minimumTimeMs);
142142
}
143143
}
144144

@@ -168,7 +168,7 @@ export class GlobalEditorMouseMoveMonitor extends Disposable {
168168
this._globalMouseMoveMonitor.stopMonitoring(true);
169169
}, true);
170170

171-
let myMerger: dom.IEventMerger<EditorMouseEvent> = (lastEvent: EditorMouseEvent, currentEvent: MouseEvent): EditorMouseEvent => {
171+
let myMerger: dom.IEventMerger<EditorMouseEvent, MouseEvent> = (lastEvent: EditorMouseEvent, currentEvent: MouseEvent): EditorMouseEvent => {
172172
return merger(lastEvent, new EditorMouseEvent(currentEvent, this._editorViewDomNode));
173173
};
174174

‎src/vs/workbench/parts/files/browser/editors/textFileEditor.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,8 @@ export class TextFileEditor extends BaseTextEditor {
190190

191191
// Best we can do is to reveal the folder in the explorer
192192
if (this.contextService.isInsideWorkspace(input.getResource())) {
193-
this.viewletService.openViewlet(VIEWLET_ID, true).done((viewlet: ExplorerViewlet) => {
194-
return viewlet.getExplorerView().select(input.getResource(), true);
193+
this.viewletService.openViewlet(VIEWLET_ID, true).done(viewlet => {
194+
return (viewlet as ExplorerViewlet).getExplorerView().select(input.getResource(), true);
195195
}, errors.onUnexpectedError);
196196
}
197197
}, errors.onUnexpectedError);

0 commit comments

Comments
 (0)