Dynamic extensions reloading: IPlugin.deactivate, Application.registerPlugin and Application.deactivatePlugin proposal

[krassowski]

Problem

The dynamic extensions provided by jupyterlab-plugin-playground cannot be registered more than once with the same id. This forces the user to reload the page every time, or at least change the id each time they activate the plugin, which is really annoying for the prototyping workflows as intended for the plugin playground.

Neither normal JupyterLab/Lumino extensions can be unloaded on runtime. It might be by design - maybe there was an assumption that reloading websites is cheap - but the cost is suboptimal for some more recent use cases like:

• JupyterLab Desktop where refresh = close and reopen app
• JupyterLite = the bundle weights much more and kernels will take long to load

To allow to reload extension with new updated code we need to unload it first. We could remove the plugin manually from the plugin map (inelegantly accessing private properties of Application, which is what is currently done in jupyterlab/jupyterlab-plugin-playground#28) but when we register module anew, the old one remains active, with all of its commands, widgets, toolbars and menus attached.

Proposed Solution

There should be three additions to the API are needed:

• Application.unregisterPlugin() method to remove the plugin from the plugin map and complement the existing registerPlugin()
• an optional deactivate method which plugins may decide to implement. If it is provided, it should undo all the changes the activate method made, including removing the added commands, menus, etc. to complement the current activate() method.
• Application.deactivatePlugin method to call the deactivate method on the plugin and all of it dependants

The unregisterPlugin would roughly be:

/**
 * @param id - id of the plugin to unregister
 * @param force - whether to unregister even if active
 */
async unregisterPlugin(id: str, force?: boolean) {
  const data = this._pluginMap[id];
  if (data.activated && !force) {
     reject('plugin is still active');
  }
  delete this._pluginMap[plugin.id];
}

deactivatePlugin would reject if plugin does not have deactivate() method:

/**
 * Will deactivate the plugin and its dependant if and only if the plugin
 * and its dependants all support `deactivate`.
 * @returns other dependant plugins deactivated as a consequence
 */
deactivatePlugin(id: string): IPlugin[] {
  if (!plugin.deactivate) {
     reject('This plugin does not support deactivation');
  }
  // find an optimal deactivation order for dependants, probably some DAG flattened to an array
  const pluginDependantsInDAGOrder = this._findDependants();
  for (const dependant of pluginDependantsInDAGOrder) {
     if (!dependant.deactivate) {
         reject('Plugin dependant does not support deactivation: ' + dependant.id)
     }
  }
  for (const dependant of pluginDependantsInDAGOrder) {
     await this.deactivatePlugin(dependant.id);
  }
  await plugin.deactivate()
  return pluginDependantsDAG;
}

It will be up to plugin authors to adopt the deactivate method. This would be a backwards-compatible change.

The signature of deactivate could be:

deactivate?: (app: T, ...args: any[]) => void | Promise<void>; 

Because plugins are also service providers we would want to unregister the associated service after the deactivate() call completed (allowing the plugin to cleanly shut the service down to avoid memory leaks), thus we want to allow the plugins to return a promise.

If deactivate is not present the service provided by the plugin should never be removed.

Questions:

• I think that the return type should be void because we would know what the associated service is; any other takes?
• what should happen if deactivate() is present but throws or rejects? Should we still unregister the service?

Additional context

• VSCode Extensions API have both activate and deactivate functions as entry points. The deactivate() method is optional in VSCode.
• InteliJ Platform Plugin SDK has plugin load/unload events for its dynamic extensions; if I read the documentation correctly it also disallows use of any entry points which cannot by loaded/unloaded dynamically for dynamic plugins

Adding a deactivate which enables hot reloading of dynamic extensions is a pre-requisite for using plugin-playground (also cf jupyterlab/jupyterlab#8866) to develop extensions inside JupyterLab itself (jupyterlab/jupyterlab#7469).

Finally, deregistration wrapper will also need to be added downstream in JupyterLab where registerPluginModule is implemented.

[krassowski]

I extended the proposal to Application.registerPlugin, Application.deactivatePlugin which would also need to be implemented in lumino (and not as I previously thought in JupyterLab).

[jasongrout]

Overall, I think this is an excellent idea and would love to see it in Lumino/JLab. Thanks for proposing it!

maybe there was an assumption that reloading websites is cheap -

Yes, there was the assumption in JupyterLab that reload was at least cheap enough to not justify the effort at the time. Thanks for showing that in certain cases it would be very valuable.

The unregisterPlugin would roughly be:

Should unregisterPlugin also call deactivate if the plugin is activated? I think so, so I'm curious why you didn't choose that.

  // find an optimal deactivation order for dependants, probably some DAG flattened to an array
  const pluginDependantsInDAGOrder = this._findDependants();

There is a topological sort in Lumino that it uses for activation that we could use.

what should happen if deactivate() is present but throws or rejects? Should we still unregister the service?

If the unregister is calling deactivate, perhaps that is part of what the force option would do - if force is false, it would throw the error, and if force was true, it would still unregister the plugin, possibly leaving your system in an inconsistent state.

Application.deactivatePlugin which would also need to be implemented in lumino (and not as I previously thought in JupyterLab).

+1

Also, we should check that the following scenario works:

1. plugin A depends on plugin B. Both plugins have a deactivate method.
2. plugin B is unregistered (causing plugin A to deactivate, but not unregister)
3. plugin B is registered again.

Does plugin A get activated again automatically by the system, with the new B? What happens in the four combinations of auto-activation settings for plugin A and plugin B?

[krassowski]

Should unregisterPlugin also call deactivate if the plugin is activated? I think so, so I'm curious why you didn't choose that.

It was my initial thought too but I decided not to include deactivate in unregister there for two reasons:

• the opposite registerPlugin does not activate plugins - only registers
• in jupyterlab-plugin-playground I want to call deactivatePlugin first, handle any errors (and show them to the user/developer) and then force unregister the plugin even if deactivation failed so the new version can be registered even if the initial version had errors. Indeed we could still preserve the force aspect without the separation, but errors from the deactivation would need to be returned rather than caught increasing the cognitive complexity of the error handling logic.

what should happen if deactivate() is present but throws or rejects? Should we still unregister the service?

If the unregister is calling deactivate, perhaps that is part of what the force option would do - if force is false, it would throw the error, and if force was true, it would still unregister the plugin, possibly leaving your system in an inconsistent state.

Right, under the revised proposal this question is easier to answer because unregister does not call deactivate. I think that this answers itself: there are now two separate functions and it's the responsibility of the downstream to handle this concern:

• in plugin-playground we will ignore deactivate failures,
• in case of a potential future JupyterLab dynamic extension system (other than the playground) we might refuse to unregister if deactivate fails)

plugin B is unregistered (causing plugin A to deactivate, but not unregister)

I think that under the current proposal this will not happen.

What happens in the four combinations of auto-activation settings for plugin A and plugin B?

This is a good question; again I don't think it is a problem when deactive and unregister are separated. Thinking about it brought me another question (which is more about expectations and documentation rather than the code itself I think):

• should the expectation for downstream applications be that deactivate gets called:
  1. when user requests it only, (this is what my proposal focused on so dat) or
  2. when user requests it, and when the application closes

I think the answer is (2) as it would make this method more useful and enable better resource management for the extension developers (if they spawn any external resources, etc - they will no longer need to rely on window events like onbeforeunload etc).

[vidartf]

While I do think that it is worth thinking through the dependency tree of deactivations etc., a first step might be to only allow for the deactivation (and unregistering?) of plugins that are not depended upon by other plugins. I.e. only leaf nodes in the dependency tree. We can future-proof it by documenting that "if you try to deactivate a plugin that is depended on by another plugin, this will currently throw an exception, but this might change in the future to allow deactivation of a plugin and all of its dependents, so do not rely on this feature."

[jtpio]

In addition to this it could also be useful to allow for more app introspection, for example to check if a plugin is activated (and has not failed to activate), which could be exposed via a app.isPluginActivated(id: string)

[fcollonval]

What about using a generator for the activate function that returns [Token?, deactivate function?] instead of a separate deactivate.

The idea behind using a generator is linked to the actions the deactivate method will need to carry; aka mainly disposing created resources and disconnecting signals.

using separated deactivate

const plugin = {
  provides: IMyToken,
  activate: (app): IMyToken => {
     const john = new MyAwesomeDisposable();

     const onAppRestored = () => {
       // no-op
       john.initialize()
     }
     app.restored.connect(onAppRestored);

     return john;
  }
  deactivate: () => {
    // How to disconnect the signal
    // How to dispose john
    // => the solution will probably to create a class implementing IPlugin rather than a dictionary
  }
}

using generator

const plugin = {
  provides: IMyToken,
  activate: *(app): Iterable<IMyToken | (() => void)> => {
     const john = new MyAwesomeDisposable();

     const onAppRestored = () => {
       // no-op
       john.initialize()
     }
     app.restored.connect(onAppRestored);

     yield john;

     const deactivate = () => {
       app.restored.disconnect(onAppRestored);
       john.dispose();
     }
     yield deactivate;
  }
}

[krassowski]

Indeed I was thinking extension equals a class.

The generator would be another pattern on top of IDisposable. I would be inclined towards a separate deactivate but I don't have a strong feeling; rationale below:

In the use-case of plugin-playground (which in my mind is meant to be a good first experience for powerusers who may not have had an experience with TypeScript) having deactivate in every example is essential, so using it would now also require learning about generators in TS. I also somewhat dislike having the sequence of yields which has to be done in a very specific order.

What we could do instead is:

class MyAwesomeDisposableExtension implements IMyToken {
  constructor (restored: ISignal) {
     this._restored = restored;
     this._restored.connect(this._onAppRestored, this);
  }

  doX() {
    // no-op
  }

  dispose() {
      this._restored.disconnect(this._onAppRestored, this);
  }

  private _onAppRestored(): {
    this._initialize()
  }

  private _initialize() {
    // no-op
  }
}

const plugin = {
  provides: IMyToken,
  requires: [ISomethingRegistry],
  activate: (app, somethingRegistry: ISomethingRegistry): IMyToken => {
     const john = new MyAwesomeDisposableExtension(app.restored);
     somethingRegistry.register('john-does-x', john.doX)
     return john;
  },
  // what if `provides` is empty? give a null here? or should it be the first argument?
  deactivate: (app, john: IMyToken, somethingRegistry: ISomethingRegistry) => {
    somethingRegistry.unregister('john-does-x');
    john.dispose();
  }
}

Note that in the generators example, john being a disposable holds state anyways (so my example could possibly be rewritten to a set of closures).

[fcollonval]

As you commented,

// what if `provides` is empty? give a null here? or should it be the first argument?
  deactivate: (app, john: IMyToken, somethingRegistry: ISomethingRegistry)

the trouble is that it will be hard to know what to provide as input to deactivate. I would rather prefer a plugin class pattern:

const plugin = new (class extends JupyterFrontEndPlugin<IMyToken> {
  readonly provides = IMyToken,
  readonly requires = [ISomethingRegistry],
  activate(app, somethingRegistry: ISomethingRegistry): IMyToken {
     this._john = new MyAwesomeDisposableExtension();

     app.restored.connect(onAppRestored, this);

     return john;
  }

  deactivate(app, somethingRegistry: ISomethingRegistry){
    app.restored.disconnect(onAppRestored, this);
    this._john.dispose();
    this._john = null;
  }

  onAppRestored() {
     this._john.initialize()
  }

  private _john: MyAwesomeDisposableExtension | null = null;
})();