From 3245912322141b431f2530c9ca66908de9d562d8 Mon Sep 17 00:00:00 2001 From: Jason Grout Date: Thu, 17 Feb 2022 21:17:23 -0800 Subject: [PATCH] Port c599dca9ba to 8.x: Introduce synchronous has_model function in the manager base class From #3377. This simplifies our code that has had to work around get_model either returning undefined or a rejected promise if a model was not registered. This paves the way for a future get_model in 8.0 that is truly asynchronous, never returning undefined. --- packages/base-manager/src/manager-base.ts | 118 +++++++++------------- 1 file changed, 47 insertions(+), 71 deletions(-) diff --git a/packages/base-manager/src/manager-base.ts b/packages/base-manager/src/manager-base.ts index e1c95269f1..b9157cc1b3 100644 --- a/packages/base-manager/src/manager-base.ts +++ b/packages/base-manager/src/manager-base.ts @@ -221,6 +221,16 @@ export abstract class ManagerBase implements IWidgetManager { return this._models[model_id]; } + /** + * Returns true if the given model is registered, otherwise false. + * + * #### Notes + * This is a synchronous way to check if a model is registered. + */ + has_model(model_id: string): boolean { + return this._models[model_id] !== undefined; + } + /** * Handle when a comm is opened. */ @@ -443,24 +453,9 @@ export abstract class ManagerBase implements IWidgetManager { // Create comms for all new widgets. const widget_comms = await Promise.all( Object.keys(states).map(async (widget_id) => { - let comm = undefined; - let modelPromise = undefined; - try { - modelPromise = this.get_model(widget_id); - if (modelPromise === undefined) { - comm = await this._create_comm('jupyter.widget', widget_id); - } else { - // For JLab, the promise is rejected, so we have to await to - // find out if it is actually a model. - await modelPromise; - } - } catch (e) { - // The JLab widget manager will throw an error with this specific error message. - if (e.message !== 'widget model not found') { - throw e; - } - comm = await this._create_comm('jupyter.widget', widget_id); - } + const comm = this.has_model(widget_id) + ? undefined + : await this._create_comm('jupyter.widget', widget_id); return { widget_id, comm }; }) ); @@ -477,11 +472,7 @@ export abstract class ManagerBase implements IWidgetManager { ); } try { - if (comm === undefined) { - // model already exists here - const model = await this.get_model(widget_id); - model!.set_state(state.state); - } else { + if (comm) { // This must be the first await in the code path that // reaches here so that registering the model promise in // new_model can register the widget promise before it may @@ -496,6 +487,10 @@ export abstract class ManagerBase implements IWidgetManager { }, state.state ); + } else { + // model already exists here + const model = await this.get_model(widget_id); + model!.set_state(state.state); } } catch (error) { // Failed to create a widget model, we continue creating other models so that @@ -518,52 +513,35 @@ export abstract class ManagerBase implements IWidgetManager { // For each comm id that we do not know about, create the comm, and request the state. const widgets_info = await Promise.all( Object.keys(comm_ids).map(async (comm_id) => { - try { - const model = this.get_model(comm_id); - // TODO Have the same this.get_model implementation for - // the widgetsnbextension and labextension, the one that - // throws an error if the model is not found instead of - // returning undefined - if (model === undefined) { - throw new Error('widget model not found'); - } - await model; - // If we successfully get the model, do no more. + if (this.has_model(comm_id)) { return; - } catch (e) { - // If we have the widget model not found error, then we can create the - // widget. Otherwise, rethrow the error. We have to check the error - // message text explicitly because the get_model function in this - // class throws a generic error with this specific text. - if (e.message !== 'widget model not found') { - throw e; + } + + const comm = await this._create_comm(this.comm_target_name, comm_id); + + let msg_id = ''; + const info = new PromiseDelegate(); + comm.on_msg((msg) => { + if ( + (msg.parent_header as any).msg_id === msg_id && + msg.header.msg_type === 'comm_msg' && + msg.content.data.method === 'update' + ) { + const data = msg.content.data as any; + const buffer_paths = data.buffer_paths || []; + const buffers = msg.buffers || []; + utils.put_buffers(data.state, buffer_paths, buffers); + info.resolve({ comm, msg }); } - const comm = await this._create_comm(this.comm_target_name, comm_id); - - let msg_id = ''; - const info = new PromiseDelegate(); - comm.on_msg((msg) => { - if ( - (msg.parent_header as any).msg_id === msg_id && - msg.header.msg_type === 'comm_msg' && - msg.content.data.method === 'update' - ) { - const data = msg.content.data as any; - const buffer_paths = data.buffer_paths || []; - const buffers = msg.buffers || []; - utils.put_buffers(data.state, buffer_paths, buffers); - info.resolve({ comm, msg }); - } - }); - msg_id = comm.send( - { - method: 'request_state', - }, - this.callbacks(undefined) - ); + }); + msg_id = comm.send( + { + method: 'request_state', + }, + this.callbacks(undefined) + ); - return info.promise; - } + return info.promise; }) ); @@ -724,8 +702,8 @@ export abstract class ManagerBase implements IWidgetManager { // If the model has already been created, set its state and then // return it. - if (this._models[model_id] !== undefined) { - return this._models[model_id].then((model) => { + if (this.has_model(model_id)) { + return this.get_model(model_id)!.then((model) => { // deserialize state return (model.constructor as typeof WidgetModel) ._deserialize_state(modelState || {}, this) @@ -876,9 +854,7 @@ export abstract class ManagerBase implements IWidgetManager { protected filterExistingModelState(serialized_state: any): any { let models = serialized_state.state; models = Object.keys(models) - .filter((model_id) => { - return !this._models[model_id]; - }) + .filter((model_id) => !this.has_model(model_id)) .reduce((res, model_id) => { res[model_id] = models[model_id]; return res;