Skip to content

Commit

Permalink
Port c599dca to 8.x: Introduce synchronous has_model function in the …
Browse files Browse the repository at this point in the history 
…manager base class

From jupyter-widgets#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.
  • Loading branch information
@jasongrout
jasongrout committed Feb 18, 2022
1 parent 92632cb commit 3245912
Showing 1 changed file with 47 additions and 71 deletions.
118 changes: 47 additions & 71 deletions packages/base-manager/src/manager-base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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 };
})
);
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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<Private.ICommUpdateData>();
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<Private.ICommUpdateData>();
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;
})
);

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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<IManagerStateMap>((res, model_id) => {
res[model_id] = models[model_id];
return res;
Expand Down

0 comments on commit 3245912

Please sign in to comment.