From fbd1f03ae118de9205909ef26ec3051c10130e9a Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 13 May 2019 13:36:31 +0100 Subject: [PATCH 01/54] Remove `window` from activity monitor. --- packages/coreutils/src/activitymonitor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/coreutils/src/activitymonitor.ts b/packages/coreutils/src/activitymonitor.ts index 2f286d03b6bd..781ab18d43b0 100644 --- a/packages/coreutils/src/activitymonitor.ts +++ b/packages/coreutils/src/activitymonitor.ts @@ -65,7 +65,7 @@ export class ActivityMonitor implements IDisposable { clearTimeout(this._timer); this._sender = sender; this._args = args; - this._timer = window.setTimeout(() => { + this._timer = setTimeout(() => { this._activityStopped.emit({ sender: this._sender, args: this._args From d44f2a183d07266e8944d7ee7f7a0396b03fe57d Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 13 May 2019 13:37:17 +0100 Subject: [PATCH 02/54] [WIP] make state database use in-memory storage instead of local storage. --- packages/coreutils/src/statedb.ts | 253 +++++++++++++----------------- 1 file changed, 107 insertions(+), 146 deletions(-) diff --git a/packages/coreutils/src/statedb.ts b/packages/coreutils/src/statedb.ts index 4e035212303e..00802bb2791d 100644 --- a/packages/coreutils/src/statedb.ts +++ b/packages/coreutils/src/statedb.ts @@ -23,11 +23,6 @@ export const IStateDB = new Token('@jupyterlab/coreutils:IStateDB'); */ export interface IStateDB extends IDataConnector { - /** - * The maximum allowed length of the data after it has been serialized. - */ - readonly maxLength: number; - /** * The namespace prefix for all state database entries. * @@ -94,11 +89,6 @@ export class StateDB return this._changed; } - /** - * The maximum allowed length of the data after it has been serialized. - */ - readonly maxLength: number = 2000; - /** * The namespace prefix for all state database entries. * @@ -112,16 +102,13 @@ export class StateDB /** * Clear the entire database. */ - clear(silent = false): Promise { - return this._ready.then(() => { - this._clear(); - - if (silent) { - return; - } - - this._changed.emit({ id: null, type: 'clear' }); - }); + async clear(silent = false): Promise { + await this._ready; + this._clear(); + if (silent) { + return; + } + this._changed.emit({ id: null, type: 'clear' }); } /** @@ -164,13 +151,15 @@ export class StateDB * console in order to optimistically return any extant data without failing. * This promise will always succeed. */ - list(namespace: string): Promise<{ ids: string[]; values: T[] }> { - return this._ready.then(() => { - const prefix = `${this._window}:${this.namespace}:`; - const mask = (key: string) => key.replace(prefix, ''); + async list(namespace: string): Promise<{ ids: string[]; values: T[] }> { + await this._ready; - return Private.fetchNamespace(`${prefix}${namespace}:`, mask); - }); + const prefix = `${this._window}:${this.namespace}:`; + const list = await this._connector.list(`${prefix}${namespace}:`); + + list.ids = list.ids.map((key: string) => key.replace(prefix, '')); + + return list as { ids: string[]; values: T[] }; } /** @@ -180,11 +169,10 @@ export class StateDB * * @returns A promise that is rejected if remove fails and succeeds otherwise. */ - remove(id: string): Promise { - return this._ready.then(() => { - this._remove(id); - this._changed.emit({ id, type: 'remove' }); - }); + async remove(id: string): Promise { + await this._ready; + await this._remove(id); + this._changed.emit({ id, type: 'remove' }); } /** @@ -203,11 +191,10 @@ export class StateDB * requirement for `fetch()`, `remove()`, and `save()`, it *is* necessary for * using the `list(namespace: string)` method. */ - save(id: string, value: T): Promise { - return this._ready.then(() => { - this._save(id, value); - this._changed.emit({ id, type: 'save' }); - }); + async save(id: string, value: T): Promise { + await this._ready; + await this._save(id, value); + this._changed.emit({ id, type: 'save' }); } /** @@ -215,49 +202,48 @@ export class StateDB * * @returns A promise that bears the database contents as JSON. */ - toJSON(): Promise<{ [id: string]: T }> { - return this._ready.then(() => { - const prefix = `${this._window}:${this.namespace}:`; - const mask = (key: string) => key.replace(prefix, ''); + async toJSON(): Promise<{ [id: string]: T }> { + await this._ready; - return Private.toJSON(prefix, mask); - }); + const prefix = `${this._window}:${this.namespace}:`; + const { ids, values } = await this._connector.list(prefix); + + const transformed: { [id: string]: T } = values.reduce( + (acc: { [id: string]: T }, val, idx) => { + const key = ids[idx].replace(prefix, ''); + const envelope = JSON.parse(val as string) as Private.Envelope; + + acc[key] = envelope.v as T; + return acc; + }, + {} + ); + + return transformed; } /** * Clear the entire database. - * - * #### Notes - * Unlike the public `clear` method, this method is synchronous. */ - private _clear(): void { - const { localStorage } = window; + private async _clear(): Promise { + const connector = this._connector; const prefix = `${this._window}:${this.namespace}:`; - let i = localStorage.length; - - while (i) { - let key = localStorage.key(--i); + const removals = (await connector.list()).ids + .map(id => id && id.indexOf(prefix) === 0 && connector.remove(id)) + .filter(removal => !!removal); - if (key && key.indexOf(prefix) === 0) { - localStorage.removeItem(key); - } - } + await Promise.all(removals); } /** * Fetch a value from the database. - * - * #### Notes - * Unlike the public `fetch` method, this method is synchronous. */ - private _fetch(id: string): ReadonlyJSONValue | undefined { + private async _fetch(id: string): Promise { const key = `${this._window}:${this.namespace}:${id}`; - const value = window.localStorage.getItem(key); + const value = await this._connector.fetch(key); if (value) { - const envelope = JSON.parse(value) as Private.Envelope; - - return envelope.v; + return (JSON.parse(value) as Private.Envelope).v as T; } return undefined; @@ -266,53 +252,43 @@ export class StateDB /** * Merge data into the state database. */ - private _merge(contents: ReadonlyJSONObject): void { - Object.keys(contents).forEach(key => { - this._save(key, contents[key]); + private async _merge(contents: { [id: string]: T }): Promise { + const saves = Object.keys(contents).map(key => { + return this._save(key, contents[key]); }); + await Promise.all(saves); } /** * Overwrite the entire database with new contents. */ - private _overwrite(contents: ReadonlyJSONObject): void { - this._clear(); - this._merge(contents); + private async _overwrite(contents: { [id: string]: T }): Promise { + await this._clear(); + await this._merge(contents); } /** * Remove a key in the database. - * - * #### Notes - * Unlike the public `remove` method, this method is synchronous. */ - private _remove(id: string): void { + private async _remove(id: string): Promise { const key = `${this._window}:${this.namespace}:${id}`; - window.localStorage.removeItem(key); + return this._connector.remove(key); } /** * Save a key and its value in the database. - * - * #### Notes - * Unlike the public `save` method, this method is synchronous. */ - private _save(id: string, value: ReadonlyJSONValue): void { + private async _save(id: string, value: T): Promise { const key = `${this._window}:${this.namespace}:${id}`; const envelope: Private.Envelope = { v: value }; const serialized = JSON.stringify(envelope); - const length = serialized.length; - const max = this.maxLength; - - if (length > max) { - throw new Error(`Data length (${length}) exceeds maximum (${max})`); - } - window.localStorage.setItem(key, serialized); + return this._connector.save(key, serialized); } private _changed = new Signal(this); + private _connector: IDataConnector; private _ready: Promise; private _window: string; } @@ -358,6 +334,11 @@ export namespace StateDB { * The instantiation options for a state database. */ export interface IOptions { + /** + * Optional data connector for a database. Defaults to in-memory connector. + */ + connector?: IDataConnector; + /** * The namespace prefix for all state database entries. */ @@ -393,68 +374,48 @@ namespace Private { export type Envelope = { readonly v: ReadonlyJSONValue }; /** - * Retrieve all the saved bundles for a given namespace in local storage. - * - * @param prefix - The namespace to retrieve. - * - * @param mask - Optional mask function to transform each key retrieved. - * - * @returns A collection of data payloads for a given prefix. - * - * #### Notes - * If there are any errors in retrieving the data, they will be logged to the - * console in order to optimistically return any extant data without failing. + * The in-memory storage */ - export function fetchNamespace< - T extends ReadonlyJSONValue = ReadonlyJSONValue - >( - namespace: string, - mask: (key: string) => string = key => key - ): { ids: string[]; values: T[] } { - const { localStorage } = window; - - let ids: string[] = []; - let values: T[] = []; - let i = localStorage.length; - - while (i) { - let key = localStorage.key(--i); - - if (key && key.indexOf(namespace) === 0) { - let value = localStorage.getItem(key); - - try { - let envelope = JSON.parse(value) as Envelope; - let id = mask(key); - - values[ids.push(id) - 1] = envelope ? (envelope.v as T) : undefined; - } catch (error) { - console.warn(error); - localStorage.removeItem(key); - } - } - } + const storage: { [id: string]: string } = {}; - return { ids, values }; - } + export const connector: IDataConnector = { + /** + * Retrieve an item from the data connector. + */ + fetch: async (id: string): Promise => { + return storage[id]; + }, - /** - * Return a serialized copy of a namespace's contents from local storage. - * - * @returns The namespace contents as JSON. - */ - export function toJSON( - namespace: string, - mask: (key: string) => string = key => key - ): { [id: string]: T } { - const { ids, values } = fetchNamespace(namespace, mask); - - return values.reduce( - (acc, val, idx) => { - acc[ids[idx]] = val; - return acc; - }, - {} as { [id: string]: T } - ); - } + /** + * Retrieve the list of items available from the data connector. + */ + list: async ( + query: string + ): Promise<{ ids: string[]; values: string[] }> => { + return Object.keys(storage).reduce( + (acc, val) => { + if (val && val.indexOf(query) === 0) { + acc.ids.push(val); + acc.values.push(storage[val]); + } + return acc; + }, + { ids: [], values: [] } + ); + }, + + /** + * Remove a value using the data connector. + */ + remove: async (id: string): Promise => { + delete storage[id]; + }, + + /** + * Save a value using the data connector. + */ + save: async (id: string, value: string): Promise => { + storage[id] = value; + } + }; } From 311849778d8ae285f27acf99f6ad89d5d7f07354 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 13 May 2019 13:47:53 +0100 Subject: [PATCH 03/54] Handle initial state database transformation promises. --- packages/coreutils/src/statedb.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/coreutils/src/statedb.ts b/packages/coreutils/src/statedb.ts index 00802bb2791d..07ebfe2845ee 100644 --- a/packages/coreutils/src/statedb.ts +++ b/packages/coreutils/src/statedb.ts @@ -68,14 +68,11 @@ export class StateDB case 'cancel': return; case 'clear': - this._clear(); - return; + return this._clear(); case 'merge': - this._merge(contents || {}); - return; + return this._merge(contents || {}); case 'overwrite': - this._overwrite(contents || {}); - return; + return this._overwrite(contents || {}); default: return; } From 7548bf043b0f25858bfebf07bee44e6e47ae7958 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 13 May 2019 15:38:54 +0100 Subject: [PATCH 04/54] [WIP] State database refactor. --- packages/coreutils/src/statedb.ts | 67 ++++++++++++++----------------- 1 file changed, 31 insertions(+), 36 deletions(-) diff --git a/packages/coreutils/src/statedb.ts b/packages/coreutils/src/statedb.ts index 07ebfe2845ee..4a35856b6c8e 100644 --- a/packages/coreutils/src/statedb.ts +++ b/packages/coreutils/src/statedb.ts @@ -52,10 +52,11 @@ export class StateDB * @param options - The instantiation options for a state database. */ constructor(options: StateDB.IOptions) { - const { namespace, transform, windowName } = options; + const { connector, namespace, transform, windowName } = options; this.namespace = namespace; + this._connector = connector || Private.connector; this._window = windowName || ''; this._ready = (transform || Promise.resolve(null)).then(transformation => { if (!transformation) { @@ -101,7 +102,7 @@ export class StateDB */ async clear(silent = false): Promise { await this._ready; - this._clear(); + await this._clear(); if (silent) { return; } @@ -127,9 +128,8 @@ export class StateDB * `undefined`. */ async fetch(id: string): Promise { - const value = await this._ready.then(() => this._fetch(id)); - - return value as T; + await this._ready; + return this._fetch(id); } /** @@ -150,13 +150,7 @@ export class StateDB */ async list(namespace: string): Promise<{ ids: string[]; values: T[] }> { await this._ready; - - const prefix = `${this._window}:${this.namespace}:`; - const list = await this._connector.list(`${prefix}${namespace}:`); - - list.ids = list.ids.map((key: string) => key.replace(prefix, '')); - - return list as { ids: string[]; values: T[] }; + return this._list(namespace); } /** @@ -202,34 +196,22 @@ export class StateDB async toJSON(): Promise<{ [id: string]: T }> { await this._ready; - const prefix = `${this._window}:${this.namespace}:`; - const { ids, values } = await this._connector.list(prefix); - - const transformed: { [id: string]: T } = values.reduce( - (acc: { [id: string]: T }, val, idx) => { - const key = ids[idx].replace(prefix, ''); - const envelope = JSON.parse(val as string) as Private.Envelope; + const { ids, values } = await this._list(); - acc[key] = envelope.v as T; + return values.reduce( + (acc, val, idx) => { + acc[ids[idx]] = val; return acc; }, - {} + {} as { [id: string]: T } ); - - return transformed; } /** * Clear the entire database. */ private async _clear(): Promise { - const connector = this._connector; - const prefix = `${this._window}:${this.namespace}:`; - const removals = (await connector.list()).ids - .map(id => id && id.indexOf(prefix) === 0 && connector.remove(id)) - .filter(removal => !!removal); - - await Promise.all(removals); + await Promise.all((await this._list()).ids.map(id => this._remove(id))); } /** @@ -246,14 +228,28 @@ export class StateDB return undefined; } + /** + * Fetch a list from the database. + */ + private async _list(query?: string): Promise<{ ids: string[]; values: T[] }> { + const prefix = `${this._window}:${this.namespace}:`; + const { ids, values } = await this._connector.list( + query ? `${prefix}${query}:` : prefix + ); + + return { + ids: ids.map((key: string) => key.replace(prefix, '')), + values: values.map(val => (JSON.parse(val) as Private.Envelope).v as T) + }; + } + /** * Merge data into the state database. */ private async _merge(contents: { [id: string]: T }): Promise { - const saves = Object.keys(contents).map(key => { - return this._save(key, contents[key]); - }); - await Promise.all(saves); + await Promise.all( + Object.keys(contents).map(key => this._save(key, contents[key])) + ); } /** @@ -278,8 +274,7 @@ export class StateDB */ private async _save(id: string, value: T): Promise { const key = `${this._window}:${this.namespace}:${id}`; - const envelope: Private.Envelope = { v: value }; - const serialized = JSON.stringify(envelope); + const serialized = JSON.stringify({ v: value } as Private.Envelope); return this._connector.save(key, serialized); } From dfcbd4c65b5c10724f1967ac6bb0dc4e42d71851 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 13 May 2019 16:05:31 +0100 Subject: [PATCH 05/54] Remove maxLength test. --- packages/coreutils/src/statedb.ts | 2 -- tests/test-coreutils/src/statedb.spec.ts | 15 --------------- 2 files changed, 17 deletions(-) diff --git a/packages/coreutils/src/statedb.ts b/packages/coreutils/src/statedb.ts index 4a35856b6c8e..cae20e4efb9e 100644 --- a/packages/coreutils/src/statedb.ts +++ b/packages/coreutils/src/statedb.ts @@ -224,8 +224,6 @@ export class StateDB if (value) { return (JSON.parse(value) as Private.Envelope).v as T; } - - return undefined; } /** diff --git a/tests/test-coreutils/src/statedb.spec.ts b/tests/test-coreutils/src/statedb.spec.ts index bff2d2d32126..1d92c9561638 100644 --- a/tests/test-coreutils/src/statedb.spec.ts +++ b/tests/test-coreutils/src/statedb.spec.ts @@ -87,21 +87,6 @@ describe('StateDB', () => { }); }); - describe('#maxLength', () => { - it('should enforce the maximum length of a stored item', async () => { - const db = new StateDB({ namespace: 'test' }); - const key = 'test-key'; - const data = { a: new Array(db.maxLength).join('A') }; - let failed = false; - try { - await db.save(key, data); - } catch (e) { - failed = true; - } - expect(failed).to.equal(true); - }); - }); - describe('#namespace', () => { it('should be the read-only internal namespace', () => { const namespace = 'test-namespace'; From 845084e816e8c74f6bc117cfa06be5d92fd26b90 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 13 May 2019 16:09:11 +0100 Subject: [PATCH 06/54] Update doc strings. --- packages/coreutils/src/statedb.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/coreutils/src/statedb.ts b/packages/coreutils/src/statedb.ts index cae20e4efb9e..3c88fefa392e 100644 --- a/packages/coreutils/src/statedb.ts +++ b/packages/coreutils/src/statedb.ts @@ -364,10 +364,8 @@ namespace Private { export type Envelope = { readonly v: ReadonlyJSONValue }; /** - * The in-memory storage + * An in-memory data connector for a state database. */ - const storage: { [id: string]: string } = {}; - export const connector: IDataConnector = { /** * Retrieve an item from the data connector. @@ -408,4 +406,9 @@ namespace Private { storage[id] = value; } }; + + /** + * In-memory data storage for the data connector. + */ + const storage: { [id: string]: string } = {}; } From 3ad004a43f8e136af4330d9de42a202898574dd4 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 13 May 2019 17:37:52 +0100 Subject: [PATCH 07/54] Create StateDB.Connector class, remove namespace and windowName. --- packages/coreutils/src/statedb.ts | 111 +++++++++--------------------- 1 file changed, 31 insertions(+), 80 deletions(-) diff --git a/packages/coreutils/src/statedb.ts b/packages/coreutils/src/statedb.ts index 3c88fefa392e..df86fa1de8ba 100644 --- a/packages/coreutils/src/statedb.ts +++ b/packages/coreutils/src/statedb.ts @@ -23,16 +23,6 @@ export const IStateDB = new Token('@jupyterlab/coreutils:IStateDB'); */ export interface IStateDB extends IDataConnector { - /** - * The namespace prefix for all state database entries. - * - * #### Notes - * This value should be set at instantiation and will only be used - * internally by a state database. That means, for example, that an - * app could have multiple, mutually exclusive state databases. - */ - readonly namespace: string; - /** * Return a serialized copy of the state database's entire contents. * @@ -52,12 +42,9 @@ export class StateDB * @param options - The instantiation options for a state database. */ constructor(options: StateDB.IOptions) { - const { connector, namespace, transform, windowName } = options; - - this.namespace = namespace; + const { connector, transform } = options; - this._connector = connector || Private.connector; - this._window = windowName || ''; + this._connector = connector || new StateDB.Connector(); this._ready = (transform || Promise.resolve(null)).then(transformation => { if (!transformation) { return; @@ -87,16 +74,6 @@ export class StateDB return this._changed; } - /** - * The namespace prefix for all state database entries. - * - * #### Notes - * This value should be set at instantiation and will only be used internally - * by a state database. That means, for example, that an app could have - * multiple, mutually exclusive state databases. - */ - readonly namespace: string; - /** * Clear the entire database. */ @@ -218,8 +195,7 @@ export class StateDB * Fetch a value from the database. */ private async _fetch(id: string): Promise { - const key = `${this._window}:${this.namespace}:${id}`; - const value = await this._connector.fetch(key); + const value = await this._connector.fetch(id); if (value) { return (JSON.parse(value) as Private.Envelope).v as T; @@ -230,13 +206,10 @@ export class StateDB * Fetch a list from the database. */ private async _list(query?: string): Promise<{ ids: string[]; values: T[] }> { - const prefix = `${this._window}:${this.namespace}:`; - const { ids, values } = await this._connector.list( - query ? `${prefix}${query}:` : prefix - ); + const { ids, values } = await this._connector.list(query); return { - ids: ids.map((key: string) => key.replace(prefix, '')), + ids, values: values.map(val => (JSON.parse(val) as Private.Envelope).v as T) }; } @@ -262,25 +235,19 @@ export class StateDB * Remove a key in the database. */ private async _remove(id: string): Promise { - const key = `${this._window}:${this.namespace}:${id}`; - - return this._connector.remove(key); + return this._connector.remove(id); } /** * Save a key and its value in the database. */ private async _save(id: string, value: T): Promise { - const key = `${this._window}:${this.namespace}:${id}`; - const serialized = JSON.stringify({ v: value } as Private.Envelope); - - return this._connector.save(key, serialized); + return this._connector.save(id, JSON.stringify({ v: value })); } private _changed = new Signal(this); private _connector: IDataConnector; private _ready: Promise; - private _window: string; } /** @@ -340,75 +307,59 @@ export namespace StateDB { * client requests. */ transform?: Promise; - - /** - * An optional name for the application window. - * - * #### Notes - * In environments where multiple windows can instantiate a state database, - * a window name is necessary to prefix all keys that are stored within the - * local storage that is shared by all windows. In JupyterLab, this window - * name is generated by the `IWindowResolver` extension. - */ - windowName?: string; } -} - -/* - * A namespace for private module data. - */ -namespace Private { - /** - * An envelope around a JSON value stored in the state database. - */ - export type Envelope = { readonly v: ReadonlyJSONValue }; /** - * An in-memory data connector for a state database. + * An in-memory string key/value data connector. */ - export const connector: IDataConnector = { + export class Connector implements IDataConnector { /** * Retrieve an item from the data connector. */ - fetch: async (id: string): Promise => { - return storage[id]; - }, + async fetch(id: string): Promise { + return this._storage[id]; + } /** * Retrieve the list of items available from the data connector. */ - list: async ( - query: string - ): Promise<{ ids: string[]; values: string[] }> => { - return Object.keys(storage).reduce( + async list(query: string): Promise<{ ids: string[]; values: string[] }> { + return Object.keys(this._storage).reduce( (acc, val) => { if (val && val.indexOf(query) === 0) { acc.ids.push(val); - acc.values.push(storage[val]); + acc.values.push(this._storage[val]); } return acc; }, { ids: [], values: [] } ); - }, + } /** * Remove a value using the data connector. */ - remove: async (id: string): Promise => { - delete storage[id]; - }, + async remove(id: string): Promise { + delete this._storage[id]; + } /** * Save a value using the data connector. */ - save: async (id: string, value: string): Promise => { - storage[id] = value; + async save(id: string, value: string): Promise { + this._storage[id] = value; } - }; + private _storage: { [key: string]: string } = {}; + } +} + +/* + * A namespace for private module data. + */ +namespace Private { /** - * In-memory data storage for the data connector. + * An envelope around a JSON value stored in the state database. */ - const storage: { [id: string]: string } = {}; + export type Envelope = { readonly v: ReadonlyJSONValue }; } From ac103c4c54bf3086fd22462b09ce91a5db59fcc2 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Tue, 14 May 2019 13:07:47 +0100 Subject: [PATCH 08/54] More state database refactoring. --- packages/coreutils/src/statedb.ts | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/packages/coreutils/src/statedb.ts b/packages/coreutils/src/statedb.ts index df86fa1de8ba..bc18ffc498d4 100644 --- a/packages/coreutils/src/statedb.ts +++ b/packages/coreutils/src/statedb.ts @@ -77,13 +77,9 @@ export class StateDB /** * Clear the entire database. */ - async clear(silent = false): Promise { + async clear(): Promise { await this._ready; await this._clear(); - if (silent) { - return; - } - this._changed.emit({ id: null, type: 'clear' }); } /** @@ -170,7 +166,7 @@ export class StateDB * * @returns A promise that bears the database contents as JSON. */ - async toJSON(): Promise<{ [id: string]: T }> { + async toJSON(): Promise<{ readonly [id: string]: T }> { await this._ready; const { ids, values } = await this._list(); @@ -292,15 +288,10 @@ export namespace StateDB { */ export interface IOptions { /** - * Optional data connector for a database. Defaults to in-memory connector. + * Optional string key/value connector. Defaults to in-memory connector. */ connector?: IDataConnector; - /** - * The namespace prefix for all state database entries. - */ - namespace: string; - /** * An optional promise that resolves with a data transformation that is * applied to the database contents before the database begins resolving @@ -323,7 +314,7 @@ export namespace StateDB { /** * Retrieve the list of items available from the data connector. */ - async list(query: string): Promise<{ ids: string[]; values: string[] }> { + async list(query = ''): Promise<{ ids: string[]; values: string[] }> { return Object.keys(this._storage).reduce( (acc, val) => { if (val && val.indexOf(query) === 0) { From 5c0d826800762988960e3600c9b0afc1cd467656 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Tue, 14 May 2019 16:35:20 +0100 Subject: [PATCH 09/54] Simplify instantiation of state database. --- packages/apputils-extension/src/index.ts | 58 ++---------------------- 1 file changed, 4 insertions(+), 54 deletions(-) diff --git a/packages/apputils-extension/src/index.ts b/packages/apputils-extension/src/index.ts index d9943021f3ba..f0bd585ed081 100644 --- a/packages/apputils-extension/src/index.ts +++ b/packages/apputils-extension/src/index.ts @@ -62,8 +62,6 @@ namespace CommandIDs { export const loadState = 'apputils:load-statedb'; - export const recoverState = 'apputils:recover-statedb'; - export const reset = 'apputils:reset'; export const resetOnLoad = 'apputils:reset-on-load'; @@ -321,39 +319,7 @@ const state: JupyterFrontEndPlugin = { const { workspaces } = serviceManager; const workspace = resolver.name; const transform = new PromiseDelegate(); - const db = new StateDB({ - namespace: app.namespace, - transform: transform.promise, - windowName: workspace - }); - - commands.addCommand(CommandIDs.recoverState, { - execute: async ({ global }) => { - const immediate = true; - const silent = true; - - // Clear the state silently so that the state changed signal listener - // will not be triggered as it causes a save state. - await db.clear(silent); - - // If the user explictly chooses to recover state, all of local storage - // should be cleared. - if (global) { - try { - window.localStorage.clear(); - console.log('Cleared local storage'); - } catch (error) { - console.warn('Clearing local storage failed.', error); - - // To give the user time to see the console warning before redirect, - // do not set the `immediate` flag. - return commands.execute(CommandIDs.saveState); - } - } - - return commands.execute(CommandIDs.saveState, { immediate }); - } - }); + const db = new StateDB({ transform: transform.promise }); // Conflate all outstanding requests to the save state command that happen // within the `WORKSPACE_SAVE_DEBOUNCE_INTERVAL` into a single promise. @@ -468,13 +434,8 @@ const state: JupyterFrontEndPlugin = { commands.addCommand(CommandIDs.reset, { label: 'Reset Application State', execute: async () => { - const global = true; - - try { - await commands.execute(CommandIDs.recoverState, { global }); - } catch (error) { - /* Ignore failures and redirect. */ - } + await db.clear(); + await commands.execute(CommandIDs.saveState, { immediate: true }); router.reload(); } }); @@ -511,9 +472,7 @@ const state: JupyterFrontEndPlugin = { const silent = true; const hard = true; const url = path + URLExt.objectToQueryString(query) + hash; - const cleared = commands - .execute(CommandIDs.recoverState) - .then(() => router.stop); // Stop routing before new route navigation. + const cleared = db.clear().then(() => router.stop); // After the state has been reset, navigate to the URL. if (clone) { @@ -543,15 +502,6 @@ const state: JupyterFrontEndPlugin = { rank: 20 // High priority: 20:100. }); - // Clean up state database when the window unloads. - window.addEventListener('beforeunload', () => { - const silent = true; - - db.clear(silent).catch(() => { - /* no-op */ - }); - }); - return db; } }; From b1da94fa205b366a4fd37792c4cbba6b1e84004a Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Tue, 14 May 2019 16:39:48 +0100 Subject: [PATCH 10/54] State database options should have a default. --- packages/coreutils/src/statedb.ts | 2 +- tests/test-coreutils/src/statedb.spec.ts | 39 +++++++++++------------- 2 files changed, 18 insertions(+), 23 deletions(-) diff --git a/packages/coreutils/src/statedb.ts b/packages/coreutils/src/statedb.ts index bc18ffc498d4..8f76b6c8729d 100644 --- a/packages/coreutils/src/statedb.ts +++ b/packages/coreutils/src/statedb.ts @@ -41,7 +41,7 @@ export class StateDB * * @param options - The instantiation options for a state database. */ - constructor(options: StateDB.IOptions) { + constructor(options: StateDB.IOptions = {}) { const { connector, transform } = options; this._connector = connector || new StateDB.Connector(); diff --git a/tests/test-coreutils/src/statedb.spec.ts b/tests/test-coreutils/src/statedb.spec.ts index 1d92c9561638..251da0203efd 100644 --- a/tests/test-coreutils/src/statedb.spec.ts +++ b/tests/test-coreutils/src/statedb.spec.ts @@ -14,17 +14,14 @@ describe('StateDB', () => { describe('#constructor()', () => { it('should create a state database', () => { - const db = new StateDB({ namespace: 'test' }); + const db = new StateDB(); expect(db).to.be.an.instanceof(StateDB); }); it('should allow an overwrite data transformation', async () => { const transform = new PromiseDelegate(); - const db = new StateDB({ - namespace: 'test', - transform: transform.promise - }); - const prepopulate = new StateDB({ namespace: 'test' }); + const db = new StateDB({ transform: transform.promise }); + const prepopulate = new StateDB(); const key = 'foo'; const correct = 'bar'; const incorrect = 'baz'; @@ -46,8 +43,8 @@ describe('StateDB', () => { it('should allow a merge data transformation', async () => { let transform = new PromiseDelegate(); - let db = new StateDB({ namespace: 'test', transform: transform.promise }); - let prepopulate = new StateDB({ namespace: 'test' }); + let db = new StateDB({ transform: transform.promise }); + let prepopulate = new StateDB(); let key = 'baz'; let value = 'qux'; @@ -64,8 +61,7 @@ describe('StateDB', () => { describe('#changed', () => { it('should emit changes when the database is updated', async () => { - const namespace = 'test-namespace'; - const db = new StateDB({ namespace }); + const db = new StateDB(); const changes: StateDB.Change[] = [ { id: 'foo', type: 'save' }, { id: 'foo', type: 'remove' }, @@ -74,7 +70,7 @@ describe('StateDB', () => { ]; const recorded: StateDB.Change[] = []; - db.changed.connect((sender, change) => { + db.changed.connect((_, change) => { recorded.push(change); }); @@ -89,8 +85,7 @@ describe('StateDB', () => { describe('#namespace', () => { it('should be the read-only internal namespace', () => { - const namespace = 'test-namespace'; - const db = new StateDB({ namespace }); + const db = new StateDB(); expect(db.namespace).to.equal(namespace); }); }); @@ -99,7 +94,7 @@ describe('StateDB', () => { it('should empty the items in a state database', async () => { const { localStorage } = window; - const db = new StateDB({ namespace: 'test-namespace' }); + const db = new StateDB(); const key = 'foo:bar'; const value = { qux: 'quux' }; @@ -113,8 +108,8 @@ describe('StateDB', () => { it('should only clear its own namespace', async () => { const { localStorage } = window; - const db1 = new StateDB({ namespace: 'test-namespace-1' }); - const db2 = new StateDB({ namespace: 'test-namespace-2' }); + const db1 = new StateDB(); + const db2 = new StateDB(); expect(localStorage.length).to.equal(0); await db1.save('foo', { bar: null }); @@ -132,7 +127,7 @@ describe('StateDB', () => { it('should fetch a stored key', async () => { const { localStorage } = window; - const db = new StateDB({ namespace: 'test-namespace' }); + const db = new StateDB(); const key = 'foo:bar'; const value = { baz: 'qux' }; @@ -147,7 +142,7 @@ describe('StateDB', () => { it('should resolve a nonexistent key fetch with undefined', async () => { let { localStorage } = window; - let db = new StateDB({ namespace: 'test-namespace' }); + let db = new StateDB(); let key = 'foo:bar'; expect(localStorage.length).to.equal(0); @@ -160,7 +155,7 @@ describe('StateDB', () => { it('should fetch a stored namespace', async () => { const { localStorage } = window; - const db = new StateDB({ namespace: 'test-namespace' }); + const db = new StateDB(); const keys = [ 'foo:bar', 'foo:baz', @@ -201,7 +196,7 @@ describe('StateDB', () => { it('should remove a stored key', async () => { const { localStorage } = window; - const db = new StateDB({ namespace: 'test-namespace' }); + const db = new StateDB(); const key = 'foo:bar'; const value = { baz: 'qux' }; @@ -217,7 +212,7 @@ describe('StateDB', () => { it('should save a key and a value', async () => { const { localStorage } = window; - const db = new StateDB({ namespace: 'test-namespace' }); + const db = new StateDB(); const key = 'foo:bar'; const value = { baz: 'qux' }; @@ -234,7 +229,7 @@ describe('StateDB', () => { it('return the full contents of a state database', async () => { const { localStorage } = window; - const db = new StateDB({ namespace: 'test-namespace' }); + const db = new StateDB(); const contents: ReadonlyJSONObject = { abc: 'def', ghi: 'jkl', From e9f7c255e7eb3dbe0b8e0380a37adde59ca2923d Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 15 May 2019 12:59:00 +0100 Subject: [PATCH 11/54] Fix state database tests. --- tests/test-coreutils/src/statedb.spec.ts | 151 +++++++---------------- 1 file changed, 46 insertions(+), 105 deletions(-) diff --git a/tests/test-coreutils/src/statedb.spec.ts b/tests/test-coreutils/src/statedb.spec.ts index 251da0203efd..c121ddd9080c 100644 --- a/tests/test-coreutils/src/statedb.spec.ts +++ b/tests/test-coreutils/src/statedb.spec.ts @@ -8,10 +8,6 @@ import { StateDB } from '@jupyterlab/coreutils'; import { PromiseDelegate, ReadonlyJSONObject } from '@phosphor/coreutils'; describe('StateDB', () => { - beforeEach(() => { - window.localStorage.clear(); - }); - describe('#constructor()', () => { it('should create a state database', () => { const db = new StateDB(); @@ -19,43 +15,51 @@ describe('StateDB', () => { }); it('should allow an overwrite data transformation', async () => { - const transform = new PromiseDelegate(); - const db = new StateDB({ transform: transform.promise }); - const prepopulate = new StateDB(); + const connector = new StateDB.Connector(); const key = 'foo'; const correct = 'bar'; const incorrect = 'baz'; + + expect(await connector.fetch(key)).to.be.undefined; + await connector.save(key, `{ "v": "${incorrect}"}`); + expect(JSON.parse(await connector.fetch(key)).v).to.equal(incorrect); + + const transform = new PromiseDelegate(); + const db = new StateDB({ connector, transform: transform.promise }); const transformation: StateDB.DataTransform = { type: 'overwrite', contents: { [key]: correct } }; - // By sharing a namespace, the two databases will share data. - await prepopulate.save(key, incorrect); - let value = await prepopulate.fetch(key); - expect(value).to.equal(incorrect); transform.resolve(transformation); await transform.promise; - value = await db.fetch(key); - expect(value).to.equal(correct); - await db.clear(); + expect(await db.fetch(key)).to.equal(correct); + expect(JSON.parse(await connector.fetch(key)).v).to.equal(correct); }); it('should allow a merge data transformation', async () => { - let transform = new PromiseDelegate(); - let db = new StateDB({ transform: transform.promise }); - let prepopulate = new StateDB(); - let key = 'baz'; - let value = 'qux'; - - // By sharing a namespace, the two databases will share data. - await prepopulate.save('foo', 'bar'); - transform.resolve({ type: 'merge', contents: { [key]: value } }); - let saved = await db.fetch('foo'); - expect(saved).to.equal('bar'); - saved = await db.fetch(key); - expect(saved).to.equal(value); - await db.clear(); + const connector = new StateDB.Connector(); + const k1 = 'foo'; + const v1 = 'bar'; + const k2 = 'baz'; + const v2 = 'qux'; + + expect(await connector.fetch(k1)).to.be.undefined; + expect(await connector.fetch(k2)).to.be.undefined; + await connector.save(k1, `{ "v": "${v1}"}`); + expect(JSON.parse(await connector.fetch(k1)).v).to.equal(v1); + + const transform = new PromiseDelegate(); + const db = new StateDB({ connector, transform: transform.promise }); + const transformation: StateDB.DataTransform = { + type: 'merge', + contents: { [k2]: v2 } + }; + + transform.resolve(transformation); + await transform.promise; + expect(await db.fetch(k1)).to.equal(v1); + expect(await db.fetch(k2)).to.equal(v2); }); }); @@ -79,82 +83,36 @@ describe('StateDB', () => { await db.save('bar', 1); await db.remove('bar'); expect(recorded).to.deep.equal(changes); - await db.clear(); - }); - }); - - describe('#namespace', () => { - it('should be the read-only internal namespace', () => { - const db = new StateDB(); - expect(db.namespace).to.equal(namespace); }); }); describe('#clear()', () => { it('should empty the items in a state database', async () => { - const { localStorage } = window; + const connector = new StateDB.Connector(); + const db = new StateDB({ connector }); - const db = new StateDB(); - const key = 'foo:bar'; - const value = { qux: 'quux' }; - - expect(localStorage.length).to.equal(0); - await db.save(key, value); - expect(localStorage).to.have.length(1); + expect((await connector.list()).ids).to.be.empty; + await db.save('foo', 'bar'); + expect((await connector.list()).ids).not.to.be.empty; await db.clear(); - expect(localStorage.length).to.equal(0); - }); - - it('should only clear its own namespace', async () => { - const { localStorage } = window; - - const db1 = new StateDB(); - const db2 = new StateDB(); - - expect(localStorage.length).to.equal(0); - await db1.save('foo', { bar: null }); - expect(localStorage).to.have.length(1); - await db2.save('baz', { qux: null }); - expect(localStorage).to.have.length(2); - await db1.clear(); - expect(localStorage).to.have.length(1); - await db2.clear(); - expect(localStorage.length).to.equal(0); + expect((await connector.list()).ids).to.be.empty; }); }); describe('#fetch()', () => { it('should fetch a stored key', async () => { - const { localStorage } = window; - const db = new StateDB(); const key = 'foo:bar'; const value = { baz: 'qux' }; - expect(localStorage.length).to.equal(0); + expect(await db.fetch(key)).to.be.undefined; await db.save(key, value); - expect(localStorage).to.have.length(1); - const fetched = await db.fetch(key); - expect(fetched).to.deep.equal(value); - await db.clear(); - }); - - it('should resolve a nonexistent key fetch with undefined', async () => { - let { localStorage } = window; - - let db = new StateDB(); - let key = 'foo:bar'; - - expect(localStorage.length).to.equal(0); - const fetched = await db.fetch(key); - expect(fetched).to.be.undefined; + expect(await db.fetch(key)).to.deep.equal(value); }); }); describe('#list()', () => { it('should fetch a stored namespace', async () => { - const { localStorage } = window; - const db = new StateDB(); const keys = [ 'foo:bar', @@ -165,10 +123,8 @@ describe('StateDB', () => { 'abc:jkl' ]; - expect(localStorage.length).to.equal(0); - const promises = keys.map(key => db.save(key, { value: key })); - await Promise.all(promises); - expect(localStorage).to.have.length(keys.length); + await Promise.all(keys.map(key => db.save(key, { value: key }))); + let fetched = await db.list('foo'); expect(fetched.ids.length).to.equal(3); expect(fetched.values.length).to.equal(3); @@ -183,52 +139,39 @@ describe('StateDB', () => { expect(fetched.values.length).to.equal(3); sorted = fetched.ids.sort((a, b) => a.localeCompare(b)); - expect(sorted[0]).to.equal(keys[3]); expect(sorted[1]).to.equal(keys[4]); expect(sorted[2]).to.equal(keys[5]); - - await db.clear(); }); }); describe('#remove()', () => { it('should remove a stored key', async () => { - const { localStorage } = window; - const db = new StateDB(); const key = 'foo:bar'; const value = { baz: 'qux' }; - expect(localStorage.length).to.equal(0); + expect(await db.fetch(key)).to.be.undefined; await db.save(key, value); - expect(localStorage).to.have.length(1); + expect(await db.fetch(key)).to.deep.equal(value); await db.remove(key); - expect(localStorage.length).to.equal(0); + expect(await db.fetch(key)).to.be.undefined; }); }); describe('#save()', () => { it('should save a key and a value', async () => { - const { localStorage } = window; - const db = new StateDB(); const key = 'foo:bar'; const value = { baz: 'qux' }; - expect(localStorage.length).to.equal(0); await db.save(key, value); - const fetched = await db.fetch(key); - expect(fetched).to.deep.equal(value); - await db.remove(key); - expect(localStorage.length).to.equal(0); + expect(await db.fetch(key)).to.deep.equal(value); }); }); describe('#toJSON()', () => { it('return the full contents of a state database', async () => { - const { localStorage } = window; - const db = new StateDB(); const contents: ReadonlyJSONObject = { abc: 'def', @@ -239,13 +182,11 @@ describe('StateDB', () => { } }; - expect(localStorage.length).to.equal(0); await Promise.all( Object.keys(contents).map(key => db.save(key, contents[key])) ); const serialized = await db.toJSON(); expect(serialized).to.deep.equal(contents); - await db.clear(); }); }); }); From 6e610f0508507343c1463245b47dc2696392ad99 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 15 May 2019 13:29:43 +0100 Subject: [PATCH 12/54] Fix layout restorer tests. --- .../src/layoutrestorer.spec.ts | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/tests/test-application/src/layoutrestorer.spec.ts b/tests/test-application/src/layoutrestorer.spec.ts index 52c02536e6d1..db201bdd45db 100644 --- a/tests/test-application/src/layoutrestorer.spec.ts +++ b/tests/test-application/src/layoutrestorer.spec.ts @@ -15,8 +15,6 @@ import { PromiseDelegate } from '@phosphor/coreutils'; import { DockPanel, Widget } from '@phosphor/widgets'; -const NAMESPACE = 'jupyterlab-layout-restorer-tests'; - describe('apputils', () => { describe('LayoutRestorer', () => { describe('#constructor()', () => { @@ -24,7 +22,7 @@ describe('apputils', () => { const restorer = new LayoutRestorer({ first: Promise.resolve(void 0), registry: new CommandRegistry(), - state: new StateDB({ namespace: NAMESPACE }) + state: new StateDB() }); expect(restorer).to.be.an.instanceof(LayoutRestorer); }); @@ -35,7 +33,7 @@ describe('apputils', () => { const restorer = new LayoutRestorer({ first: Promise.resolve(void 0), registry: new CommandRegistry(), - state: new StateDB({ namespace: NAMESPACE }) + state: new StateDB() }); expect(restorer.restored).to.be.an.instanceof(Promise); }); @@ -45,7 +43,7 @@ describe('apputils', () => { const restorer = new LayoutRestorer({ first: ready.promise, registry: new CommandRegistry(), - state: new StateDB({ namespace: NAMESPACE }) + state: new StateDB() }); let promise = restorer.restored; ready.resolve(void 0); @@ -59,7 +57,7 @@ describe('apputils', () => { const restorer = new LayoutRestorer({ first: ready.promise, registry: new CommandRegistry(), - state: new StateDB({ namespace: NAMESPACE }) + state: new StateDB() }); const currentWidget = new Widget(); const mode: DockPanel.Mode = 'single-document'; @@ -83,7 +81,7 @@ describe('apputils', () => { const restorer = new LayoutRestorer({ first: Promise.resolve(void 0), registry: new CommandRegistry(), - state: new StateDB({ namespace: NAMESPACE }) + state: new StateDB() }); const layout = await restorer.fetch(); expect(layout).to.not.equal(null); @@ -94,7 +92,7 @@ describe('apputils', () => { const restorer = new LayoutRestorer({ first: ready.promise, registry: new CommandRegistry(), - state: new StateDB({ namespace: NAMESPACE }) + state: new StateDB() }); const currentWidget = new Widget(); // The `fresh` attribute is only here to check against the return value. @@ -123,7 +121,7 @@ describe('apputils', () => { namespace: 'foo-widget' }); const registry = new CommandRegistry(); - const state = new StateDB({ namespace: NAMESPACE }); + const state = new StateDB(); const ready = new PromiseDelegate(); const restorer = new LayoutRestorer({ first: ready.promise, @@ -157,7 +155,7 @@ describe('apputils', () => { // no op }), registry: new CommandRegistry(), - state: new StateDB({ namespace: NAMESPACE }) + state: new StateDB() }); const dehydrated: ILabShell.ILayout = { mainArea: { currentWidget: null, dock: null, mode: null }, @@ -176,7 +174,7 @@ describe('apputils', () => { const restorer = new LayoutRestorer({ first: ready.promise, registry: new CommandRegistry(), - state: new StateDB({ namespace: NAMESPACE }) + state: new StateDB() }); const currentWidget = new Widget(); // The `fresh` attribute is only here to check against the return value. From 7883e35d026be153eba42a83f1fb118b0541e773 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 15 May 2019 14:42:40 +0100 Subject: [PATCH 13/54] Fix setting registry test build. --- tests/test-coreutils/src/settingregistry.spec.ts | 4 ---- tests/test-filebrowser/src/model.spec.ts | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/test-coreutils/src/settingregistry.spec.ts b/tests/test-coreutils/src/settingregistry.spec.ts index db221a9f67e1..34142198ffe9 100644 --- a/tests/test-coreutils/src/settingregistry.spec.ts +++ b/tests/test-coreutils/src/settingregistry.spec.ts @@ -18,10 +18,6 @@ import { JSONObject } from '@phosphor/coreutils'; class TestConnector extends StateDB { schemas: { [key: string]: ISettingRegistry.ISchema } = {}; - constructor() { - super({ namespace: 'setting-registry-tests' }); - } - async fetch(id: string): Promise { const fetched = await super.fetch(id); if (!fetched && !this.schemas[id]) { diff --git a/tests/test-filebrowser/src/model.spec.ts b/tests/test-filebrowser/src/model.spec.ts index 0f7f60f32c20..a524ca69f544 100644 --- a/tests/test-filebrowser/src/model.spec.ts +++ b/tests/test-filebrowser/src/model.spec.ts @@ -77,7 +77,7 @@ describe('filebrowser/model', () => { opener, manager: serviceManager }); - state = new StateDB({ namespace: 'filebrowser/model' }); + state = new StateDB(); }); beforeEach(async () => { From 6a29b562f411c2d9b03efc4b44f8276793f894fa Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 15 May 2019 18:54:11 +0100 Subject: [PATCH 14/54] Add `debounce()` function. --- packages/coreutils/src/poll.ts | 41 ++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/packages/coreutils/src/poll.ts b/packages/coreutils/src/poll.ts index 61eef0689aa2..04aee862a3eb 100644 --- a/packages/coreutils/src/poll.ts +++ b/packages/coreutils/src/poll.ts @@ -234,11 +234,11 @@ export class Poll implements IDisposable, IPoll { throw new Error('Poll backoff growth factor must be at least 1'); } - if (interval < 0 || interval > max) { + if ((interval < 0 || interval > max) && interval !== Private.NEVER) { throw new Error('Poll interval must be between 0 and max'); } - if (max > Poll.MAX_INTERVAL) { + if (max > Poll.MAX_INTERVAL && max !== Private.NEVER) { throw new Error(`Max interval must be less than ${Poll.MAX_INTERVAL}`); } @@ -544,6 +544,43 @@ export namespace Poll { export const MAX_INTERVAL = 2147483647; } +class Debouncer extends Poll { + constructor(factory: Poll.Factory, public interval: number) { + super({ factory, name: 'DEBOUNCER' }); + void super.stop(); + } + + readonly frequency: IPoll.Frequency = { + backoff: false, + interval: Infinity, + max: Infinity + }; + + readonly standby = 'when-hidden'; + + async debounce(): Promise { + await this.schedule({ + cancel: last => last.phase === 'refreshed', + interval: this.interval, + phase: 'refreshed' + }); + await this.tick; + } +} + +/** + * Returns a debounced function that can be called multiple times safely and + * only executes the underlying function once per `interval`. + * + * @param fn - The function to debounce. + * + * @param interval - The debounce interval; defaults to 500ms. + */ +export function debounce(fn: () => any, interval = 500): () => Promise { + const debouncer = new Debouncer(async () => fn(), interval); + return () => debouncer.debounce(); +} + /** * A namespace for private module data. */ From d64bf6231273a9f5d1c4491be4ba0020717aba1b Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 15 May 2019 18:54:38 +0100 Subject: [PATCH 15/54] Use debounce function for saving IStateDB; remove bespoke debouncing logic. --- packages/apputils-extension/src/index.ts | 71 ++++-------------------- 1 file changed, 10 insertions(+), 61 deletions(-) diff --git a/packages/apputils-extension/src/index.ts b/packages/apputils-extension/src/index.ts index f0bd585ed081..cf12ecb35cf8 100644 --- a/packages/apputils-extension/src/index.ts +++ b/packages/apputils-extension/src/index.ts @@ -21,6 +21,7 @@ import { } from '@jupyterlab/apputils'; import { + debounce, ISettingRegistry, IStateDB, SettingRegistry, @@ -42,13 +43,6 @@ import { Palette } from './palette'; import '../style/index.css'; -/** - * The interval in milliseconds that calls to save a workspace are debounced - * to allow for multiple quickly executed state changes to result in a single - * workspace save operation. - */ -const WORKSPACE_SAVE_DEBOUNCE_INTERVAL = 750; - /** * The interval in milliseconds before recover options appear during splash. */ @@ -65,8 +59,6 @@ namespace CommandIDs { export const reset = 'apputils:reset'; export const resetOnLoad = 'apputils:reset-on-load'; - - export const saveState = 'apputils:save-statedb'; } /** @@ -312,59 +304,20 @@ const state: JupyterFrontEndPlugin = { resolver: IWindowResolver, splash: ISplashScreen | null ) => { - let debouncer: number; let resolved = false; - const { commands, serviceManager } = app; const { workspaces } = serviceManager; const workspace = resolver.name; const transform = new PromiseDelegate(); const db = new StateDB({ transform: transform.promise }); + const save = debounce(async () => { + const id = workspace; + const metadata = { id }; + const data = await db.toJSON(); - // Conflate all outstanding requests to the save state command that happen - // within the `WORKSPACE_SAVE_DEBOUNCE_INTERVAL` into a single promise. - let conflated: PromiseDelegate | null = null; - - commands.addCommand(CommandIDs.saveState, { - label: () => `Save Workspace (${workspace})`, - execute: ({ immediate }) => { - const timeout = immediate ? 0 : WORKSPACE_SAVE_DEBOUNCE_INTERVAL; - const id = workspace; - const metadata = { id }; - - // Only instantiate a new conflated promise if one is not outstanding. - if (!conflated) { - conflated = new PromiseDelegate(); - } - - if (debouncer) { - window.clearTimeout(debouncer); - } - - debouncer = window.setTimeout(async () => { - const data = await db.toJSON(); - - try { - await workspaces.save(id, { data, metadata }); - if (conflated) { - conflated.resolve(undefined); - } - } catch (error) { - if (conflated) { - conflated.reject(error); - } - } - conflated = null; - }, timeout); - - return conflated.promise; - } + return await workspaces.save(id, { data, metadata }); }); - const listener = (sender: any, change: StateDB.Change) => { - return commands.execute(CommandIDs.saveState); - }; - commands.addCommand(CommandIDs.loadState, { execute: async (args: IRouter.ILocation) => { // Since the command can be executed an arbitrary number of times, make @@ -405,18 +358,14 @@ const state: JupyterFrontEndPlugin = { } // Any time the local state database changes, save the workspace. - db.changed.connect(listener, db); - - const immediate = true; + db.changed.connect(() => void save(), db); if (source === clone) { // Maintain the query string parameters but remove `clone`. delete query['clone']; const url = path + URLExt.objectToQueryString(query) + hash; - const cloned = commands - .execute(CommandIDs.saveState, { immediate }) - .then(() => router.stop); + const cloned = save().then(() => router.stop); // After the state has been cloned, navigate to the URL. void cloned.then(() => { @@ -427,7 +376,7 @@ const state: JupyterFrontEndPlugin = { } // After the state database has finished loading, save it. - return commands.execute(CommandIDs.saveState, { immediate }); + await save(); } }); @@ -435,7 +384,7 @@ const state: JupyterFrontEndPlugin = { label: 'Reset Application State', execute: async () => { await db.clear(); - await commands.execute(CommandIDs.saveState, { immediate: true }); + await save(); router.reload(); } }); From 2fc07f6ae7296c279ea9527604bcc18ae1de079b Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 15 May 2019 18:58:35 +0100 Subject: [PATCH 16/54] Remove superfluous return. --- packages/apputils-extension/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/apputils-extension/src/index.ts b/packages/apputils-extension/src/index.ts index d458e75ddf7f..7e963b7bcd76 100644 --- a/packages/apputils-extension/src/index.ts +++ b/packages/apputils-extension/src/index.ts @@ -339,7 +339,7 @@ const state: JupyterFrontEndPlugin = { const metadata = { id }; const data = await db.toJSON(); - return await workspaces.save(id, { data, metadata }); + await workspaces.save(id, { data, metadata }); }); commands.addCommand(CommandIDs.loadState, { From aa79d265df1e2713de10c2393bd38ba1c1f9c70e Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Thu, 16 May 2019 18:34:30 +0100 Subject: [PATCH 17/54] Add doc strings and export `Debouncer`. --- packages/coreutils/src/poll.ts | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/coreutils/src/poll.ts b/packages/coreutils/src/poll.ts index abc0a312cae0..e6cc43dd0faa 100644 --- a/packages/coreutils/src/poll.ts +++ b/packages/coreutils/src/poll.ts @@ -549,20 +549,39 @@ export namespace Poll { export const MAX_INTERVAL = 2147483647; } -class Debouncer extends Poll { +/** + * A poll that only schedules ticks manually. + */ +export class Debouncer extends Poll { + /** + * Instantiate a debouncer poll. + * + * @param factory - The poll factory. + * + * @param interval - The debounce interval. + */ constructor(factory: Poll.Factory, public interval: number) { super({ factory, name: 'DEBOUNCER' }); void super.stop(); } + /** + * The debouncer frequency. + */ readonly frequency: IPoll.Frequency = { backoff: false, interval: Infinity, max: Infinity }; + /** + * The debouncer poll standby value. + */ readonly standby = 'when-hidden'; + /** + * Debounce a request to refresh the poll within the debounce interval. + */ async debounce(): Promise { await this.schedule({ cancel: last => last.phase === 'refreshed', From fd68181f2dfacbae84d00540b97b34c3e1412b75 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Fri, 17 May 2019 13:50:33 +0100 Subject: [PATCH 18/54] Allow IPoll.Phase to be extended with additional tick types. --- packages/coreutils/src/poll.ts | 98 ++++++++++++++++++++-------------- 1 file changed, 57 insertions(+), 41 deletions(-) diff --git a/packages/coreutils/src/poll.ts b/packages/coreutils/src/poll.ts index e6cc43dd0faa..fc72d1662f0b 100644 --- a/packages/coreutils/src/poll.ts +++ b/packages/coreutils/src/poll.ts @@ -11,12 +11,12 @@ import { ISignal, Signal } from '@phosphor/signaling'; * A readonly poll that calls an asynchronous function with each tick. * * @typeparam T - The resolved type of the factory's promises. - * Defaults to `any`. * * @typeparam U - The rejected type of the factory's promises. - * Defaults to `any`. + * + * @typeparam V - The type to extend the phases supported by a poll. */ -export interface IPoll { +export interface IPoll { /** * A signal emitted when the poll is disposed. */ @@ -40,7 +40,7 @@ export interface IPoll { /** * The poll state, which is the content of the currently-scheduled poll tick. */ - readonly state: IPoll.State; + readonly state: IPoll.State; /** * A promise that resolves when the currently-scheduled tick completes. @@ -50,12 +50,12 @@ export interface IPoll { * `state.timestamp`. It can resolve earlier if the user starts or refreshes the * poll, etc. */ - readonly tick: Promise>; + readonly tick: Promise>; /** * A signal emitted when the poll state changes, i.e., a new tick is scheduled. */ - readonly ticked: ISignal, IPoll.State>; + readonly ticked: ISignal, IPoll.State>; } /** @@ -99,8 +99,11 @@ export namespace IPoll { /** * The phase of the poll when the current tick was scheduled. + * + * @typeparam T - A type for any additional tick phases a poll supports. */ - export type Phase = + export type Phase = + | T | 'constructed' | 'disposed' | 'reconnected' @@ -117,12 +120,12 @@ export namespace IPoll { * Definition of poll state at any given time. * * @typeparam T - The resolved type of the factory's promises. - * Defaults to `any`. * * @typeparam U - The rejected type of the factory's promises. - * Defaults to `any`. + * + * @typeparam V - The type to extend the phases supported by a poll. */ - export type State = { + export type State = { /** * The number of milliseconds until the current tick resolves. */ @@ -140,7 +143,7 @@ export namespace IPoll { /** * The current poll phase. */ - readonly phase: Phase; + readonly phase: Phase; /** * The timestamp for when this tick was scheduled. @@ -158,14 +161,18 @@ export namespace IPoll { * * @typeparam U - The rejected type of the factory's promises. * Defaults to `any`. + * + * @typeparam V - An optional type to extend the phases supported by a poll. + * Defaults to `standby`, which already exists in the `Phase` type. */ -export class Poll implements IDisposable, IPoll { +export class Poll + implements IDisposable, IPoll { /** * Instantiate a new poll with exponential backoff in case of failure. * * @param options - The poll instantiation options. */ - constructor(options: Poll.IOptions) { + constructor(options: Poll.IOptions) { this._factory = options.factory; this._standby = options.standby || Private.DEFAULT_STANDBY; this._state = { ...Private.DEFAULT_STATE, timestamp: new Date().getTime() }; @@ -269,7 +276,7 @@ export class Poll implements IDisposable, IPoll { /** * The poll state, which is the content of the current poll tick. */ - get state(): IPoll.State { + get state(): IPoll.State { return this._state; } @@ -283,7 +290,7 @@ export class Poll implements IDisposable, IPoll { /** * A signal emitted when the poll ticks and fires off a new request. */ - get ticked(): ISignal> { + get ticked(): ISignal> { return this._ticked; } @@ -363,7 +370,9 @@ export class Poll implements IDisposable, IPoll { * schedule poll ticks. */ protected async schedule( - next: Partial boolean }> = {} + next: Partial< + IPoll.State & { cancel: (last: IPoll.State) => boolean } + > = {} ): Promise { if (this.isDisposed) { return; @@ -385,7 +394,7 @@ export class Poll implements IDisposable, IPoll { const last = this.state; const pending = this._tick; const scheduled = new PromiseDelegate(); - const state: IPoll.State = { + const state: IPoll.State = { interval: this.frequency.interval, payload: null, phase: 'standby', @@ -469,12 +478,12 @@ export class Poll implements IDisposable, IPoll { } private _disposed = new Signal(this); - private _factory: Poll.Factory; + private _factory: Poll.Factory; private _frequency: IPoll.Frequency; private _standby: Poll.Standby | (() => boolean | Poll.Standby); - private _state: IPoll.State; + private _state: IPoll.State; private _tick = new PromiseDelegate(); - private _ticked = new Signal>(this); + private _ticked = new Signal>(this); private _timeout = -1; } @@ -488,8 +497,12 @@ export namespace Poll { * @typeparam T - The resolved type of the factory's promises. * * @typeparam U - The rejected type of the factory's promises. + * + * @typeparam V - The type to extend the phases supported by a poll. */ - export type Factory = (state: IPoll.State) => Promise; + export type Factory = ( + state: IPoll.State + ) => Promise; /** * Indicates when the poll switches to standby. @@ -500,16 +513,16 @@ export namespace Poll { * Instantiation options for polls. * * @typeparam T - The resolved type of the factory's promises. - * Defaults to `any`. * * @typeparam U - The rejected type of the factory's promises. - * Defaults to `any`. + * + * @typeparam V - The type to extend the phases supported by a poll. */ - export interface IOptions { + export interface IOptions { /** * A factory function that is passed a poll tick and returns a poll promise. */ - factory: Factory; + factory: Factory; /** * The polling frequency parameters. @@ -552,41 +565,41 @@ export namespace Poll { /** * A poll that only schedules ticks manually. */ -export class Debouncer extends Poll { +export class Debouncer extends Poll { /** * Instantiate a debouncer poll. * - * @param factory - The poll factory. + * @param factory - The factory function being debounced. * * @param interval - The debounce interval. */ - constructor(factory: Poll.Factory, public interval: number) { - super({ factory, name: 'DEBOUNCER' }); + constructor( + factory: Poll.Factory, + public interval: number + ) { + super({ factory }); + this.frequency = { backoff: false, interval: Infinity, max: Infinity }; void super.stop(); } /** * The debouncer frequency. */ - readonly frequency: IPoll.Frequency = { - backoff: false, - interval: Infinity, - max: Infinity - }; + readonly frequency: IPoll.Frequency; /** * The debouncer poll standby value. */ - readonly standby = 'when-hidden'; + readonly standby = Private.DEFAULT_STANDBY; /** - * Debounce a request to refresh the poll within the debounce interval. + * Invokes the debounced function after the interval has elapsed. */ async debounce(): Promise { await this.schedule({ - cancel: last => last.phase === 'refreshed', + cancel: last => last.phase === 'debounced', interval: this.interval, - phase: 'refreshed' + phase: 'debounced' }); await this.tick; } @@ -646,7 +659,7 @@ namespace Private { /** * The first poll tick state's default values superseded in constructor. */ - export const DEFAULT_STATE: IPoll.State = { + export const DEFAULT_STATE: IPoll.State = { interval: NEVER, payload: null, phase: 'constructed', @@ -656,7 +669,7 @@ namespace Private { /** * The disposed tick state values. */ - export const DISPOSED_STATE: IPoll.State = { + export const DISPOSED_STATE: IPoll.State = { interval: NEVER, payload: null, phase: 'disposed', @@ -686,7 +699,10 @@ namespace Private { * @param frequency - The poll's base frequency. * @param last - The poll's last tick. */ - export function sleep(frequency: IPoll.Frequency, last: IPoll.State): number { + export function sleep( + frequency: IPoll.Frequency, + last: IPoll.State + ): number { const { backoff, interval, max } = frequency; const growth = backoff === true ? DEFAULT_BACKOFF : backoff === false ? 1 : backoff; From 197a0f899538b364a572aa25138985c42668db88 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Fri, 17 May 2019 13:50:54 +0100 Subject: [PATCH 19/54] Update poll tests to adhere to new types without modifying their logic. --- tests/test-coreutils/src/poll.spec.ts | 42 +++++++++++++++------------ 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/tests/test-coreutils/src/poll.spec.ts b/tests/test-coreutils/src/poll.spec.ts index ff788f0a7396..e8eb6a2ca29e 100644 --- a/tests/test-coreutils/src/poll.spec.ts +++ b/tests/test-coreutils/src/poll.spec.ts @@ -7,10 +7,16 @@ import { IPoll, Poll } from '@jupyterlab/coreutils'; import { sleep } from '@jupyterlab/testutils'; -class TestPoll extends Poll { - schedule( - next: Partial boolean }> = {} - ): Promise { +class TestPoll extends Poll< + T, + U, + V +> { + async schedule( + next: Partial< + IPoll.State & { cancel: (last: IPoll.State) => boolean } + > = {} + ) { return super.schedule(next); } } @@ -208,8 +214,8 @@ describe('Poll', () => { name: '@jupyterlab/test-coreutils:Poll#tick-1' }); const expected = 'when-resolved resolved'; - const ticker: IPoll.Phase[] = []; - const tock = (poll: IPoll) => { + const ticker: IPoll.Phase[] = []; + const tock = (poll: Poll) => { ticker.push(poll.state.phase); poll.tick.then(tock).catch(() => undefined); }; @@ -225,13 +231,13 @@ describe('Poll', () => { frequency: { interval: 0, backoff: false }, name: '@jupyterlab/test-coreutils:Poll#tick-2' }); - const ticker: IPoll.Phase[] = []; - const tocker: IPoll.Phase[] = []; + const ticker: IPoll.Phase[] = []; + const tocker: IPoll.Phase[] = []; poll.ticked.connect(async (_, state) => { ticker.push(state.phase); expect(ticker.length).to.equal(tocker.length + 1); }); - const tock = async (poll: IPoll) => { + const tock = async (poll: Poll) => { tocker.push(poll.state.phase); expect(ticker.join(' ')).to.equal(tocker.join(' ')); poll.tick.then(tock).catch(() => undefined); @@ -266,7 +272,7 @@ describe('Poll', () => { it('should emit when the poll ticks after `when` resolves', async () => { const expected = 'when-resolved resolved'; - const ticker: IPoll.Phase[] = []; + const ticker: IPoll.Phase[] = []; poll = new Poll({ factory: () => Promise.resolve(), frequency: { interval: 2000, backoff: false }, @@ -281,7 +287,7 @@ describe('Poll', () => { it('should emit when the poll ticks after `when` rejects', async () => { const expected = 'when-rejected resolved'; - const ticker: IPoll.Phase[] = []; + const ticker: IPoll.Phase[] = []; const promise = Promise.reject(); poll = new Poll({ factory: () => Promise.resolve(), @@ -319,7 +325,7 @@ describe('Poll', () => { it('should dispose the poll and be safe to call repeatedly', async () => { let rejected = false; - let tick: Promise; + let tick: Promise; poll = new Poll({ name: '@jupyterlab/test-coreutils:Poll#dispose()-1', factory: () => Promise.resolve() @@ -347,7 +353,7 @@ describe('Poll', () => { it('should refresh the poll when the poll is ready', async () => { const expected = 'when-resolved refreshed resolved'; - const ticker: IPoll.Phase[] = []; + const ticker: IPoll.Phase[] = []; poll = new Poll({ name: '@jupyterlab/test-coreutils:Poll#refresh()-1', frequency: { interval: 100 }, @@ -366,7 +372,7 @@ describe('Poll', () => { it('should be safe to call multiple times', async () => { const expected = 'when-resolved refreshed resolved'; - const ticker: IPoll.Phase[] = []; + const ticker: IPoll.Phase[] = []; poll = new Poll({ name: '@jupyterlab/test-coreutils:Poll#refresh()-2', frequency: { interval: 100 }, @@ -399,7 +405,7 @@ describe('Poll', () => { it('should start the poll if it is stopped', async () => { const expected = 'when-resolved stopped started resolved'; - const ticker: IPoll.Phase[] = []; + const ticker: IPoll.Phase[] = []; poll = new Poll({ name: '@jupyterlab/test-coreutils:Poll#start()-1', frequency: { interval: 100 }, @@ -421,7 +427,7 @@ describe('Poll', () => { it('be safe to call multiple times and no-op if unnecessary', async () => { const expected = 'when-resolved resolved stopped started resolved'; - const ticker: IPoll.Phase[] = []; + const ticker: IPoll.Phase[] = []; poll = new Poll({ name: '@jupyterlab/test-coreutils:Poll#start()-2', frequency: { interval: 100 }, @@ -458,7 +464,7 @@ describe('Poll', () => { it('should stop the poll if it is active', async () => { const expected = 'when-resolved stopped started resolved'; - const ticker: IPoll.Phase[] = []; + const ticker: IPoll.Phase[] = []; poll = new Poll({ name: '@jupyterlab/test-coreutils:Poll#stop()-1', frequency: { interval: 100 }, @@ -479,7 +485,7 @@ describe('Poll', () => { it('be safe to call multiple times', async () => { const expected = 'when-resolved stopped started resolved'; - const ticker: IPoll.Phase[] = []; + const ticker: IPoll.Phase[] = []; poll = new Poll({ name: '@jupyterlab/test-coreutils:Poll#stop()-2', frequency: { interval: 100 }, From b0816dd34343a24396aa33fa1bbdc834be0704dd Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Fri, 17 May 2019 15:52:10 +0100 Subject: [PATCH 20/54] Make sure terminal ready rejection is caught at least once. --- packages/services/src/terminal/manager.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/services/src/terminal/manager.ts b/packages/services/src/terminal/manager.ts index 06057e5f8898..a00b32c6fe9f 100644 --- a/packages/services/src/terminal/manager.ts +++ b/packages/services/src/terminal/manager.ts @@ -27,6 +27,7 @@ export class TerminalManager implements TerminalSession.IManager { // Check if terminals are available if (!TerminalSession.isAvailable()) { this._ready = Promise.reject('Terminals unavailable'); + this._ready.catch(_ => undefined); return; } From 537a13da34797e87a2950276778ce079e1dc46dc Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Fri, 17 May 2019 15:52:27 +0100 Subject: [PATCH 21/54] Use better source map for filebrowser example. --- examples/filebrowser/webpack.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/filebrowser/webpack.config.js b/examples/filebrowser/webpack.config.js index 339ca2aa9fd5..741af6a5b988 100644 --- a/examples/filebrowser/webpack.config.js +++ b/examples/filebrowser/webpack.config.js @@ -6,7 +6,7 @@ module.exports = { publicPath: './example/' }, bail: true, - devtool: 'cheap-source-map', + devtool: 'source-map', mode: 'production', module: { rules: [ From ef5c19eb6206fabcad2ac3bc584f03708de96161 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Sat, 18 May 2019 15:25:09 +0100 Subject: [PATCH 22/54] Create standalone debounce() function. --- packages/coreutils/src/debounce.ts | 24 +++++++++++++ packages/coreutils/src/index.ts | 1 + packages/coreutils/src/poll.ts | 56 ------------------------------ 3 files changed, 25 insertions(+), 56 deletions(-) create mode 100644 packages/coreutils/src/debounce.ts diff --git a/packages/coreutils/src/debounce.ts b/packages/coreutils/src/debounce.ts new file mode 100644 index 000000000000..35d4ed54cdc8 --- /dev/null +++ b/packages/coreutils/src/debounce.ts @@ -0,0 +1,24 @@ +import { PromiseDelegate } from '@phosphor/coreutils'; + +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. + +/** + * Returns a debounced function that can be called multiple times safely and + * only executes the underlying function once per `interval`. + * + * @param fn - The function to debounce. + * + * @param interval - The debounce interval; defaults to 500ms. + */ +export function debounce(fn: () => any, interval = 500): () => Promise { + let debouncer = 0; + return () => { + const delegate = new PromiseDelegate(); + clearTimeout(debouncer); + debouncer = setTimeout(async () => { + delegate.resolve(void (await fn())); + }, interval); + return delegate.promise; + }; +} diff --git a/packages/coreutils/src/index.ts b/packages/coreutils/src/index.ts index 7b52f64f5f9a..e187a8545702 100644 --- a/packages/coreutils/src/index.ts +++ b/packages/coreutils/src/index.ts @@ -3,6 +3,7 @@ export * from './activitymonitor'; export * from './dataconnector'; +export * from './debounce'; export * from './interfaces'; export * from './markdowncodeblocks'; export * from './nbformat'; diff --git a/packages/coreutils/src/poll.ts b/packages/coreutils/src/poll.ts index fc72d1662f0b..f7f7c3f80dab 100644 --- a/packages/coreutils/src/poll.ts +++ b/packages/coreutils/src/poll.ts @@ -562,62 +562,6 @@ export namespace Poll { export const MAX_INTERVAL = 2147483647; } -/** - * A poll that only schedules ticks manually. - */ -export class Debouncer extends Poll { - /** - * Instantiate a debouncer poll. - * - * @param factory - The factory function being debounced. - * - * @param interval - The debounce interval. - */ - constructor( - factory: Poll.Factory, - public interval: number - ) { - super({ factory }); - this.frequency = { backoff: false, interval: Infinity, max: Infinity }; - void super.stop(); - } - - /** - * The debouncer frequency. - */ - readonly frequency: IPoll.Frequency; - - /** - * The debouncer poll standby value. - */ - readonly standby = Private.DEFAULT_STANDBY; - - /** - * Invokes the debounced function after the interval has elapsed. - */ - async debounce(): Promise { - await this.schedule({ - cancel: last => last.phase === 'debounced', - interval: this.interval, - phase: 'debounced' - }); - await this.tick; - } -} - -/** - * Returns a debounced function that can be called multiple times safely and - * only executes the underlying function once per `interval`. - * - * @param fn - The function to debounce. - * - * @param interval - The debounce interval; defaults to 500ms. - */ -export function debounce(fn: () => any, interval = 500): () => Promise { - const debouncer = new Debouncer(async () => fn(), interval); - return () => debouncer.debounce(); -} - /** * A namespace for private module data. */ From 9570ca9a23fd0a214e85a17a58aefe97c59a88b2 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Sat, 18 May 2019 18:02:23 +0100 Subject: [PATCH 23/54] Resove the debounce function with the same return type as the function being debounced. --- packages/coreutils/src/debounce.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/coreutils/src/debounce.ts b/packages/coreutils/src/debounce.ts index 35d4ed54cdc8..3fdae6dd9ad0 100644 --- a/packages/coreutils/src/debounce.ts +++ b/packages/coreutils/src/debounce.ts @@ -11,13 +11,16 @@ import { PromiseDelegate } from '@phosphor/coreutils'; * * @param interval - The debounce interval; defaults to 500ms. */ -export function debounce(fn: () => any, interval = 500): () => Promise { +export function debounce( + fn: () => T | Promise, + interval = 500 +): () => Promise { let debouncer = 0; return () => { - const delegate = new PromiseDelegate(); + const delegate = new PromiseDelegate(); clearTimeout(debouncer); debouncer = setTimeout(async () => { - delegate.resolve(void (await fn())); + delegate.resolve(await fn()); }, interval); return delegate.promise; }; From eda0d6e885238994c3ce5598cccdbb0589dbcdc5 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Sat, 18 May 2019 18:23:11 +0100 Subject: [PATCH 24/54] Update state database docstrings. --- packages/coreutils/src/statedb.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/coreutils/src/statedb.ts b/packages/coreutils/src/statedb.ts index 8f76b6c8729d..cc351269083b 100644 --- a/packages/coreutils/src/statedb.ts +++ b/packages/coreutils/src/statedb.ts @@ -19,14 +19,15 @@ export const IStateDB = new Token('@jupyterlab/coreutils:IStateDB'); /* tslint:enable */ /** - * The description of a state database. + * A state database for saving small application state data that persists + * between user sessions. */ export interface IStateDB extends IDataConnector { /** * Return a serialized copy of the state database's entire contents. * - * @returns A promise that bears the database contents as JSON. + * @returns A promise that resolves with the database contents as JSON. */ toJSON(): Promise<{ [id: string]: T }>; } @@ -164,7 +165,7 @@ export class StateDB /** * Return a serialized copy of the state database's entire contents. * - * @returns A promise that bears the database contents as JSON. + * @returns A promise that resolves with the database contents as JSON. */ async toJSON(): Promise<{ readonly [id: string]: T }> { await this._ready; From 4090d91d654c9a78c5c400fd072840458553df73 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Sun, 19 May 2019 16:24:45 +0100 Subject: [PATCH 25/54] Fix debounce function to always return a viable promise. --- packages/coreutils/src/debounce.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/coreutils/src/debounce.ts b/packages/coreutils/src/debounce.ts index 3fdae6dd9ad0..2b45de941433 100644 --- a/packages/coreutils/src/debounce.ts +++ b/packages/coreutils/src/debounce.ts @@ -15,12 +15,16 @@ export function debounce( fn: () => T | Promise, interval = 500 ): () => Promise { - let debouncer = 0; + let delegate: PromiseDelegate | null = null; + return () => { - const delegate = new PromiseDelegate(); - clearTimeout(debouncer); - debouncer = setTimeout(async () => { + if (delegate) { + return delegate.promise; + } + delegate = new PromiseDelegate(); + setTimeout(async () => { delegate.resolve(await fn()); + delegate = null; }, interval); return delegate.promise; }; From 2a8dcf4b3138c6965d8b7653dc4602192656b1c0 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Sun, 19 May 2019 16:24:54 +0100 Subject: [PATCH 26/54] Test debounce function. --- tests/test-coreutils/src/debounce.spec.ts | 35 +++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 tests/test-coreutils/src/debounce.spec.ts diff --git a/tests/test-coreutils/src/debounce.spec.ts b/tests/test-coreutils/src/debounce.spec.ts new file mode 100644 index 000000000000..c8f03a779ae2 --- /dev/null +++ b/tests/test-coreutils/src/debounce.spec.ts @@ -0,0 +1,35 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. + +import { expect } from 'chai'; + +import { debounce } from '@jupyterlab/coreutils'; + +describe('debounce()', () => { + it('should debounce a function within an interval', async () => { + let called = 0; + const interval = 500; + const debounced = debounce(async () => { + called += 1; + }, interval); + const one = debounced(); + const two = debounced(); + const three = debounced(); + const four = debounced(); + const five = debounced(); + await five; + expect(called).to.equal(1); + await four; + expect(called).to.equal(1); + await three; + expect(called).to.equal(1); + await two; + expect(called).to.equal(1); + await one; + expect(called).to.equal(1); + await debounced(); + expect(called).to.equal(2); + await debounced(); + expect(called).to.equal(3); + }); +}); From 19352258fde70d571734a41217e863189be97560 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Sun, 19 May 2019 16:38:50 +0100 Subject: [PATCH 27/54] Fix file header. --- packages/coreutils/src/debounce.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/coreutils/src/debounce.ts b/packages/coreutils/src/debounce.ts index 2b45de941433..bf3969b2122e 100644 --- a/packages/coreutils/src/debounce.ts +++ b/packages/coreutils/src/debounce.ts @@ -1,8 +1,8 @@ -import { PromiseDelegate } from '@phosphor/coreutils'; - // Copyright (c) Jupyter Development Team. // Distributed under the terms of the Modified BSD License. +import { PromiseDelegate } from '@phosphor/coreutils'; + /** * Returns a debounced function that can be called multiple times safely and * only executes the underlying function once per `interval`. From d65e5e3bc10e2f9d18d065ae9e8163b25484359f Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 20 May 2019 17:14:34 +0100 Subject: [PATCH 28/54] Make `NEVER` and `IMMEDIATE` Poll class statics. --- packages/coreutils/src/poll.ts | 43 +++++++++++++++++----------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/packages/coreutils/src/poll.ts b/packages/coreutils/src/poll.ts index f7f7c3f80dab..d06546d8b73e 100644 --- a/packages/coreutils/src/poll.ts +++ b/packages/coreutils/src/poll.ts @@ -191,7 +191,7 @@ export class Poll } return this.schedule({ - interval: Private.IMMEDIATE, + interval: Poll.IMMEDIATE, phase: 'when-resolved' }); }) @@ -203,7 +203,7 @@ export class Poll console.warn(`Poll (${this.name}) started despite rejection.`, reason); return this.schedule({ - interval: Private.IMMEDIATE, + interval: Poll.IMMEDIATE, phase: 'when-rejected' }); }); @@ -241,11 +241,11 @@ export class Poll throw new Error('Poll backoff growth factor must be at least 1'); } - if ((interval < 0 || interval > max) && interval !== Private.NEVER) { + if ((interval < 0 || interval > max) && interval !== Poll.NEVER) { throw new Error('Poll interval must be between 0 and max'); } - if (max > Poll.MAX_INTERVAL && max !== Private.NEVER) { + if (max > Poll.MAX_INTERVAL && max !== Poll.NEVER) { throw new Error(`Max interval must be less than ${Poll.MAX_INTERVAL}`); } @@ -325,7 +325,7 @@ export class Poll refresh(): Promise { return this.schedule({ cancel: last => last.phase === 'refreshed', - interval: Private.IMMEDIATE, + interval: Poll.IMMEDIATE, phase: 'refreshed' }); } @@ -338,7 +338,7 @@ export class Poll start(): Promise { return this.schedule({ cancel: last => last.phase !== 'standby' && last.phase !== 'stopped', - interval: Private.IMMEDIATE, + interval: Poll.IMMEDIATE, phase: 'started' }); } @@ -351,7 +351,7 @@ export class Poll stop(): Promise { return this.schedule({ cancel: last => last.phase === 'stopped', - interval: Private.NEVER, + interval: Poll.NEVER, phase: 'stopped' }); } @@ -405,7 +405,7 @@ export class Poll this._tick = scheduled; // Clear the schedule if possible. - if (last.interval === Private.IMMEDIATE) { + if (last.interval === Poll.IMMEDIATE) { cancelAnimationFrame(this._timeout); } else { clearTimeout(this._timeout); @@ -425,9 +425,9 @@ export class Poll this._execute(); }; this._timeout = - state.interval === Private.IMMEDIATE + state.interval === Poll.IMMEDIATE ? requestAnimationFrame(execute) - : state.interval === Private.NEVER + : state.interval === Poll.NEVER ? -1 : setTimeout(execute, state.interval); } @@ -552,6 +552,10 @@ export namespace Poll { */ when?: Promise; } + /** + * An interval value that indicates the poll should tick immediately. + */ + export const IMMEDIATE = 0; /** * Delays are 32-bit integers in many browsers so intervals need to be capped. @@ -560,22 +564,17 @@ export namespace Poll { * https://developer.mozilla.org/en-US/docs/Web/API/WindowOrWorkerGlobalScope/setTimeout#Maximum_delay_value */ export const MAX_INTERVAL = 2147483647; -} - -/** - * A namespace for private module data. - */ -namespace Private { - /** - * An interval value that indicates the poll should tick immediately. - */ - export const IMMEDIATE = 0; /** * An interval value that indicates the poll should never tick. */ export const NEVER = Infinity; +} +/** + * A namespace for private module data. + */ +namespace Private { /** * The default backoff growth rate if `backoff` is `true`. */ @@ -604,7 +603,7 @@ namespace Private { * The first poll tick state's default values superseded in constructor. */ export const DEFAULT_STATE: IPoll.State = { - interval: NEVER, + interval: Poll.NEVER, payload: null, phase: 'constructed', timestamp: new Date(0).getTime() @@ -614,7 +613,7 @@ namespace Private { * The disposed tick state values. */ export const DISPOSED_STATE: IPoll.State = { - interval: NEVER, + interval: Poll.NEVER, payload: null, phase: 'disposed', timestamp: new Date(0).getTime() From 015285bebd7137c39075ac2d9f41347b47c3715f Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 20 May 2019 17:15:11 +0100 Subject: [PATCH 29/54] Switch back to a Poll sub-class to support both `throttle()` and `debounce()`. --- packages/coreutils/src/debounce.ts | 31 ------------- packages/coreutils/src/index.ts | 2 +- packages/coreutils/src/regulator.ts | 68 +++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 32 deletions(-) delete mode 100644 packages/coreutils/src/debounce.ts create mode 100644 packages/coreutils/src/regulator.ts diff --git a/packages/coreutils/src/debounce.ts b/packages/coreutils/src/debounce.ts deleted file mode 100644 index bf3969b2122e..000000000000 --- a/packages/coreutils/src/debounce.ts +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright (c) Jupyter Development Team. -// Distributed under the terms of the Modified BSD License. - -import { PromiseDelegate } from '@phosphor/coreutils'; - -/** - * Returns a debounced function that can be called multiple times safely and - * only executes the underlying function once per `interval`. - * - * @param fn - The function to debounce. - * - * @param interval - The debounce interval; defaults to 500ms. - */ -export function debounce( - fn: () => T | Promise, - interval = 500 -): () => Promise { - let delegate: PromiseDelegate | null = null; - - return () => { - if (delegate) { - return delegate.promise; - } - delegate = new PromiseDelegate(); - setTimeout(async () => { - delegate.resolve(await fn()); - delegate = null; - }, interval); - return delegate.promise; - }; -} diff --git a/packages/coreutils/src/index.ts b/packages/coreutils/src/index.ts index e187a8545702..795f71356979 100644 --- a/packages/coreutils/src/index.ts +++ b/packages/coreutils/src/index.ts @@ -3,13 +3,13 @@ export * from './activitymonitor'; export * from './dataconnector'; -export * from './debounce'; export * from './interfaces'; export * from './markdowncodeblocks'; export * from './nbformat'; export * from './pageconfig'; export * from './path'; export * from './poll'; +export * from './regulator'; export * from './settingregistry'; export * from './statedb'; export * from './text'; diff --git a/packages/coreutils/src/regulator.ts b/packages/coreutils/src/regulator.ts new file mode 100644 index 000000000000..32046752b6ae --- /dev/null +++ b/packages/coreutils/src/regulator.ts @@ -0,0 +1,68 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. + +import { Poll } from './poll'; + +interface IRegulator { + invoke(): Promise; + stop(): Promise; +} + +export type Debounced = IRegulator; + +export type Throttled = IRegulator; + +class Regulator extends Poll { + constructor(options: { + factory: Poll.Factory; + constraints: { interval: number; strategy: 'debounce' | 'throttle' }; + }) { + super({ factory: options.factory }); + this.constraints = options.constraints; + this.frequency = { backoff: false, interval: Poll.NEVER, max: Poll.NEVER }; + this.standby = 'never'; + void super.stop(); + } + + readonly constraints: { interval: number; strategy: 'debounce' | 'throttle' }; + + async invoke(): Promise { + const { interval, strategy } = this.constraints; + if (strategy === 'debounce') { + return this.schedule({ interval, phase: 'invoked' }); + } + if (this.state.phase !== 'invoked') { + return this.schedule({ interval, phase: 'invoked' }); + } + } +} + +/** + * Returns a debounced function that can be called multiple times and only + * executes the underlying function one `interval` after the last invocation. + * + * @param fn - The function to debounce. + * + * @param interval - The debounce interval; defaults to 500ms. + */ +export function debounce(fn: () => any, interval = 500): Debounced { + return new Regulator({ + factory: async () => await fn(), + constraints: { interval, strategy: 'debounce' } + }); +} + +/** + * Returns a throttled function that can be called multiple times and only + * executes the underlying function once per `interval`. + * + * @param fn - The function to throttle. + * + * @param interval - The throttle interval; defaults to 500ms. + */ +export function throttle(fn: () => any, interval = 500): Throttled { + return new Regulator({ + factory: async () => await fn(), + constraints: { interval, strategy: 'throttle' } + }); +} From 31d0486ea0cf5889d7b6f5f0f041cd9509ba09b1 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 20 May 2019 17:16:20 +0100 Subject: [PATCH 30/54] Throttle splash screen recover and debounce workspace saving. --- packages/apputils-extension/src/index.ts | 249 +++++++++-------------- 1 file changed, 94 insertions(+), 155 deletions(-) diff --git a/packages/apputils-extension/src/index.ts b/packages/apputils-extension/src/index.ts index 7e963b7bcd76..317b7f9996ba 100644 --- a/packages/apputils-extension/src/index.ts +++ b/packages/apputils-extension/src/index.ts @@ -27,16 +27,16 @@ import { IStateDB, SettingRegistry, StateDB, + throttle, + Throttled, URLExt } from '@jupyterlab/coreutils'; import { IMainMenu } from '@jupyterlab/mainmenu'; -import { CommandRegistry } from '@phosphor/commands'; - import { PromiseDelegate } from '@phosphor/coreutils'; -import { DisposableDelegate, IDisposable } from '@phosphor/disposable'; +import { DisposableDelegate } from '@phosphor/disposable'; import { Menu } from '@phosphor/widgets'; @@ -281,11 +281,95 @@ const splash: JupyterFrontEndPlugin = { autoStart: true, provides: ISplashScreen, activate: app => { + const { commands, restored } = app; + + // Create splash element and populate it. + const splash = document.createElement('div'); + const galaxy = document.createElement('div'); + const logo = document.createElement('div'); + + splash.id = 'jupyterlab-splash'; + galaxy.id = 'galaxy'; + logo.id = 'main-logo'; + + galaxy.appendChild(logo); + ['1', '2', '3'].forEach(id => { + const moon = document.createElement('div'); + const planet = document.createElement('div'); + + moon.id = `moon${id}`; + moon.className = 'moon orbit'; + planet.id = `planet${id}`; + planet.className = 'planet'; + + moon.appendChild(planet); + galaxy.appendChild(moon); + }); + + splash.appendChild(galaxy); + + // Create debounced recovery dialog function. + let dialog: Dialog; + const recovery: Throttled = throttle(async () => { + if (dialog) { + return; + } + + dialog = new Dialog({ + title: 'Loading...', + body: `The loading screen is taking a long time. + Would you like to clear the workspace or keep waiting?`, + buttons: [ + Dialog.cancelButton({ label: 'Keep Waiting' }), + Dialog.warnButton({ label: 'Clear Workspace' }) + ] + }); + + try { + const result = await dialog.launch(); + dialog.dispose(); + dialog = null; + if (result.button.accept && commands.hasCommand(CommandIDs.reset)) { + return commands.execute(CommandIDs.reset); + } + + // Re-invoke the recovery timer in the next frame. + requestAnimationFrame(() => { + void recovery.invoke(); + }); + } catch (error) { + /* no-op */ + } + }, SPLASH_RECOVER_TIMEOUT); + + // Return ISplashScreen. + let splashCount = 0; return { show: (light = true) => { - const { commands, restored } = app; - - return Private.showSplash(restored, commands, CommandIDs.reset, light); + splash.classList.remove('splash-fade'); + splash.classList.toggle('light', light); + splash.classList.toggle('dark', !light); + splashCount++; + document.body.appendChild(splash); + + void recovery.invoke(); + + return new DisposableDelegate(async () => { + await restored; + if (--splashCount === 0) { + void recovery.stop(); + + if (dialog) { + dialog.dispose(); + dialog = null; + } + + splash.classList.add('splash-fade'); + window.setTimeout(() => { + document.body.removeChild(splash); + }, 200); + } + }); } }; } @@ -382,14 +466,14 @@ const state: JupyterFrontEndPlugin = { } // Any time the local state database changes, save the workspace. - db.changed.connect(() => void save(), db); + db.changed.connect(() => void save.invoke(), db); if (source === clone) { // Maintain the query string parameters but remove `clone`. delete query['clone']; const url = path + URLExt.objectToQueryString(query) + hash; - const cloned = save().then(() => router.stop); + const cloned = save.invoke().then(() => router.stop); // After the state has been cloned, navigate to the URL. void cloned.then(() => { @@ -400,7 +484,7 @@ const state: JupyterFrontEndPlugin = { } // After the state database has finished loading, save it. - await save(); + await save.invoke(); } }); @@ -408,7 +492,7 @@ const state: JupyterFrontEndPlugin = { label: 'Reset Application State', execute: async () => { await db.clear(); - await save(); + await save.invoke(); router.reload(); } }); @@ -514,149 +598,4 @@ namespace Private { ? URLExt.join(paths.urls.base, paths.urls.workspaces, workspace) : paths.urls.defaultWorkspace; } - - /** - * Create a splash element. - */ - function createSplash(): HTMLElement { - const splash = document.createElement('div'); - const galaxy = document.createElement('div'); - const logo = document.createElement('div'); - - splash.id = 'jupyterlab-splash'; - galaxy.id = 'galaxy'; - logo.id = 'main-logo'; - - galaxy.appendChild(logo); - ['1', '2', '3'].forEach(id => { - const moon = document.createElement('div'); - const planet = document.createElement('div'); - - moon.id = `moon${id}`; - moon.className = 'moon orbit'; - planet.id = `planet${id}`; - planet.className = 'planet'; - - moon.appendChild(planet); - galaxy.appendChild(moon); - }); - - splash.appendChild(galaxy); - - return splash; - } - - /** - * A debouncer for recovery attempts. - */ - let debouncer = 0; - - /** - * The recovery dialog. - */ - let dialog: Dialog; - - /** - * Allows the user to clear state if splash screen takes too long. - */ - function recover(fn: () => void): void { - if (dialog) { - return; - } - - dialog = new Dialog({ - title: 'Loading...', - body: `The loading screen is taking a long time. - Would you like to clear the workspace or keep waiting?`, - buttons: [ - Dialog.cancelButton({ label: 'Keep Waiting' }), - Dialog.warnButton({ label: 'Clear Workspace' }) - ] - }); - - dialog - .launch() - .then(result => { - if (result.button.accept) { - return fn(); - } - - dialog.dispose(); - dialog = null; - - debouncer = window.setTimeout(() => { - recover(fn); - }, SPLASH_RECOVER_TIMEOUT); - }) - .catch(() => { - /* no-op */ - }); - } - - /** - * The splash element. - */ - const splash = createSplash(); - - /** - * The splash screen counter. - */ - let splashCount = 0; - - /** - * Show the splash element. - * - * @param ready - A promise that must be resolved before splash disappears. - * - * @param commands - The application's command registry. - * - * @param recovery - A command that recovers from a hanging splash. - * - * @param light - A flag indicating whether the theme is light or dark. - */ - export function showSplash( - ready: Promise, - commands: CommandRegistry, - recovery: string, - light: boolean - ): IDisposable { - splash.classList.remove('splash-fade'); - splash.classList.toggle('light', light); - splash.classList.toggle('dark', !light); - splashCount++; - - if (debouncer) { - window.clearTimeout(debouncer); - } - debouncer = window.setTimeout(() => { - if (commands.hasCommand(recovery)) { - recover(() => { - return commands.execute(recovery); - }); - } - }, SPLASH_RECOVER_TIMEOUT); - - document.body.appendChild(splash); - - return new DisposableDelegate(() => { - void ready.then(() => { - if (--splashCount === 0) { - if (debouncer) { - window.clearTimeout(debouncer); - debouncer = 0; - } - - if (dialog) { - dialog.dispose(); - dialog = null; - } - - splash.classList.add('splash-fade'); - window.setTimeout(() => { - document.body.removeChild(splash); - }, 500); - } - }); - }); - } } From 93b84267f4e03760a8ee8f5d1afa95f606faf4e4 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Mon, 20 May 2019 17:51:30 +0100 Subject: [PATCH 31/54] Update debounce test. --- .../src/{debounce.spec.ts => regulator.spec.ts} | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) rename tests/test-coreutils/src/{debounce.spec.ts => regulator.spec.ts} (74%) diff --git a/tests/test-coreutils/src/debounce.spec.ts b/tests/test-coreutils/src/regulator.spec.ts similarity index 74% rename from tests/test-coreutils/src/debounce.spec.ts rename to tests/test-coreutils/src/regulator.spec.ts index c8f03a779ae2..995c1f87ba8d 100644 --- a/tests/test-coreutils/src/debounce.spec.ts +++ b/tests/test-coreutils/src/regulator.spec.ts @@ -12,11 +12,11 @@ describe('debounce()', () => { const debounced = debounce(async () => { called += 1; }, interval); - const one = debounced(); - const two = debounced(); - const three = debounced(); - const four = debounced(); - const five = debounced(); + const one = debounced.invoke(); + const two = debounced.invoke(); + const three = debounced.invoke(); + const four = debounced.invoke(); + const five = debounced.invoke(); await five; expect(called).to.equal(1); await four; @@ -27,9 +27,9 @@ describe('debounce()', () => { expect(called).to.equal(1); await one; expect(called).to.equal(1); - await debounced(); + await debounced.invoke(); expect(called).to.equal(2); - await debounced(); + await debounced.invoke(); expect(called).to.equal(3); }); }); From 288a3ac109a1f5f39877dba0c7a680884f6dfa0c Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Tue, 21 May 2019 15:44:47 +0100 Subject: [PATCH 32/54] Add a rate limiter interface. --- packages/coreutils/src/interfaces.ts | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/coreutils/src/interfaces.ts b/packages/coreutils/src/interfaces.ts index 08250dbb579d..003d5a0a78ca 100644 --- a/packages/coreutils/src/interfaces.ts +++ b/packages/coreutils/src/interfaces.ts @@ -95,3 +95,24 @@ export interface IDataConnector { */ save(id: V, value: U): Promise; } + +/** + * A function whose invocations are rate limited and can be stopped after + * invocation before it has fired. + */ +export interface IRateLimiter { + /** + * The rate limit in milliseconds. + */ + readonly limit: number; + + /** + * Invoke the rate limited function. + */ + invoke(): Promise; + + /** + * Stop the function if it is mid-flight. + */ + stop(): Promise; +} From cdc37224c7ab33a68b630a0427bf360cce4bc817 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Tue, 21 May 2019 15:45:16 +0100 Subject: [PATCH 33/54] Make Poll#schedule public so polls can be composed. --- packages/coreutils/src/poll.ts | 58 +++++++++++++++++----------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/packages/coreutils/src/poll.ts b/packages/coreutils/src/poll.ts index d06546d8b73e..17bd17ebf7fe 100644 --- a/packages/coreutils/src/poll.ts +++ b/packages/coreutils/src/poll.ts @@ -330,32 +330,6 @@ export class Poll }); } - /** - * Starts the poll. Schedules `started` tick if necessary. - * - * @returns A promise that resolves after tick is scheduled and never rejects. - */ - start(): Promise { - return this.schedule({ - cancel: last => last.phase !== 'standby' && last.phase !== 'stopped', - interval: Poll.IMMEDIATE, - phase: 'started' - }); - } - - /** - * Stops the poll. Schedules `stopped` tick if necessary. - * - * @returns A promise that resolves after tick is scheduled and never rejects. - */ - stop(): Promise { - return this.schedule({ - cancel: last => last.phase === 'stopped', - interval: Poll.NEVER, - phase: 'stopped' - }); - } - /** * Schedule the next poll tick. * @@ -366,10 +340,10 @@ export class Poll * @returns A promise that resolves when the next poll state is active. * * #### Notes - * This method is protected to allow sub-classes to implement methods that can - * schedule poll ticks. + * This method is not meant to be invoked by user code typically. It is public + * to allow poll instances to be composed into classes that schedule ticks. */ - protected async schedule( + async schedule( next: Partial< IPoll.State & { cancel: (last: IPoll.State) => boolean } > = {} @@ -432,6 +406,32 @@ export class Poll : setTimeout(execute, state.interval); } + /** + * Starts the poll. Schedules `started` tick if necessary. + * + * @returns A promise that resolves after tick is scheduled and never rejects. + */ + start(): Promise { + return this.schedule({ + cancel: last => last.phase !== 'standby' && last.phase !== 'stopped', + interval: Poll.IMMEDIATE, + phase: 'started' + }); + } + + /** + * Stops the poll. Schedules `stopped` tick if necessary. + * + * @returns A promise that resolves after tick is scheduled and never rejects. + */ + stop(): Promise { + return this.schedule({ + cancel: last => last.phase === 'stopped', + interval: Poll.NEVER, + phase: 'stopped' + }); + } + /** * Execute a new poll factory promise or stand by if necessary. */ From 6f250660c117149e889ae52a943ee34f6ded0064 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Tue, 21 May 2019 15:46:57 +0100 Subject: [PATCH 34/54] Change Regulator to RateLimiter and export Debouncer, Throttler, and RateLimiter (abstract class). --- packages/coreutils/src/index.ts | 2 +- packages/coreutils/src/ratelimiter.ts | 80 +++++++++++++++++++++++++++ packages/coreutils/src/regulator.ts | 68 ----------------------- 3 files changed, 81 insertions(+), 69 deletions(-) create mode 100644 packages/coreutils/src/ratelimiter.ts delete mode 100644 packages/coreutils/src/regulator.ts diff --git a/packages/coreutils/src/index.ts b/packages/coreutils/src/index.ts index 795f71356979..1de9dc829756 100644 --- a/packages/coreutils/src/index.ts +++ b/packages/coreutils/src/index.ts @@ -9,7 +9,7 @@ export * from './nbformat'; export * from './pageconfig'; export * from './path'; export * from './poll'; -export * from './regulator'; +export * from './ratelimiter'; export * from './settingregistry'; export * from './statedb'; export * from './text'; diff --git a/packages/coreutils/src/ratelimiter.ts b/packages/coreutils/src/ratelimiter.ts new file mode 100644 index 000000000000..21d848ed177e --- /dev/null +++ b/packages/coreutils/src/ratelimiter.ts @@ -0,0 +1,80 @@ +// Copyright (c) Jupyter Development Team. +// Distributed under the terms of the Modified BSD License. + +import { IRateLimiter } from './interfaces'; + +import { Poll } from './poll'; + +/** + * A base class to implement rate limiters with different invocation strategies. + */ +export abstract class RateLimiter implements IRateLimiter { + /** + * Instantiate a rate limiter. + * + * @param fn - The function to rate limit. + * + * @param limit - The rate limit; defaults to 500ms. + */ + constructor(fn: () => any, limit = 500) { + this.limit = limit; + this.poll = new Poll({ + factory: async () => await fn(), + frequency: { backoff: false, interval: Poll.NEVER, max: Poll.NEVER }, + standby: 'never' + }); + void this.poll.stop(); + } + + /** + * The rate limit in milliseconds. + */ + readonly limit: number; + + /** + * Invoke the rate limited function. + */ + abstract async invoke(): Promise; + + /** + * Stop the function if it is mid-flight. + */ + async stop(): Promise { + return this.poll.stop(); + } + + /** + * The underlying poll instance used by the rate limiter. + */ + protected poll: Poll; +} + +/** + * Wraps and debounces a function that can be called multiple times and only + * executes the underlying function one `interval` after the last invocation. + * + * @param fn - The function to debounce. + * + * @param interval - The debounce interval; defaults to 500ms. + */ +export class Debouncer extends RateLimiter { + async invoke(): Promise { + return this.poll.schedule({ interval: this.limit, phase: 'invoked' }); + } +} + +/** + * Wraps and throttles a function that can be called multiple times and only + * executes the underlying function once per `interval`. + * + * @param fn - The function to throttle. + * + * @param interval - The throttle interval; defaults to 500ms. + */ +export class Throttler extends RateLimiter { + async invoke(): Promise { + if (this.poll.state.phase !== 'invoked') { + return this.poll.schedule({ interval: this.limit, phase: 'invoked' }); + } + } +} diff --git a/packages/coreutils/src/regulator.ts b/packages/coreutils/src/regulator.ts deleted file mode 100644 index 32046752b6ae..000000000000 --- a/packages/coreutils/src/regulator.ts +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright (c) Jupyter Development Team. -// Distributed under the terms of the Modified BSD License. - -import { Poll } from './poll'; - -interface IRegulator { - invoke(): Promise; - stop(): Promise; -} - -export type Debounced = IRegulator; - -export type Throttled = IRegulator; - -class Regulator extends Poll { - constructor(options: { - factory: Poll.Factory; - constraints: { interval: number; strategy: 'debounce' | 'throttle' }; - }) { - super({ factory: options.factory }); - this.constraints = options.constraints; - this.frequency = { backoff: false, interval: Poll.NEVER, max: Poll.NEVER }; - this.standby = 'never'; - void super.stop(); - } - - readonly constraints: { interval: number; strategy: 'debounce' | 'throttle' }; - - async invoke(): Promise { - const { interval, strategy } = this.constraints; - if (strategy === 'debounce') { - return this.schedule({ interval, phase: 'invoked' }); - } - if (this.state.phase !== 'invoked') { - return this.schedule({ interval, phase: 'invoked' }); - } - } -} - -/** - * Returns a debounced function that can be called multiple times and only - * executes the underlying function one `interval` after the last invocation. - * - * @param fn - The function to debounce. - * - * @param interval - The debounce interval; defaults to 500ms. - */ -export function debounce(fn: () => any, interval = 500): Debounced { - return new Regulator({ - factory: async () => await fn(), - constraints: { interval, strategy: 'debounce' } - }); -} - -/** - * Returns a throttled function that can be called multiple times and only - * executes the underlying function once per `interval`. - * - * @param fn - The function to throttle. - * - * @param interval - The throttle interval; defaults to 500ms. - */ -export function throttle(fn: () => any, interval = 500): Throttled { - return new Regulator({ - factory: async () => await fn(), - constraints: { interval, strategy: 'throttle' } - }); -} From 6f469ae430311a472b2f45c06e7eba7e7be78ccd Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Tue, 21 May 2019 15:47:26 +0100 Subject: [PATCH 35/54] Update apputils. --- packages/apputils-extension/src/index.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/apputils-extension/src/index.ts b/packages/apputils-extension/src/index.ts index 317b7f9996ba..d71f34214fc9 100644 --- a/packages/apputils-extension/src/index.ts +++ b/packages/apputils-extension/src/index.ts @@ -22,13 +22,13 @@ import { } from '@jupyterlab/apputils'; import { - debounce, + Debouncer, + IRateLimiter, ISettingRegistry, IStateDB, SettingRegistry, StateDB, - throttle, - Throttled, + Throttler, URLExt } from '@jupyterlab/coreutils'; @@ -310,7 +310,7 @@ const splash: JupyterFrontEndPlugin = { // Create debounced recovery dialog function. let dialog: Dialog; - const recovery: Throttled = throttle(async () => { + const recovery: IRateLimiter = new Throttler(async () => { if (dialog) { return; } @@ -418,7 +418,7 @@ const state: JupyterFrontEndPlugin = { const workspace = resolver.name; const transform = new PromiseDelegate(); const db = new StateDB({ transform: transform.promise }); - const save = debounce(async () => { + const save = new Debouncer(async () => { const id = workspace; const metadata = { id }; const data = await db.toJSON(); From be985f0779836f725aaf78f9ec9d4092896ff65d Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Tue, 21 May 2019 15:47:39 +0100 Subject: [PATCH 36/54] Update tests. --- ...{regulator.spec.ts => ratelimiter.spec.ts} | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) rename tests/test-coreutils/src/{regulator.spec.ts => ratelimiter.spec.ts} (50%) diff --git a/tests/test-coreutils/src/regulator.spec.ts b/tests/test-coreutils/src/ratelimiter.spec.ts similarity index 50% rename from tests/test-coreutils/src/regulator.spec.ts rename to tests/test-coreutils/src/ratelimiter.spec.ts index 995c1f87ba8d..708248e10bc4 100644 --- a/tests/test-coreutils/src/regulator.spec.ts +++ b/tests/test-coreutils/src/ratelimiter.spec.ts @@ -3,20 +3,20 @@ import { expect } from 'chai'; -import { debounce } from '@jupyterlab/coreutils'; +import { Debouncer } from '@jupyterlab/coreutils'; -describe('debounce()', () => { - it('should debounce a function within an interval', async () => { +describe('Debouncer', () => { + it('should debounce a function within a limit interval', async () => { let called = 0; - const interval = 500; - const debounced = debounce(async () => { + const limit = 500; + const debouncer = new Debouncer(async () => { called += 1; - }, interval); - const one = debounced.invoke(); - const two = debounced.invoke(); - const three = debounced.invoke(); - const four = debounced.invoke(); - const five = debounced.invoke(); + }, limit); + const one = debouncer.invoke(); + const two = debouncer.invoke(); + const three = debouncer.invoke(); + const four = debouncer.invoke(); + const five = debouncer.invoke(); await five; expect(called).to.equal(1); await four; @@ -27,9 +27,9 @@ describe('debounce()', () => { expect(called).to.equal(1); await one; expect(called).to.equal(1); - await debounced.invoke(); + await debouncer.invoke(); expect(called).to.equal(2); - await debounced.invoke(); + await debouncer.invoke(); expect(called).to.equal(3); }); }); From e6dbb80ed774b44b262b657b27b34fb1bc91abb2 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Tue, 21 May 2019 15:55:31 +0100 Subject: [PATCH 37/54] Update doc strings and use RateLimiter#stop() in constructor. --- packages/coreutils/src/ratelimiter.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/coreutils/src/ratelimiter.ts b/packages/coreutils/src/ratelimiter.ts index 21d848ed177e..b9de37b8f910 100644 --- a/packages/coreutils/src/ratelimiter.ts +++ b/packages/coreutils/src/ratelimiter.ts @@ -23,7 +23,7 @@ export abstract class RateLimiter implements IRateLimiter { frequency: { backoff: false, interval: Poll.NEVER, max: Poll.NEVER }, standby: 'never' }); - void this.poll.stop(); + void this.stop(); } /** @@ -55,9 +55,13 @@ export abstract class RateLimiter implements IRateLimiter { * * @param fn - The function to debounce. * - * @param interval - The debounce interval; defaults to 500ms. + * @param limit - The debounce rate limit; defaults to 500ms. */ export class Debouncer extends RateLimiter { + /** + * Invokes the function and only executes after rate limit has elapsed. + * Each invocation resets the timer. + */ async invoke(): Promise { return this.poll.schedule({ interval: this.limit, phase: 'invoked' }); } @@ -69,9 +73,12 @@ export class Debouncer extends RateLimiter { * * @param fn - The function to throttle. * - * @param interval - The throttle interval; defaults to 500ms. + * @param limit - The throttle rate limit; defaults to 500ms. */ export class Throttler extends RateLimiter { + /** + * Throttles function invocations if one is currently in flight. + */ async invoke(): Promise { if (this.poll.state.phase !== 'invoked') { return this.poll.schedule({ interval: this.limit, phase: 'invoked' }); From 5007c3ef30125c2f50f8509407cff10d1c182b34 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 00:21:42 +0100 Subject: [PATCH 38/54] Add support for returning a promise that always resolves or rejects when the factory does. --- packages/coreutils/src/interfaces.ts | 10 ++++- packages/coreutils/src/ratelimiter.ts | 63 +++++++++++++++++++++------ 2 files changed, 57 insertions(+), 16 deletions(-) diff --git a/packages/coreutils/src/interfaces.ts b/packages/coreutils/src/interfaces.ts index 003d5a0a78ca..7c93dc915bd4 100644 --- a/packages/coreutils/src/interfaces.ts +++ b/packages/coreutils/src/interfaces.ts @@ -1,6 +1,8 @@ // Copyright (c) Jupyter Development Team. // Distributed under the terms of the Modified BSD License. +import { IDisposable } from '@phosphor/disposable'; + /** * A generic interface for change emitter payloads. */ @@ -99,8 +101,12 @@ export interface IDataConnector { /** * A function whose invocations are rate limited and can be stopped after * invocation before it has fired. + * + * @typeparam T - The resolved type of the underlying function. Defaults to any. + * + * @typeparam U - The rejected type of the underlying function. Defaults to any. */ -export interface IRateLimiter { +export interface IRateLimiter extends IDisposable { /** * The rate limit in milliseconds. */ @@ -109,7 +115,7 @@ export interface IRateLimiter { /** * Invoke the rate limited function. */ - invoke(): Promise; + invoke(): Promise; /** * Stop the function if it is mid-flight. diff --git a/packages/coreutils/src/ratelimiter.ts b/packages/coreutils/src/ratelimiter.ts index b9de37b8f910..8d05e34183d3 100644 --- a/packages/coreutils/src/ratelimiter.ts +++ b/packages/coreutils/src/ratelimiter.ts @@ -1,14 +1,20 @@ // Copyright (c) Jupyter Development Team. // Distributed under the terms of the Modified BSD License. +import { PromiseDelegate } from '@phosphor/coreutils'; + import { IRateLimiter } from './interfaces'; import { Poll } from './poll'; /** * A base class to implement rate limiters with different invocation strategies. + * + * @typeparam T - The resolved type of the underlying function. + * + * @typeparam U - The rejected type of the underlying function. */ -export abstract class RateLimiter implements IRateLimiter { +export abstract class RateLimiter implements IRateLimiter { /** * Instantiate a rate limiter. * @@ -16,7 +22,7 @@ export abstract class RateLimiter implements IRateLimiter { * * @param limit - The rate limit; defaults to 500ms. */ - constructor(fn: () => any, limit = 500) { + constructor(fn: () => T | Promise, limit = 500) { this.limit = limit; this.poll = new Poll({ factory: async () => await fn(), @@ -24,6 +30,31 @@ export abstract class RateLimiter implements IRateLimiter { standby: 'never' }); void this.stop(); + this.poll.ticked.connect((_, state) => { + const { payload } = this; + if (state.phase === 'resolved' || state.phase === 'reconnected') { + this.payload = new PromiseDelegate(); + payload.resolve(state.payload as T); + return; + } + if (state.phase === 'rejected') { + this.payload = new PromiseDelegate(); + payload.reject(state.payload as U); + return; + } + }, this); + } + + get isDisposed(): boolean { + return this.payload === null; + } + + dispose(): void { + if (this.isDisposed) { + return; + } + this.payload = null; + this.poll.dispose(); } /** @@ -34,7 +65,7 @@ export abstract class RateLimiter implements IRateLimiter { /** * Invoke the rate limited function. */ - abstract async invoke(): Promise; + abstract async invoke(): Promise; /** * Stop the function if it is mid-flight. @@ -43,27 +74,30 @@ export abstract class RateLimiter implements IRateLimiter { return this.poll.stop(); } + protected payload: PromiseDelegate | null = new PromiseDelegate(); + /** * The underlying poll instance used by the rate limiter. */ - protected poll: Poll; + protected poll: Poll; } /** * Wraps and debounces a function that can be called multiple times and only * executes the underlying function one `interval` after the last invocation. * - * @param fn - The function to debounce. + * @typeparam T - The resolved type of the underlying function. Defaults to any. * - * @param limit - The debounce rate limit; defaults to 500ms. + * @typeparam U - The rejected type of the underlying function. Defaults to any. */ -export class Debouncer extends RateLimiter { +export class Debouncer extends RateLimiter { /** * Invokes the function and only executes after rate limit has elapsed. * Each invocation resets the timer. */ - async invoke(): Promise { - return this.poll.schedule({ interval: this.limit, phase: 'invoked' }); + async invoke(): Promise { + void this.poll.schedule({ interval: this.limit, phase: 'invoked' }); + return this.payload.promise; } } @@ -71,17 +105,18 @@ export class Debouncer extends RateLimiter { * Wraps and throttles a function that can be called multiple times and only * executes the underlying function once per `interval`. * - * @param fn - The function to throttle. + * @typeparam T - The resolved type of the underlying function. Defaults to any. * - * @param limit - The throttle rate limit; defaults to 500ms. + * @typeparam U - The rejected type of the underlying function. Defaults to any. */ -export class Throttler extends RateLimiter { +export class Throttler extends RateLimiter { /** * Throttles function invocations if one is currently in flight. */ - async invoke(): Promise { + async invoke(): Promise { if (this.poll.state.phase !== 'invoked') { - return this.poll.schedule({ interval: this.limit, phase: 'invoked' }); + void this.poll.schedule({ interval: this.limit, phase: 'invoked' }); } + return this.payload.promise; } } From 5bc5041099b52aefa9294388a2b9be4995474b31 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 01:40:26 +0100 Subject: [PATCH 39/54] Support passing in NEVER to sleep function. --- packages/coreutils/src/poll.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/coreutils/src/poll.ts b/packages/coreutils/src/poll.ts index 17bd17ebf7fe..1979e34c4ab7 100644 --- a/packages/coreutils/src/poll.ts +++ b/packages/coreutils/src/poll.ts @@ -647,6 +647,11 @@ namespace Private { last: IPoll.State ): number { const { backoff, interval, max } = frequency; + + if (interval === Poll.NEVER) { + return interval; + } + const growth = backoff === true ? DEFAULT_BACKOFF : backoff === false ? 1 : backoff; const random = getRandomIntInclusive(interval, last.interval * growth); From 97adb2f7f6f3a06ca6af5e3e754e93b06f524a6a Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 10:08:38 +0100 Subject: [PATCH 40/54] Simplify poll tests now that Poll#schedule is public. --- tests/test-coreutils/src/poll.spec.ts | 90 +++------------------------ 1 file changed, 8 insertions(+), 82 deletions(-) diff --git a/tests/test-coreutils/src/poll.spec.ts b/tests/test-coreutils/src/poll.spec.ts index e8eb6a2ca29e..0720969bb067 100644 --- a/tests/test-coreutils/src/poll.spec.ts +++ b/tests/test-coreutils/src/poll.spec.ts @@ -7,28 +7,14 @@ import { IPoll, Poll } from '@jupyterlab/coreutils'; import { sleep } from '@jupyterlab/testutils'; -class TestPoll extends Poll< - T, - U, - V -> { - async schedule( - next: Partial< - IPoll.State & { cancel: (last: IPoll.State) => boolean } - > = {} - ) { - return super.schedule(next); - } -} - describe('Poll', () => { - describe('#constructor()', () => { - let poll: Poll; + let poll: Poll; - afterEach(() => { - poll.dispose(); - }); + afterEach(() => { + poll.dispose(); + }); + describe('#constructor()', () => { it('should create a poll', () => { poll = new Poll({ factory: () => Promise.resolve(), @@ -140,12 +126,6 @@ describe('Poll', () => { }); describe('#name', () => { - let poll: Poll; - - afterEach(() => { - poll.dispose(); - }); - it('should be set to value passed in during instantation', () => { const factory = () => Promise.resolve(); const name = '@jupyterlab/test-coreutils:Poll#name-1'; @@ -162,12 +142,6 @@ describe('Poll', () => { }); describe('#disposed', () => { - let poll: Poll; - - afterEach(() => { - poll.dispose(); - }); - it('should emit when the poll is disposed', () => { poll = new Poll({ factory: () => Promise.resolve(), @@ -183,12 +157,6 @@ describe('Poll', () => { }); describe('#isDisposed', () => { - let poll: Poll; - - afterEach(() => { - poll.dispose(); - }); - it('should indicate whether the poll is disposed', () => { poll = new Poll({ factory: () => Promise.resolve(), @@ -201,12 +169,6 @@ describe('Poll', () => { }); describe('#tick', () => { - let poll: Poll; - - afterEach(() => { - poll.dispose(); - }); - it('should resolve after a tick', async () => { poll = new Poll({ factory: () => Promise.resolve(), @@ -264,12 +226,6 @@ describe('Poll', () => { }); describe('#ticked', () => { - let poll: Poll; - - afterEach(() => { - poll.dispose(); - }); - it('should emit when the poll ticks after `when` resolves', async () => { const expected = 'when-resolved resolved'; const ticker: IPoll.Phase[] = []; @@ -317,12 +273,6 @@ describe('Poll', () => { }); describe('#dispose()', () => { - let poll: Poll; - - afterEach(() => { - poll.dispose(); - }); - it('should dispose the poll and be safe to call repeatedly', async () => { let rejected = false; let tick: Promise; @@ -345,12 +295,6 @@ describe('Poll', () => { }); describe('#refresh()', () => { - let poll: Poll; - - afterEach(() => { - poll.dispose(); - }); - it('should refresh the poll when the poll is ready', async () => { const expected = 'when-resolved refreshed resolved'; const ticker: IPoll.Phase[] = []; @@ -397,12 +341,6 @@ describe('Poll', () => { }); describe('#start()', () => { - let poll: Poll; - - afterEach(() => { - poll.dispose(); - }); - it('should start the poll if it is stopped', async () => { const expected = 'when-resolved stopped started resolved'; const ticker: IPoll.Phase[] = []; @@ -456,12 +394,6 @@ describe('Poll', () => { }); describe('#stop()', () => { - let poll: Poll; - - afterEach(() => { - poll.dispose(); - }); - it('should stop the poll if it is active', async () => { const expected = 'when-resolved stopped started resolved'; const ticker: IPoll.Phase[] = []; @@ -510,14 +442,8 @@ describe('Poll', () => { }); describe('#schedule()', () => { - let poll: TestPoll; - - afterEach(() => { - poll.dispose(); - }); - it('should schedule the next poll state', async () => { - poll = new TestPoll({ + poll = new Poll({ factory: () => Promise.resolve(), frequency: { interval: 100 }, name: '@jupyterlab/test-coreutils:Poll#schedule()-1' @@ -533,7 +459,7 @@ describe('Poll', () => { }); it('should default to standby state', async () => { - poll = new TestPoll({ + poll = new Poll({ factory: () => Promise.resolve(), frequency: { interval: 100 }, name: '@jupyterlab/test-coreutils:Poll#schedule()-2' @@ -549,7 +475,7 @@ describe('Poll', () => { }); it('should support phase transition cancellation', async () => { - poll = new TestPoll({ + poll = new Poll({ factory: () => Promise.resolve(), frequency: { interval: 100 }, name: '@jupyterlab/test-coreutils:Poll#schedule()-3' From f3683a79848434c4a59d4d604ec204def6c5bebe Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 11:46:54 +0100 Subject: [PATCH 41/54] Test debouncing and throttling. --- tests/test-coreutils/src/ratelimiter.spec.ts | 125 +++++++++++++++---- 1 file changed, 99 insertions(+), 26 deletions(-) diff --git a/tests/test-coreutils/src/ratelimiter.spec.ts b/tests/test-coreutils/src/ratelimiter.spec.ts index 708248e10bc4..936221abaa07 100644 --- a/tests/test-coreutils/src/ratelimiter.spec.ts +++ b/tests/test-coreutils/src/ratelimiter.spec.ts @@ -3,33 +3,106 @@ import { expect } from 'chai'; -import { Debouncer } from '@jupyterlab/coreutils'; +import { Debouncer, Throttler } from '@jupyterlab/coreutils'; + +import { sleep } from '@jupyterlab/testutils'; describe('Debouncer', () => { - it('should debounce a function within a limit interval', async () => { - let called = 0; - const limit = 500; - const debouncer = new Debouncer(async () => { - called += 1; - }, limit); - const one = debouncer.invoke(); - const two = debouncer.invoke(); - const three = debouncer.invoke(); - const four = debouncer.invoke(); - const five = debouncer.invoke(); - await five; - expect(called).to.equal(1); - await four; - expect(called).to.equal(1); - await three; - expect(called).to.equal(1); - await two; - expect(called).to.equal(1); - await one; - expect(called).to.equal(1); - await debouncer.invoke(); - expect(called).to.equal(2); - await debouncer.invoke(); - expect(called).to.equal(3); + let debouncer: Debouncer; + + afterEach(() => { + debouncer.dispose(); + }); + + describe('#invoke()', () => { + it('should debounce a function', async () => { + let called = 0; + const limit = 500; + + debouncer = new Debouncer(async () => ++called, limit); + + let one = debouncer.invoke(); + let two = debouncer.invoke(); + let three = debouncer.invoke(); + let four = debouncer.invoke(); + let five = debouncer.invoke(); + let six = debouncer.invoke(); + + expect(await one).to.equal(1); + expect(await two).to.equal(1); + expect(await three).to.equal(1); + expect(await four).to.equal(1); + expect(await five).to.equal(1); + expect(await six).to.equal(1); + + one = debouncer.invoke(); + await sleep(300); + two = debouncer.invoke(); + await sleep(300); + three = debouncer.invoke(); + await sleep(300); + four = debouncer.invoke(); + await sleep(300); + five = debouncer.invoke(); + await sleep(300); + six = debouncer.invoke(); + + expect(await one).to.equal(2); + expect(await two).to.equal(2); + expect(await three).to.equal(2); + expect(await four).to.equal(2); + expect(await five).to.equal(2); + expect(await six).to.equal(2); + }); + }); +}); + +describe('Throttler', () => { + let throttler: Throttler; + + afterEach(() => { + throttler.dispose(); + }); + + describe('#invoke()', () => { + it('should throttle a function', async () => { + let called = 0; + const limit = 500; + + throttler = new Throttler(async () => ++called, limit); + + let one = throttler.invoke(); + let two = throttler.invoke(); + let three = throttler.invoke(); + let four = throttler.invoke(); + let five = throttler.invoke(); + let six = throttler.invoke(); + + expect(await one).to.equal(1); + expect(await two).to.equal(1); + expect(await three).to.equal(1); + expect(await four).to.equal(1); + expect(await five).to.equal(1); + expect(await six).to.equal(1); + + one = throttler.invoke(); + await sleep(300); + two = throttler.invoke(); + await sleep(300); + three = throttler.invoke(); + await sleep(300); + four = throttler.invoke(); + await sleep(300); + five = throttler.invoke(); + await sleep(300); + six = throttler.invoke(); + + expect(await one).to.equal(2); + expect(await two).to.equal(2); + expect(await three).to.equal(3); + expect(await four).to.equal(3); + expect(await five).to.equal(4); + expect(await six).to.equal(4); + }); }); }); From a184cbd4506d4cf0f37c41ac9863cada9879855f Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 12:14:36 +0100 Subject: [PATCH 42/54] Update rate limiter tests. --- tests/test-coreutils/src/ratelimiter.spec.ts | 152 +++++++++++-------- 1 file changed, 86 insertions(+), 66 deletions(-) diff --git a/tests/test-coreutils/src/ratelimiter.spec.ts b/tests/test-coreutils/src/ratelimiter.spec.ts index 936221abaa07..92756c305acd 100644 --- a/tests/test-coreutils/src/ratelimiter.spec.ts +++ b/tests/test-coreutils/src/ratelimiter.spec.ts @@ -15,44 +15,54 @@ describe('Debouncer', () => { }); describe('#invoke()', () => { - it('should debounce a function', async () => { + it('should rate limit invocations', async () => { + const limit = 500; + const wanted = [1, 1, 1, 1, 1, 1]; let called = 0; + + debouncer = new Debouncer(async () => ++called, limit); + + const one = debouncer.invoke(); + const two = debouncer.invoke(); + const three = debouncer.invoke(); + const four = debouncer.invoke(); + const five = debouncer.invoke(); + const six = debouncer.invoke(); + + expect(await one).to.equal(wanted[0]); + expect(await two).to.equal(wanted[1]); + expect(await three).to.equal(wanted[2]); + expect(await four).to.equal(wanted[3]); + expect(await five).to.equal(wanted[4]); + expect(await six).to.equal(wanted[5]); + }); + + it('should debounce invocations within an interval', async () => { const limit = 500; + const sleeps = [200, 200, 200, 200, 600]; + const wanted = [1, 1, 1, 1, 1, 2]; + let called = 0; debouncer = new Debouncer(async () => ++called, limit); - let one = debouncer.invoke(); - let two = debouncer.invoke(); - let three = debouncer.invoke(); - let four = debouncer.invoke(); - let five = debouncer.invoke(); - let six = debouncer.invoke(); - - expect(await one).to.equal(1); - expect(await two).to.equal(1); - expect(await three).to.equal(1); - expect(await four).to.equal(1); - expect(await five).to.equal(1); - expect(await six).to.equal(1); - - one = debouncer.invoke(); - await sleep(300); - two = debouncer.invoke(); - await sleep(300); - three = debouncer.invoke(); - await sleep(300); - four = debouncer.invoke(); - await sleep(300); - five = debouncer.invoke(); - await sleep(300); - six = debouncer.invoke(); - - expect(await one).to.equal(2); - expect(await two).to.equal(2); - expect(await three).to.equal(2); - expect(await four).to.equal(2); - expect(await five).to.equal(2); - expect(await six).to.equal(2); + const one = debouncer.invoke(); + await sleep(sleeps[0]); + const two = debouncer.invoke(); + await sleep(sleeps[1]); + const three = debouncer.invoke(); + await sleep(sleeps[2]); + const four = debouncer.invoke(); + await sleep(sleeps[3]); + const five = debouncer.invoke(); + await sleep(sleeps[4]); + const six = debouncer.invoke(); + + expect(await one).to.equal(wanted[0]); + expect(await two).to.equal(wanted[1]); + expect(await three).to.equal(wanted[2]); + expect(await four).to.equal(wanted[3]); + expect(await five).to.equal(wanted[4]); + expect(await six).to.equal(wanted[5]); }); }); }); @@ -65,44 +75,54 @@ describe('Throttler', () => { }); describe('#invoke()', () => { - it('should throttle a function', async () => { + it('should rate limit invocations', async () => { + const limit = 500; + const wanted = [1, 1, 1, 1, 1, 1]; let called = 0; + + throttler = new Debouncer(async () => ++called, limit); + + const one = throttler.invoke(); + const two = throttler.invoke(); + const three = throttler.invoke(); + const four = throttler.invoke(); + const five = throttler.invoke(); + const six = throttler.invoke(); + + expect(await one).to.equal(wanted[0]); + expect(await two).to.equal(wanted[1]); + expect(await three).to.equal(wanted[2]); + expect(await four).to.equal(wanted[3]); + expect(await five).to.equal(wanted[4]); + expect(await six).to.equal(wanted[5]); + }); + + it('should throttle invocations within an interval', async () => { const limit = 500; + const sleeps = [200, 200, 200, 200, 600]; + const wanted = [1, 1, 1, 2, 2, 3]; + let called = 0; throttler = new Throttler(async () => ++called, limit); - let one = throttler.invoke(); - let two = throttler.invoke(); - let three = throttler.invoke(); - let four = throttler.invoke(); - let five = throttler.invoke(); - let six = throttler.invoke(); - - expect(await one).to.equal(1); - expect(await two).to.equal(1); - expect(await three).to.equal(1); - expect(await four).to.equal(1); - expect(await five).to.equal(1); - expect(await six).to.equal(1); - - one = throttler.invoke(); - await sleep(300); - two = throttler.invoke(); - await sleep(300); - three = throttler.invoke(); - await sleep(300); - four = throttler.invoke(); - await sleep(300); - five = throttler.invoke(); - await sleep(300); - six = throttler.invoke(); - - expect(await one).to.equal(2); - expect(await two).to.equal(2); - expect(await three).to.equal(3); - expect(await four).to.equal(3); - expect(await five).to.equal(4); - expect(await six).to.equal(4); + const one = throttler.invoke(); + await sleep(sleeps[0]); + const two = throttler.invoke(); + await sleep(sleeps[1]); + const three = throttler.invoke(); + await sleep(sleeps[2]); + const four = throttler.invoke(); + await sleep(sleeps[3]); + const five = throttler.invoke(); + await sleep(sleeps[4]); + const six = throttler.invoke(); + + expect(await one).to.equal(wanted[0]); + expect(await two).to.equal(wanted[1]); + expect(await three).to.equal(wanted[2]); + expect(await four).to.equal(wanted[3]); + expect(await five).to.equal(wanted[4]); + expect(await six).to.equal(wanted[5]); }); }); }); From 5b14fbf094edce32726bf0725b448e8338a0aa84 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 13:51:22 +0100 Subject: [PATCH 43/54] Fix RateLimiter#stop(). --- packages/coreutils/src/ratelimiter.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packages/coreutils/src/ratelimiter.ts b/packages/coreutils/src/ratelimiter.ts index 8d05e34183d3..5288b7ce04e2 100644 --- a/packages/coreutils/src/ratelimiter.ts +++ b/packages/coreutils/src/ratelimiter.ts @@ -29,16 +29,19 @@ export abstract class RateLimiter implements IRateLimiter { frequency: { backoff: false, interval: Poll.NEVER, max: Poll.NEVER }, standby: 'never' }); - void this.stop(); - this.poll.ticked.connect((_, state) => { + this.payload = new PromiseDelegate(); + this.poll.ticked.connect(async (_, state) => { const { payload } = this; - if (state.phase === 'resolved' || state.phase === 'reconnected') { + + if (state.phase === 'resolved') { this.payload = new PromiseDelegate(); - payload.resolve(state.payload as T); + payload.resolve((state.payload as T) || undefined); return; } - if (state.phase === 'rejected') { + + if (state.phase === 'rejected' || state.phase === 'stopped') { this.payload = new PromiseDelegate(); + payload.promise.catch(_ => undefined); payload.reject(state.payload as U); return; } @@ -74,7 +77,7 @@ export abstract class RateLimiter implements IRateLimiter { return this.poll.stop(); } - protected payload: PromiseDelegate | null = new PromiseDelegate(); + protected payload: PromiseDelegate | null = null; /** * The underlying poll instance used by the rate limiter. From bd2ca9335b3e40091193a15a1ea1939b4fcdc029 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 13:51:48 +0100 Subject: [PATCH 44/54] Add rate limiter stop tests. --- tests/test-coreutils/src/ratelimiter.spec.ts | 62 ++++++++++++++++++-- 1 file changed, 58 insertions(+), 4 deletions(-) diff --git a/tests/test-coreutils/src/ratelimiter.spec.ts b/tests/test-coreutils/src/ratelimiter.spec.ts index 92756c305acd..6453326f0818 100644 --- a/tests/test-coreutils/src/ratelimiter.spec.ts +++ b/tests/test-coreutils/src/ratelimiter.spec.ts @@ -20,7 +20,7 @@ describe('Debouncer', () => { const wanted = [1, 1, 1, 1, 1, 1]; let called = 0; - debouncer = new Debouncer(async () => ++called, limit); + debouncer = new Debouncer(() => ++called, limit); const one = debouncer.invoke(); const two = debouncer.invoke(); @@ -43,7 +43,7 @@ describe('Debouncer', () => { const wanted = [1, 1, 1, 1, 1, 2]; let called = 0; - debouncer = new Debouncer(async () => ++called, limit); + debouncer = new Debouncer(() => ++called, limit); const one = debouncer.invoke(); await sleep(sleeps[0]); @@ -65,6 +65,33 @@ describe('Debouncer', () => { expect(await six).to.equal(wanted[5]); }); }); + + describe('#stop()', () => { + it('should stop an invocation that has not fired yet', async () => { + let called = 0; + let caught = 0; + const wanted = [1, 2, 3, 4, 5, 6]; + + debouncer = new Debouncer(() => ++called); + + const one = debouncer.invoke().catch(_ => ++caught); + const two = debouncer.invoke().catch(_ => ++caught); + const three = debouncer.invoke().catch(_ => ++caught); + const four = debouncer.invoke().catch(_ => ++caught); + const five = debouncer.invoke().catch(_ => ++caught); + const six = debouncer.invoke().catch(_ => ++caught); + + void debouncer.stop(); + + expect(await one).to.equal(wanted[0]); + expect(await two).to.equal(wanted[1]); + expect(await three).to.equal(wanted[2]); + expect(await four).to.equal(wanted[3]); + expect(await five).to.equal(wanted[4]); + expect(await six).to.equal(wanted[5]); + expect(called).to.equal(0); + }); + }); }); describe('Throttler', () => { @@ -80,7 +107,7 @@ describe('Throttler', () => { const wanted = [1, 1, 1, 1, 1, 1]; let called = 0; - throttler = new Debouncer(async () => ++called, limit); + throttler = new Throttler(() => ++called, limit); const one = throttler.invoke(); const two = throttler.invoke(); @@ -103,7 +130,7 @@ describe('Throttler', () => { const wanted = [1, 1, 1, 2, 2, 3]; let called = 0; - throttler = new Throttler(async () => ++called, limit); + throttler = new Throttler(() => ++called, limit); const one = throttler.invoke(); await sleep(sleeps[0]); @@ -125,4 +152,31 @@ describe('Throttler', () => { expect(await six).to.equal(wanted[5]); }); }); + + describe('#stop()', () => { + it('should stop an invocation that has not fired yet', async () => { + let called = 0; + let caught = 0; + const wanted = [1, 2, 3, 4, 5, 6]; + + throttler = new Throttler(() => ++called); + + const one = throttler.invoke().catch(_ => ++caught); + const two = throttler.invoke().catch(_ => ++caught); + const three = throttler.invoke().catch(_ => ++caught); + const four = throttler.invoke().catch(_ => ++caught); + const five = throttler.invoke().catch(_ => ++caught); + const six = throttler.invoke().catch(_ => ++caught); + + void throttler.stop(); + + expect(await one).to.equal(wanted[0]); + expect(await two).to.equal(wanted[1]); + expect(await three).to.equal(wanted[2]); + expect(await four).to.equal(wanted[3]); + expect(await five).to.equal(wanted[4]); + expect(await six).to.equal(wanted[5]); + expect(called).to.equal(0); + }); + }); }); From c37dff35bf2ac98a633224ec91a68df0343d21a6 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 15:10:47 +0100 Subject: [PATCH 45/54] Update doc strings. --- packages/coreutils/src/ratelimiter.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/coreutils/src/ratelimiter.ts b/packages/coreutils/src/ratelimiter.ts index 5288b7ce04e2..9feffe691f0d 100644 --- a/packages/coreutils/src/ratelimiter.ts +++ b/packages/coreutils/src/ratelimiter.ts @@ -48,10 +48,16 @@ export abstract class RateLimiter implements IRateLimiter { }, this); } + /** + * Whether the rate limiter is disposed. + */ get isDisposed(): boolean { return this.payload === null; } + /** + * Disposes the rate limiter. + */ dispose(): void { if (this.isDisposed) { return; @@ -77,6 +83,9 @@ export abstract class RateLimiter implements IRateLimiter { return this.poll.stop(); } + /** + * A promise that resolves on each successful invocation. + */ protected payload: PromiseDelegate | null = null; /** From 98b9487054d507c26746096cd33abba7169144e4 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 16:09:08 +0100 Subject: [PATCH 46/54] Respect poll interval after poll's `when` promise resolves. --- packages/coreutils/src/poll.ts | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/packages/coreutils/src/poll.ts b/packages/coreutils/src/poll.ts index 1979e34c4ab7..b0830f2a0494 100644 --- a/packages/coreutils/src/poll.ts +++ b/packages/coreutils/src/poll.ts @@ -190,10 +190,7 @@ export class Poll return; } - return this.schedule({ - interval: Poll.IMMEDIATE, - phase: 'when-resolved' - }); + void this.schedule({ phase: 'when-resolved' }); }) .catch(reason => { if (this.isDisposed) { @@ -201,11 +198,7 @@ export class Poll } console.warn(`Poll (${this.name}) started despite rejection.`, reason); - - return this.schedule({ - interval: Poll.IMMEDIATE, - phase: 'when-rejected' - }); + void this.schedule({ phase: 'when-rejected' }); }); } From f3424fdc8209d0e8e4e14d17a229ecaedbf1f729 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 16:09:29 +0100 Subject: [PATCH 47/54] Update poll tests to reflect when promise behavior change. --- tests/test-coreutils/src/poll.spec.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test-coreutils/src/poll.spec.ts b/tests/test-coreutils/src/poll.spec.ts index 0720969bb067..ccd4f9f97d86 100644 --- a/tests/test-coreutils/src/poll.spec.ts +++ b/tests/test-coreutils/src/poll.spec.ts @@ -172,7 +172,7 @@ describe('Poll', () => { it('should resolve after a tick', async () => { poll = new Poll({ factory: () => Promise.resolve(), - frequency: { interval: 2000, backoff: false }, + frequency: { interval: 200, backoff: false }, name: '@jupyterlab/test-coreutils:Poll#tick-1' }); const expected = 'when-resolved resolved'; @@ -182,7 +182,7 @@ describe('Poll', () => { poll.tick.then(tock).catch(() => undefined); }; void poll.tick.then(tock); - await sleep(200); // Sleep for less than the interval. + await sleep(250); // Sleep for longer than the interval. expect(ticker.join(' ')).to.equal(expected); }); @@ -231,13 +231,13 @@ describe('Poll', () => { const ticker: IPoll.Phase[] = []; poll = new Poll({ factory: () => Promise.resolve(), - frequency: { interval: 2000, backoff: false }, + frequency: { interval: 200, backoff: false }, name: '@jupyterlab/test-coreutils:Poll#ticked-1' }); poll.ticked.connect(() => { ticker.push(poll.state.phase); }); - await sleep(200); // Sleep for less than the interval. + await sleep(250); // Sleep for longer than the interval. expect(ticker.join(' ')).to.equal(expected); }); @@ -247,7 +247,7 @@ describe('Poll', () => { const promise = Promise.reject(); poll = new Poll({ factory: () => Promise.resolve(), - frequency: { interval: 2000, backoff: false }, + frequency: { interval: 200, backoff: false }, name: '@jupyterlab/test-coreutils:Poll#ticked-2', when: promise }); @@ -255,7 +255,7 @@ describe('Poll', () => { ticker.push(poll.state.phase); }); await promise.catch(() => undefined); - await sleep(200); // Sleep for less than the interval. + await sleep(250); // Sleep for longer than the interval. expect(ticker.join(' ')).to.equal(expected); }); From 2aa927f1b4aae5a10bcba076834f23763f36bf62 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 16:10:03 +0100 Subject: [PATCH 48/54] Catch recover invocation rejection in splash screen because it calls Throttler#stop. --- packages/apputils-extension/src/index.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/apputils-extension/src/index.ts b/packages/apputils-extension/src/index.ts index d71f34214fc9..c55b186bdded 100644 --- a/packages/apputils-extension/src/index.ts +++ b/packages/apputils-extension/src/index.ts @@ -335,7 +335,8 @@ const splash: JupyterFrontEndPlugin = { // Re-invoke the recovery timer in the next frame. requestAnimationFrame(() => { - void recovery.invoke(); + // Because recovery can be stopped, handle invocation rejection. + void recovery.invoke().catch(_ => undefined); }); } catch (error) { /* no-op */ @@ -352,7 +353,8 @@ const splash: JupyterFrontEndPlugin = { splashCount++; document.body.appendChild(splash); - void recovery.invoke(); + // Because recovery can be stopped, handle invocation rejection. + void recovery.invoke().catch(_ => undefined); return new DisposableDelegate(async () => { await restored; From f2a3f45e00228214992df929a76c0e0ac011c8d5 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 16:13:39 +0100 Subject: [PATCH 49/54] Use Debouncer in LabShell. --- packages/application/src/shell.ts | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/packages/application/src/shell.ts b/packages/application/src/shell.ts index 9c7eead2548e..ebe83a684f39 100644 --- a/packages/application/src/shell.ts +++ b/packages/application/src/shell.ts @@ -1,6 +1,8 @@ // Copyright (c) Jupyter Development Team. // Distributed under the terms of the Modified BSD License. +import { Debouncer } from '@jupyterlab/coreutils'; + import { DocumentRegistry } from '@jupyterlab/docregistry'; import { ArrayExt, find, IIterator, iter, toArray } from '@phosphor/algorithm'; @@ -539,6 +541,17 @@ export class LabShell extends Widget implements JupyterFrontEnd.IShell { this._onLayoutModified(); } + /** + * Dispose the shell. + */ + dispose(): void { + if (this.isDisposed) { + return; + } + this._layoutDebouncer.dispose(); + super.dispose(); + } + /** * Expand the left area. * @@ -923,16 +936,7 @@ export class LabShell extends Widget implements JupyterFrontEnd.IShell { * Handle a change to the layout. */ private _onLayoutModified(): void { - // The dock can emit layout modified signals while in transient - // states (for instance, when switching from single-document to - // multiple-document mode). In those states, it can be unreliable - // for the signal consumers to query layout properties. - // We fix this by debouncing the layout modified signal so that it - // is only emitted after rearranging is done. - window.clearTimeout(this._debouncer); - this._debouncer = window.setTimeout(() => { - this._layoutModified.emit(undefined); - }, 0); + void this._layoutDebouncer.invoke(); } /** @@ -963,6 +967,9 @@ export class LabShell extends Widget implements JupyterFrontEnd.IShell { private _dockPanel: DockPanel; private _isRestored = false; private _layoutModified = new Signal(this); + private _layoutDebouncer = new Debouncer(() => { + this._layoutModified.emit(undefined); + }, 0); private _leftHandler: Private.SideBarHandler; private _restored = new PromiseDelegate(); private _rightHandler: Private.SideBarHandler; @@ -970,7 +977,6 @@ export class LabShell extends Widget implements JupyterFrontEnd.IShell { private _headerPanel: Panel; private _topPanel: Panel; private _bottomPanel: Panel; - private _debouncer = 0; private _mainOptionsCache = new Map(); private _sideOptionsCache = new Map(); } From e74bba85a2736019492a5e236f0a7063ae4dba05 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 16:29:45 +0100 Subject: [PATCH 50/54] Use Debouncer in document search. --- packages/documentsearch/package.json | 1 + packages/documentsearch/src/searchoverlay.tsx | 25 +++++-------------- packages/documentsearch/tsconfig.json | 3 +++ 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/packages/documentsearch/package.json b/packages/documentsearch/package.json index bd1cc10ffa0b..830e215d2132 100644 --- a/packages/documentsearch/package.json +++ b/packages/documentsearch/package.json @@ -32,6 +32,7 @@ "@jupyterlab/cells": "^1.0.0-alpha.7", "@jupyterlab/codeeditor": "^1.0.0-alpha.6", "@jupyterlab/codemirror": "^1.0.0-alpha.6", + "@jupyterlab/coreutils": "^3.0.0-alpha.6", "@jupyterlab/fileeditor": "^1.0.0-alpha.6", "@jupyterlab/notebook": "^1.0.0-alpha.7", "@phosphor/coreutils": "^1.3.0", diff --git a/packages/documentsearch/src/searchoverlay.tsx b/packages/documentsearch/src/searchoverlay.tsx index 842bb52f898a..167d028fff02 100644 --- a/packages/documentsearch/src/searchoverlay.tsx +++ b/packages/documentsearch/src/searchoverlay.tsx @@ -4,6 +4,7 @@ import { IDisplayState } from './interfaces'; import { SearchInstance } from './searchinstance'; import { ReactWidget, UseSignal } from '@jupyterlab/apputils'; +import { Debouncer } from '@jupyterlab/coreutils'; import { Signal } from '@phosphor/signaling'; import { Widget } from '@phosphor/widgets'; import * as React from 'react'; @@ -252,7 +253,7 @@ class SearchOverlay extends React.Component< private _onSearchChange(event: React.ChangeEvent) { const searchText = (event.target as HTMLInputElement).value; this.setState({ searchText: searchText }); - this._debouncedStartSearch(true, searchText); + void this._debouncedStartSearch.invoke(); } private _onReplaceChange(event: React.ChangeEvent) { @@ -312,24 +313,6 @@ class SearchOverlay extends React.Component< this.props.onEndSearch(); } - private _debounce(func: Function, wait: number) { - const context = this; - let timeout: number; - return function(...args: any[]) { - const later = function() { - timeout = null; - return func.apply(context, args); - }; - clearTimeout(timeout); - timeout = setTimeout(later, wait); - }; - } - - private _debouncedStartSearch = this._debounce( - this._executeSearch.bind(this), - 100 - ); - private _onReplaceToggled() { this.setState({ replaceEntryShown: !this.state.replaceEntryShown @@ -430,6 +413,10 @@ class SearchOverlay extends React.Component< ]; } + + private _debouncedStartSearch = new Debouncer(() => { + this._executeSearch(true, this.state.searchText); + }, 100); } export function createSearchOverlay( diff --git a/packages/documentsearch/tsconfig.json b/packages/documentsearch/tsconfig.json index 51e546dcfb3c..fed14a468961 100644 --- a/packages/documentsearch/tsconfig.json +++ b/packages/documentsearch/tsconfig.json @@ -23,6 +23,9 @@ }, { "path": "../notebook" + }, + { + "path": "../coreutils" } ] } From 1cee0d76516f806a1199b6b028558c2dc4d74764 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 16:39:59 +0100 Subject: [PATCH 51/54] Update document search close logic to dispose debouncer, make sure onClose is called when escape key is pressed. --- packages/documentsearch/src/searchoverlay.tsx | 5 +++-- packages/documentsearch/tsconfig.json | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/documentsearch/src/searchoverlay.tsx b/packages/documentsearch/src/searchoverlay.tsx index 167d028fff02..50d7c86795a8 100644 --- a/packages/documentsearch/src/searchoverlay.tsx +++ b/packages/documentsearch/src/searchoverlay.tsx @@ -268,7 +268,7 @@ class SearchOverlay extends React.Component< } else if (event.keyCode === 27) { event.preventDefault(); event.stopPropagation(); - this.props.onEndSearch(); + this._onClose(); } } @@ -309,8 +309,9 @@ class SearchOverlay extends React.Component< } private _onClose() { - // clean up and close widget + // Clean up and close widget. this.props.onEndSearch(); + this._debouncedStartSearch.dispose(); } private _onReplaceToggled() { diff --git a/packages/documentsearch/tsconfig.json b/packages/documentsearch/tsconfig.json index fed14a468961..219ad7850eee 100644 --- a/packages/documentsearch/tsconfig.json +++ b/packages/documentsearch/tsconfig.json @@ -19,13 +19,13 @@ "path": "../codemirror" }, { - "path": "../fileeditor" + "path": "../coreutils" }, { - "path": "../notebook" + "path": "../fileeditor" }, { - "path": "../coreutils" + "path": "../notebook" } ] } From 5cec89850a6fa23f323073745e2352875fb325e3 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Wed, 22 May 2019 16:59:59 +0100 Subject: [PATCH 52/54] Remove superfluous async. --- packages/coreutils/src/ratelimiter.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/coreutils/src/ratelimiter.ts b/packages/coreutils/src/ratelimiter.ts index 9feffe691f0d..c7893697c466 100644 --- a/packages/coreutils/src/ratelimiter.ts +++ b/packages/coreutils/src/ratelimiter.ts @@ -30,7 +30,7 @@ export abstract class RateLimiter implements IRateLimiter { standby: 'never' }); this.payload = new PromiseDelegate(); - this.poll.ticked.connect(async (_, state) => { + this.poll.ticked.connect((_, state) => { const { payload } = this; if (state.phase === 'resolved') { From 2bb58566a22e7c483cda020ed2dd78a4b6c9ca87 Mon Sep 17 00:00:00 2001 From: "Afshin T. Darian" Date: Thu, 23 May 2019 12:06:25 +0100 Subject: [PATCH 53/54] Update state database interface from `master`. --- packages/coreutils/src/tokens.ts | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/packages/coreutils/src/tokens.ts b/packages/coreutils/src/tokens.ts index e23b2baf3e93..b6a118563cd7 100644 --- a/packages/coreutils/src/tokens.ts +++ b/packages/coreutils/src/tokens.ts @@ -463,21 +463,6 @@ export const IStateDB = new Token('@jupyterlab/coreutils:IStateDB'); */ export interface IStateDB extends IDataConnector { - /** - * The maximum allowed length of the data after it has been serialized. - */ - readonly maxLength: number; - - /** - * The namespace prefix for all state database entries. - * - * #### Notes - * This value should be set at instantiation and will only be used - * internally by a state database. That means, for example, that an - * app could have multiple, mutually exclusive state databases. - */ - readonly namespace: string; - /** * Return a serialized copy of the state database's entire contents. * From b58d3816196d13b1f57a49f536b337d80e329f78 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Thu, 23 May 2019 09:25:02 -0500 Subject: [PATCH 54/54] trim memory usage in examples tests --- examples/chrome-example-test.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/examples/chrome-example-test.js b/examples/chrome-example-test.js index 98113f9a577c..eb32d4d6963f 100644 --- a/examples/chrome-example-test.js +++ b/examples/chrome-example-test.js @@ -10,7 +10,11 @@ const URL = process.argv[2]; async function main() { console.info('Starting Chrome Headless'); - const browser = await puppeteer.launch({ args: ['--no-sandbox'] }); + // Disable shm usage to save resources and prevent random memory + // errors seen on CI + const browser = await puppeteer.launch({ + args: ['--no-sandbox', '--disable-dev-shm-usage'] + }); const page = await browser.newPage(); console.info('Navigating to page:', URL);