From ffe7d59b745b20411e953f1fcaa1072ff81c509d Mon Sep 17 00:00:00 2001 From: mathuo <6710312+mathuo@users.noreply.github.com> Date: Fri, 3 May 2024 20:06:43 +0100 Subject: [PATCH 1/2] bug/feat: use a better implementation of 'asap scheduled' (and buffered per event-cycle) events --- packages/dockview-core/src/events.ts | 50 ++++++++++++++++--- .../src/gridview/baseComponentGridview.ts | 4 +- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/packages/dockview-core/src/events.ts b/packages/dockview-core/src/events.ts index 160a3b96c..8916543c5 100644 --- a/packages/dockview-core/src/events.ts +++ b/packages/dockview-core/src/events.ts @@ -203,19 +203,53 @@ export function addDisposableListener( }; } -export class TickDelayedEvent implements IDisposable { - private timer: any; - +/** + * + * Event Emitter that fires events from a Microtask callback, only one event will fire per event-loop cycle. + * + * It's kind of like using an `asapScheduler` in RxJs with additional logic to only fire once per event-loop cycle. + * This implementation exists to avoid external dependencies. + * + * @see https://developer.mozilla.org/en-US/docs/Web/API/queueMicrotask + * @see https://rxjs.dev/api/index/const/asapScheduler + */ +export class AsapEvent implements IDisposable { private readonly _onFired = new Emitter(); - readonly onEvent = this._onFired.event; + private _currentFireCount = 0; + private _queued = false; + + readonly onEvent: Event = (e) => { + /** + * when the event is first subscribed to take note of the current fire count + */ + const fireCountAtTimeOfEventSubscription = this._currentFireCount; + + return this._onFired.event(() => { + /** + * if the current fire count is greater than the fire count at event subscription + * then the event has been fired since we subscribed and it's ok to "on_next" the event. + * + * if the count is not greater then what we are recieving is an event from the microtask + * queue that was triggered before we actually subscribed and therfore we should ignore it. + */ + if (this._currentFireCount > fireCountAtTimeOfEventSubscription) { + e(); + } + }); + }; fire(): void { - if (this.timer) { - clearTimeout(this.timer); + this._currentFireCount++; + + if (this._queued) { + return; } - this.timer = setTimeout(() => { + + this._queued = true; + + queueMicrotask(() => { + this._queued = false; this._onFired.fire(); - clearTimeout(this.timer); }); } diff --git a/packages/dockview-core/src/gridview/baseComponentGridview.ts b/packages/dockview-core/src/gridview/baseComponentGridview.ts index 3150a4c81..e6e572786 100644 --- a/packages/dockview-core/src/gridview/baseComponentGridview.ts +++ b/packages/dockview-core/src/gridview/baseComponentGridview.ts @@ -1,4 +1,4 @@ -import { Emitter, Event, TickDelayedEvent } from '../events'; +import { Emitter, Event, AsapEvent } from '../events'; import { getGridLocation, Gridview, IGridView } from './gridview'; import { Position } from '../dnd/droptarget'; import { Disposable, IValueDisposable } from '../lifecycle'; @@ -92,7 +92,7 @@ export abstract class BaseGrid readonly onDidActiveChange: Event = this._onDidActiveChange.event; - protected readonly _bufferOnDidLayoutChange = new TickDelayedEvent(); + protected readonly _bufferOnDidLayoutChange = new AsapEvent(); get id(): string { return this._id; From 61abf780d177b69e79f507390eea6ef49ec78d4d Mon Sep 17 00:00:00 2001 From: mathuo <6710312+mathuo@users.noreply.github.com> Date: Sun, 5 May 2024 13:44:21 +0100 Subject: [PATCH 2/2] feat: improve onDidLayoutChange events --- .../dockview/dockviewComponent.spec.ts | 54 +++++++++++++++++++ .../src/__tests__/events.spec.ts | 36 +++++++++++++ .../src/gridview/baseComponentGridview.ts | 13 ++--- 3 files changed, 94 insertions(+), 9 deletions(-) diff --git a/packages/dockview-core/src/__tests__/dockview/dockviewComponent.spec.ts b/packages/dockview-core/src/__tests__/dockview/dockviewComponent.spec.ts index 17497364f..b429a32be 100644 --- a/packages/dockview-core/src/__tests__/dockview/dockviewComponent.spec.ts +++ b/packages/dockview-core/src/__tests__/dockview/dockviewComponent.spec.ts @@ -5331,4 +5331,58 @@ describe('dockviewComponent', () => { expect(addGroupCount).toBe(2); }); }); + + test('that `onDidLayoutChange` only subscribes to events after initial subscription time', () => { + jest.useFakeTimers(); + + const container = document.createElement('div'); + + const dockview = new DockviewComponent({ + parentElement: container, + createComponent(options) { + switch (options.name) { + case 'default': + return new PanelContentPartTest( + options.id, + options.name + ); + default: + throw new Error(`unsupported`); + } + }, + }); + const api = new DockviewApi(dockview); + + dockview.layout(1000, 1000); + + let a = 0; + + api.onDidLayoutChange((e) => { + a++; + }); + + api.addPanel({ + id: 'panel_1', + component: 'default', + }); + api.addPanel({ + id: 'panel_2', + component: 'default', + }); + api.addPanel({ + id: 'panel_3', + component: 'default', + }); + + let b = 0; + + api.onDidLayoutChange((e) => { + b++; + }); + + jest.runAllTicks(); + + expect(a).toBe(1); + expect(b).toBe(0); + }); }); diff --git a/packages/dockview-core/src/__tests__/events.spec.ts b/packages/dockview-core/src/__tests__/events.spec.ts index 532390a07..af05c3f28 100644 --- a/packages/dockview-core/src/__tests__/events.spec.ts +++ b/packages/dockview-core/src/__tests__/events.spec.ts @@ -1,4 +1,5 @@ import { + AsapEvent, Emitter, Event, addDisposableListener, @@ -82,6 +83,41 @@ describe('events', () => { }); }); + describe('asapEvent', () => { + test('that asapEvents fire once per event-loop-cycle', () => { + jest.useFakeTimers(); + + const event = new AsapEvent(); + + let preFireCount = 0; + let postFireCount = 0; + + event.onEvent(() => { + preFireCount++; + }); + + for (let i = 0; i < 100; i++) { + event.fire(); + } + + /** + * check that subscribing after the events have fired but before the event-loop cycle completes + * results in no event fires. + */ + event.onEvent((e) => { + postFireCount++; + }); + + expect(preFireCount).toBe(0); + expect(postFireCount).toBe(0); + + jest.runAllTimers(); + + expect(preFireCount).toBe(1); + expect(postFireCount).toBe(0); + }); + }); + it('should emit a value when any event fires', () => { const emitter1 = new Emitter(); const emitter2 = new Emitter(); diff --git a/packages/dockview-core/src/gridview/baseComponentGridview.ts b/packages/dockview-core/src/gridview/baseComponentGridview.ts index e6e572786..1e9c3102a 100644 --- a/packages/dockview-core/src/gridview/baseComponentGridview.ts +++ b/packages/dockview-core/src/gridview/baseComponentGridview.ts @@ -76,11 +76,8 @@ export abstract class BaseGrid private readonly _id = nextLayoutId.next(); protected readonly _groups = new Map>(); protected readonly gridview: Gridview; - // - protected _activeGroup: T | undefined; - private _onDidLayoutChange = new Emitter(); - readonly onDidLayoutChange = this._onDidLayoutChange.event; + protected _activeGroup: T | undefined; private readonly _onDidRemove = new Emitter(); readonly onDidRemove: Event = this._onDidRemove.event; @@ -93,6 +90,8 @@ export abstract class BaseGrid this._onDidActiveChange.event; protected readonly _bufferOnDidLayoutChange = new AsapEvent(); + readonly onDidLayoutChange: Event = + this._bufferOnDidLayoutChange.onEvent; get id(): string { return this._id; @@ -172,9 +171,6 @@ export abstract class BaseGrid )(() => { this._bufferOnDidLayoutChange.fire(); }), - this._bufferOnDidLayoutChange.onEvent(() => { - this._onDidLayoutChange.fire(); - }), this._bufferOnDidLayoutChange ); } @@ -187,7 +183,7 @@ export abstract class BaseGrid public setVisible(panel: T, visible: boolean): void { this.gridview.setViewVisible(getGridLocation(panel.element), visible); - this._onDidLayoutChange.fire(); + this._bufferOnDidLayoutChange.fire(); } public isVisible(panel: T): boolean { @@ -330,7 +326,6 @@ export abstract class BaseGrid this._onDidActiveChange.dispose(); this._onDidAdd.dispose(); this._onDidRemove.dispose(); - this._onDidLayoutChange.dispose(); for (const group of this.groups) { group.dispose();