Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve discovery and description of JupyterLab commands #13962

Open
bollwyvl opened this issue Feb 10, 2023 · 9 comments
Open

Improve discovery and description of JupyterLab commands #13962

bollwyvl opened this issue Feb 10, 2023 · 9 comments

Comments

@bollwyvl
Copy link
Contributor

bollwyvl commented Feb 10, 2023

Problem

JupyterLab commands are at once one of the most powerful, but also one of the most weakly-typed, and ultimately, least discoverable parts of the overall architecture.

Once a command id has actually been discovered (by DOM, source code, or debugger inspection without source maps), it is then sometimes only possible to determine the appropriate args by fully tracing the source code from call site to ultimate implementation, as these are sometimes passed as any down into another part of the application.

Proposed Solution

  • add builder/metadata_schema.json#/properties/commands
  • for each command, allow the definition of any of the static members of ICommandOptions, but require:
    • #/describedBy/args
      • as a JSON Schema, require a title and description
    • id
      • while the id would ultimately be the correct key for this object, having an arbitrary (but unique-per-package) string would allow for transparently shimming existing exported CommandIds
  • move all core CommandIds and hard-coded strings to the location where they are defined (not necessarily registered)
    • import the package.json at run/build time
      • in a composite build this... complicates things
        • each packages/*/src would need a tsconfig.json that references: {path: ../}
  • use the above to also provide build-time typescript types for all args with ajv

The payoff for this to be able to statically derive, outside of :

  • always-accurate generated static HTML documentation, which can be linked to and referenced
  • improved in-application documentation
    • Inspector/Contextual Help
    • discoverable forms a la Offer JsonSchemaForm based on rjsf #13951
      • this would work great with the sidebar command palette, but the modal one would be... not great
      • i guess it could launch layers of blocking modals 🤢

This technique could then be encouraged in extension-cookiecutter-ts, documentation, etc.

Alternatives

  • this could alternately be defined in the settings schema
    • this could theoretically allow for reusing definitions...
    • but this might require adding schema folders for packages that don't/can't currently have them

Additional context

@krassowski
Copy link
Member

Once a command id has actually been discovered (by DOM, source code, or debugger inspection without source maps),

For what its worth: https://jupyterlab.readthedocs.io/en/latest/user/commands.html#commands-list but this does not give us commands registered in extensions.

I do not understand where you propose the actual JSON command to be defined. package.json?

There are more extensions that could benefit from having this information at runtime:

@krassowski
Copy link
Member

krassowski commented Feb 12, 2023

An alternative wold be to discover this automatically by writing a TypeScript plugin with a pass-through transformer that would detect addCommand calls and dump the args type to declarations/JSON file. Thoughts?

Edit: this kind of thing: https://github.com/Microsoft/TypeScript/wiki/Using-the-Compiler-API#using-the-type-checker

@bollwyvl
Copy link
Contributor Author

package.json?

Yep. So, for example, this CommandIds would turn into an entry in package.json.

{
  "name":  "name": "@jupyterlab/filebrowser-extension",
  // ...
  "jupyterlab": {
    "extension": true,
    "schemaDir": "schema",
    "commands": {
      // ...
      "showBrowser": {
        "id": "filebrowser:activate",
        "options": {
          "describedBy": {
            "args": {
              "description": "Open the file browser for the provided `path`.",
              "type": "object",
              "properties": {
                "path": {
                  "description": "the path to open",
                  "format": "uri",
                  "type": "string"
                }
              }
            }
          }
        },
      },
      // ...
    }
  }
}

Or, as in the alternative, it could go in the plugin under a new key, jupyter.lab.commands.

Either way, the CommandIds would get exposed back in typescript with e.g.

import * as PACKAGE from "../package.json";
const Commands = PACKAGE.jupyterlab.commands;

And then installed, probably with a helper which could hoist description to label (or caption, or usage, or whatever) and translate it:

import { describeCommand } from `@jupyterlab/apputils`;
// ...
commands.addCommand(
  Commands.showBrowser.id, 
  describeCommand(
    translator,
    {
      ...Commands.showBrowser.options,
      activate: (args) => {...}
    }
  )
);

@bollwyvl
Copy link
Contributor Author

TypeScript plugin

Sure, that's a way to go, too. Got the same argument from the LSP folk who invented a whole new metamodel for describing stuff. Cue xkcd link.

I'm still of the mind, for Jupyter, that metadata in an established, language-neutral format in a well-known (or at least describable) locations wins, especially when typescript can natively import it, with decent typing. grep and jq (or literally any other language) could tear through all this stuff without nodejs and typescript.

@bollwyvl
Copy link
Contributor Author

dump the args type to declarations/JSON file.

Right, the issue is that today they aren't described (basically all any), and sometimes just handed off to somewhere else.

Writing schema-in-TS is of course possible, and could even be quite nice with authoring-type "magic" schema-to-ts by e.g. ajv (without actually importing ajv everywhere).

@vidartf
Copy link
Member

vidartf commented Mar 29, 2023

Should we alternatively make it easier for extension authors to expose their standard commands on token inferfaces? I don't think that will be more work than speccing out the args in a schema.

@bollwyvl
Copy link
Contributor Author

more work than speccing out the args in a schema.

See all the comments above: the intent is to make the knowledge of commands less driven by jupyterlab-specific implementation details in TypeScript, and more for having a concrete, declarative, human-readable definition in a well-known location.

Further, schema provides the ability to constrain things beyond what an IArgs per command could, e.g. "what the heck is an integer, much less a positive integer?"

on token

So this codebase could, by example, expose a command set as a ISomeExtensionCommands. But each of these plugins can be directly derived through the structure of the static schema, with as much specificity as the type system allows. This would allow downstreams to use the typeof ISomeExtensionCommands.shortCutName.describedBy.args to improve their static typing, without breaking existing uses of the "loosely coupled" approach that works today without every package having to declare a hard dependency on every other package.

But somewhere down the line, these describedBy should become validated at runtime, with some kind of schedule like:

  • 4.x, add an opt-in feature flag (e.g. in PageConfig) for commandValidation: 'off' | 'warn' | 'fail'
    • emit warnings with stack trace (to get the call site)
  • 4.x+2 the default becomes warn
    • for a transitional period
    • deployers can manually turn it off
  • 4.x+4 (if it comes to it) the default becomes fail
  • 5.x the feature flag is removed, and all validation errors are thrown

@Mr-Ruben
Copy link

Here you can see the pain of 'command discovery' in order to edit the settings (cell toolbar).

jupyterlab-how-to-insert-custom-elements-buttons-commands-in-the-cell-toolbar

If there is an easier way to get the list of available commands, I would really love to hear it.

@krassowski
Copy link
Member

@Mr-Ruben today:

But for the use case you are highlighting, the commands should really be a list/dropdown in the settings form.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants