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

Request for best practices/guidance on creating stable extensions which aren't version-dependent #13971

Open
rickmcgeer opened this issue Feb 13, 2023 · 17 comments

Comments

@rickmcgeer
Copy link

Problem

  • We maintain an extension that offers drag-and-drop creation of dashboards and visual web applications in JupyterLab (the repo is at git@github.com:engageLively/jupyterlab-universal-extension.git, open-source under a BSD 3-clause license). The extension itself is quite simple: it just manages file I/O and access for an external drag-and-drop editor that is instantiated in a JupyterLab tab. Our problem is that the extension breaks with every new release of JupyterLab, largely because the underlying API (CodeEditor.Model, DocumentRegistry.ICodeModel) changes. For example, the current version works very well with JupyterLab 3.5, but fails with 3.6.1 -- we believe because the YFile class has moved.

Obviously, we'd prefer to implement an extension that isn't version-dependent. Some version dependency will always be there, of course, but ATM we're tweaking with each minor-version release.

Proposed Solution

Our internal diagnosis is that we're likely extending the wrong classes to implement our solution. However, we've been unable to find in the documentation, or the extension examples, preferred methods of reading and writing files, or the contract that a CodeEditor.Model subclass should support.
We (and we suspect, others who write editor extensions) would really appreciate guidance on this, ideally with a very simple example extension. We're prepared to contribute our extension, or a simplified version of it, to the extension-examples repo to help people out with this. But, first, we need to make it robust, and we'd appreciate guidance on how to do that.

Additional context

@jupyterlab-probot jupyterlab-probot bot added the status:Needs Triage Applied to new issues that need triage label Feb 13, 2023
@fcollonval fcollonval added this to the 3.6.x milestone Feb 14, 2023
@fcollonval
Copy link
Member

Thanks @rickmcgeer

This is really unfortunate and the goal was to still be backward compatible even with the RTC changes.

In the case of the failure with 3.6.1, would you be able to share some error traces?


I think the way to restore backward compatible is to restore @jupyterlab/shared-models as a facade to @jupyter/ydoc.

cc @jtpio @afshin @hbcarlos

@fcollonval
Copy link
Member

For reference to compare the changes: 3.5.x...v3.6.1

@rickmcgeer
Copy link
Author

@fcollonval thanks for the kind words. Here's the trace. This was under JupyterLab 3.6.1

jlpm run build
yarn run v1.21.1
$ jlpm build:lib && jlpm build:labextension:dev
$ tsc
node_modules/@jupyterlab/docregistry/lib/default.d.ts:15:22 - error TS2420: Class 'DocumentModel' incorrectly implements interface 'ICodeModel'.
  Types of property 'mimeTypeChanged' are incompatible.
    Type 'ISignal<this, IChangedArgs<string, string, string>>' is not assignable to type 'ISignal<IModel, IChangedArgs<string, string, string>>'.
      Type 'this' is not assignable to type 'IModel'.
        Type 'DocumentModel' is not assignable to type 'IModel'.
          Types of property 'sharedModel' are incompatible.
            Type 'ISharedFile' is not assignable to type 'ISharedText'.

15 export declare class DocumentModel extends CodeEditor.Model implements DocumentRegistry.ICodeModel {
                        ~~~~~~~~~~~~~

node_modules/@jupyterlab/docregistry/lib/default.d.ts:90:14 - error TS2416: Property 'sharedModel' in type 'DocumentModel' is not assignable to the same property in base type 'Model'.
  Type 'ISharedFile' is not assignable to type 'ISharedText'.

90     readonly sharedModel: models.ISharedFile;
                ~~~~~~~~~~~

node_modules/@jupyterlab/docregistry/lib/registry.d.ts:353:15 - error TS2430: Interface 'ICodeModel' incorrectly extends interface 'IModel'.
  Types of property 'sharedModel' are incompatible.
    Property 'source' is missing in type 'ISharedFile' but required in type 'ISharedText'.

353     interface ICodeModel extends IModel, CodeEditor.IModel {
                  ~~~~~~~~~~

  node_modules/@jupyter/ydoc/lib/api.d.ts:93:5
    93     source: string;
           ~~~~~~
    'source' is declared here.

node_modules/@jupyterlab/notebook/lib/model.d.ts:12:18 - error TS2430: Interface 'INotebookModel' incorrectly extends interface 'IModel'.
  Types of property 'sharedModel' are incompatible.
    Type 'ISharedNotebook' is missing the following properties from type 'ISharedDocument': state, getState, setState

12 export interface INotebookModel extends DocumentRegistry.IModel {
                    ~~~~~~~~~~~~~~

node_modules/@jupyterlab/notebook/lib/modelfactory.d.ts:9:85 - error TS2344: Type 'INotebookModel' does not satisfy the constraint 'IModel'.

9 export declare class NotebookModelFactory implements DocumentRegistry.IModelFactory<INotebookModel> {
                                                                                      ~~~~~~~~~~~~~~

node_modules/@jupyterlab/notebook/lib/panel.d.ts:14:69 - error TS2344: Type 'INotebookModel' does not satisfy the constraint 'IModel'.
  Types of property 'sharedModel' are incompatible.
    Type 'ISharedNotebook' is not assignable to type 'ISharedDocument'.

14 export declare class NotebookPanel extends DocumentWidget<Notebook, INotebookModel> {
                                                                       ~~~~~~~~~~~~~~

node_modules/@jupyterlab/notebook/lib/panel.d.ts:18:60 - error TS2344: Type 'INotebookModel' does not satisfy the constraint 'IModel'.

18     constructor(options: DocumentWidget.IOptions<Notebook, INotebookModel>);
                                                              ~~~~~~~~~~~~~~

node_modules/@jupyterlab/notebook/lib/widgetfactory.d.ts:12:69 - error TS2344: Type 'NotebookPanel' does not satisfy the constraint 'IDocumentWidget<Widget, IModel>'.
  Types of property 'context' are incompatible.
    Type 'IContext<INotebookModel>' is not assignable to type 'IContext<IModel>'.
      Type 'INotebookModel' is not assignable to type 'IModel'.

12 export declare class NotebookWidgetFactory extends ABCWidgetFactory<NotebookPanel, INotebookModel> {
                                                                       ~~~~~~~~~~~~~

node_modules/@jupyterlab/notebook/lib/widgetfactory.d.ts:44:66 - error TS2344: Type 'INotebookModel' does not satisfy the constraint 'IModel'.

44     protected createNewWidget(context: DocumentRegistry.IContext<INotebookModel>, source?: NotebookPanel): NotebookPanel;
                                                                    ~~~~~~~~~~~~~~

node_modules/@jupyterlab/notebook/lib/widgetfactory.d.ts:90:64 - error TS2344: Type 'NotebookPanel' does not satisfy the constraint 'IDocumentWidget<Widget, IModel>'.

90     interface IFactory extends DocumentRegistry.IWidgetFactory<NotebookPanel, INotebookModel> {
                                                                  ~~~~~~~~~~~~~

node_modules/@jupyterlab/translation/node_modules/@jupyterlab/services/lib/event/index.d.ts:89:52 - error TS2583: Cannot find name 'AsyncIterable'. Do you need to change your target library? Try changing the `lib` compiler option to 'es2018' or later.

89     interface IStream<T, U> extends ISignal<T, U>, AsyncIterable<U> {
                                                      ~~~~~~~~~~~~~

src/editor.ts:13:3 - error TS2344: Type 'GalyleoEditor' does not satisfy the constraint 'Widget'.

13   GalyleoEditor,
     ~~~~~~~~~~~~~

src/index.ts:49:14 - error TS2420: Class 'GalyleoModel' incorrectly implements interface 'ICodeModel'.
  Types of property 'mimeTypeChanged' are incompatible.
    Type 'ISignal<this, IChangedArgs<string, string, string>>' is not assignable to type 'ISignal<IModel, IChangedArgs<string, string, string>>'.
      Type 'this' is not assignable to type 'IModel'.
        Type 'GalyleoModel' is not assignable to type 'IModel'.
          The types of 'sharedModel.source' are incompatible between these types.
            Type 'YText' is not assignable to type 'string'.

49 export class GalyleoModel
                ~~~~~~~~~~~~

src/index.ts:151:3 - error TS2416: Property 'createNew' in type 'GalyleoModelFactory' is not assignable to the same property in base type 'TextModelFactory'.
  Type '(languagePreference?: string | undefined, modelDb?: IModelDB | undefined) => GalyleoModel' is not assignable to type '(languagePreference?: string | undefined, modelDB?: IModelDB | undefined, isInitialized?: boolean | undefined) => ICodeModel'.
    Call signature return types 'GalyleoModel' and 'ICodeModel' are incompatible.
      The types of 'mimeTypeChanged' are incompatible between these types.
        Type 'ISignal<GalyleoModel, IChangedArgs<string, string, string>>' is not assignable to type 'ISignal<IModel, IChangedArgs<string, string, string>>'.
          Type 'GalyleoModel' is not assignable to type 'IModel'.
            Types of property 'sharedModel' are incompatible.
              Type 'YFile' is not assignable to type 'ISharedText'.

151   createNew(
      ~~~~~~~~~

src/index.ts:174:3 - error TS2344: Type 'GalyleoDocument' does not satisfy the constraint 'IDocumentWidget<Widget, IModel>'.
  Types of property 'content' are incompatible.
    Property '_toggleHidden' is missing in type 'GalyleoEditor' but required in type 'Widget'.

174   GalyleoDocument,
      ~~~~~~~~~~~~~~~

  node_modules/@jupyterlab/docregistry/node_modules/@lumino/widgets/types/widget.d.ts:382:13
    382     private _toggleHidden;
                    ~~~~~~~~~~~~~
    '_toggleHidden' is declared here.

src/index.ts:337:52 - error TS2345: Argument of type 'import("/workspaces/jupyterlab-universal-extension/node_modules/@jupyterlab/translation/node_modules/@jupyterlab/services/lib/session/manager").SessionManager' is not assignable to parameter of type 'import("/workspaces/jupyterlab-universal-extension/node_modules/@jupyterlab/services/lib/session/manager").SessionManager'.
  Property 'requestRunning' is protected but type 'SessionManager' is not a class derived from 'SessionManager'.

337     commsManager: new GalyleoCommunicationsManager(sessionManager),
                                                       ~~~~~~~~~~~~~~

src/index.ts:476:20 - error TS2345: Argument of type 'import("/workspaces/jupyterlab-universal-extension/node_modules/@lumino/widgets/types/menu").Menu' is not assignable to parameter of type 'import("/workspaces/jupyterlab-universal-extension/node_modules/@jupyterlab/docregistry/node_modules/@lumino/widgets/types/menu").Menu'.
  Property '_toggleHidden' is missing in type 'import("/workspaces/jupyterlab-universal-extension/node_modules/@lumino/widgets/types/menu").Menu' but required in type 'import("/workspaces/jupyterlab-universal-extension/node_modules/@jupyterlab/docregistry/node_modules/@lumino/widgets/types/menu").Menu'.

476   mainMenu.addMenu(menu, { rank: 40 });
                       ~~~~

  node_modules/@jupyterlab/docregistry/node_modules/@lumino/widgets/types/widget.d.ts:382:13
    382     private _toggleHidden;
                    ~~~~~~~~~~~~~
    '_toggleHidden' is declared here.


Found 17 errors.

error Command failed with exit code 1.

Dependencies from package.json:

 "dependencies": {
    "@jupyterlab/application": "^3.5.0",
    "@jupyterlab/apputils": "^3.5.0",
    "@jupyterlab/codeeditor": "^3.5.0",
    "@jupyterlab/docmanager": "^3.5.0",
    "@jupyterlab/docregistry": "^3.5.0",
    "@jupyterlab/filebrowser": "^3.5.0",
    "@jupyterlab/fileeditor": "^3.5.0",
    "@jupyterlab/launcher": "^3.5.0",
    "@jupyterlab/mainmenu": "^3.5.0",
    "@jupyterlab/markdownviewer": "^3.5.0",
    "@jupyterlab/notebook": "^3.5.0",
    "@jupyterlab/observables": "^4.5.0",
    "@jupyterlab/rendermime": "^3.5.0",
    "@jupyterlab/services": "^6.5.0",
    "@jupyterlab/settingregistry": "^3.5.0",
    "@jupyterlab/shared-models": "^3.5.0",
    "@jupyterlab/ui-components": "^3.5.0",
    "@lumino/coreutils": "^1.12.1",
    "@lumino/signaling": "^1.11.0",
    "@lumino/widgets": "^1.35.0"
  },
  "devDependencies": {
    "@babel/core": "^7.20.2",
    "@babel/preset-env": "^7.20.2",
    "@jupyterlab/builder": "3.5.0",
    "@jupyterlab/testutils": "^3.5.0",
    "@types/jest": "^26.0.24",
    "@typescript-eslint/eslint-plugin": "^4.33.0",
    "@typescript-eslint/parser": "^4.33.0",
    "eslint": "^7.32.0",
    "eslint-config-prettier": "^6.15.0",
    "eslint-plugin-prettier": "^3.4.1",
    "jest": "^26.6.3",
    "npm-run-all": "^4.1.5",
    "prettier": "^2.7.1",
    "rimraf": "^3.0.2",
    "stylelint": "^14.14.1",
    "stylelint-config-prettier": "^9.0.4",
    "stylelint-config-recommended": "^6.0.0",
    "stylelint-config-standard": "^24.0.0",
    "stylelint-prettier": "^2.0.0",
    "ts-jest": "^26.5.6",
    "typescript": "^4.1.6"
  },

@rickmcgeer
Copy link
Author

But this was less a request for backwards compatibiltiy of shared models (I understand this is under rapid development) than a genuine request for guidance; developers like me should wait for a stable release of shared editing. It's mostly that I can't find another way to open, read, and write a file using the JupyterLab API, which I think is likely ignorance on our part, not a development problem.

@jtpio
Copy link
Member

jtpio commented Feb 15, 2023

cc @sebreb who may have found a similar issue when updating from 3.5 to 3.6 (feel free to open a separate one otherwise, thanks!)

@fcollonval
Copy link
Member

Thanks for the error trace @rickmcgeer

Primary comments,

  • could you check that in yarn.lock you only have one version of all @jupyterlab and @lumino packages; especially Lumino widget. The error about _toggleHidden smell like a conflict between two versions used in the dev environment.
  • do you distribute your extension as pre-built extension? If yes, can you install it an execute it within JupyterLab 3.6? And if it is not working in 3.6, could you report also the error seen?

The main API break I'm seeing is:

node_modules/@jupyterlab/docregistry/lib/registry.d.ts:353:15 - error TS2430: Interface 'ICodeModel' incorrectly extends interface 'IModel'.
  Types of property 'sharedModel' are incompatible.
    Property 'source' is missing in type 'ISharedFile' but required in type 'ISharedText'.

353     interface ICodeModel extends IModel, CodeEditor.IModel {
                  ~~~~~~~~~~

  node_modules/@jupyter/ydoc/lib/api.d.ts:93:5
    93     source: string;
           ~~~~~~
    'source' is declared here.

We introduced a new source in @jupyter/ydoc:ISharedText that was not there in @jupyterlab/shared-models:

https://github.com/jupyter-server/jupyter_ydoc/blob/8854a43982e623f926a2094f58819716eb16bd77/javascript/src/api.ts#L112

export interface ISharedText extends ISharedBase {

And unfortunately it seems you defined your own source in the sharedModel of GalyleoModel that is a YText and not a string.

        Type 'GalyleoModel' is not assignable to type 'IModel'.
          The types of 'sharedModel.source' are incompatible between these types.
            Type 'YText' is not assignable to type 'string'.

@rickmcgeer
Copy link
Author

@fcollonval here is the yarn.lock file. I appreciate that this is a lot to go through, so in an hour I'll write an awk script to pull out the relevant versions, but I wanted to have the ground truth on the record before I reduce it.

yarn.lock.zip

@rickmcgeer
Copy link
Author

@fcollonval here are the versions from the yarn.lock file, reduced. I am not sure if the dependency lines represent separate versions or not.
packages-loaded.txt

@rickmcgeer
Copy link
Author

@fcollonval And here is the logfile from the prebuilt extension.
prebuilt-extension-log.txt

I'm happy to provide code, commentary, anything at all toe get this resolved.

And many thanks.

@rickmcgeer
Copy link
Author

@fcollonval

And unfortunately it seems you defined your own source in the sharedModel of GalyleoModel that is a YText and not a string.

That is quite possible, and, as I said, I believe the real fix for this is for me to fix our code. We are really asking for best-practices guidance here. What typically happens here is that our code breaks due to some change in the underlying API, and we do the best patch we can think of to make it consistent with the underlying API. Unfortunately, I don't really understand the underlying API all that well (I mostly just guess by looking at the existing code; I've been unable to find an extension example with functionality close to ours) and so it's entirely possible I'm making lots of mistakes.

What we want to do is really simple...it really would be a few lines of Python (or whatever) over a POSIX filesystem API. We just want to (a) read files; (b) write files; and (c) install an icon on the launcher and a few menu items on the File dropdown and file browser context menus, which trigger file actions. So I'm sure there's a really short and simple way to do this, but I haven't been able to find it.

@rickmcgeer
Copy link
Author

Well, I am slowly going mad. I tried to fix this by having GalyleoModel extend DocumentModel, since DocumentModel mostly does the right thing and (as I said above) all we're really doing is reading and writing files, and trying to be as compatible as possible with the JupyterLab API. However, I am now seeing this:

[{
	"resource": "/workspaces/jupyterlab-universal-extension/node_modules/@jupyterlab/docregistry/lib/default.d.ts",
	"owner": "typescript",
	"code": "2420",
	"severity": 8,
	"message": "Class 'DocumentModel' incorrectly implements interface 'ICodeModel'.\n  Types of property 'mimeTypeChanged' are incompatible.\n    Type 'ISignal<this, IChangedArgs<string, string, string>>' is not assignable to type 'ISignal<IModel, IChangedArgs<string, string, string>>'.\n      Type 'this' is not assignable to type 'IModel'.\n        Type 'DocumentModel' is not assignable to type 'IModel'.\n          Types of property 'sharedModel' are incompatible.\n            Type 'ISharedFile' is not assignable to type 'ISharedText'.",
	"source": "ts",
	"startLineNumber": 15,
	"startColumn": 22,
	"endLineNumber": 15,
	"endColumn": 35
}]

Here's the output of yarn listr @jupyterlab/docregistry:

yarn list v1.22.19
warning Filtering by arguments is deprecated. Please use the pattern option instead.
├─ @jupyterlab/application@3.5.0
│  └─ @jupyterlab/docregistry@3.6.1
├─ @jupyterlab/cells@3.6.1
│  └─ @jupyterlab/docregistry@3.6.1
├─ @jupyterlab/docmanager@3.5.0
│  └─ @jupyterlab/docregistry@3.6.1
├─ @jupyterlab/docregistry@3.5.0
├─ @jupyterlab/filebrowser@3.5.0
│  └─ @jupyterlab/docregistry@3.6.1
├─ @jupyterlab/fileeditor@3.5.0
│  └─ @jupyterlab/docregistry@3.6.1
├─ @jupyterlab/markdownviewer@3.5.0
│  └─ @jupyterlab/docregistry@3.6.1
├─ @jupyterlab/notebook@3.5.0
│  └─ @jupyterlab/docregistry@3.6.1
└─ @jupyterlab/testutils@3.5.0
   └─ @jupyterlab/docregistry@3.6.1

So it looks like a version problem, but what I don't get is DocumentModel, ICodeModel and IModel are all defined in @jupyterlab/docregistry, so this should be consistent.
Any help appreciated....

Thanks

@rickmcgeer
Copy link
Author

rickmcgeer commented Feb 17, 2023

And in fact (further digging) in docregistry/lib/registry.d.ts I have, line 318, in the declaration of IModel:

readonly sharedModel: models.ISharedDocument;
```

But in line 353-355:
```
interface ICodeModel extends IModel, CodeEditor.IModel {
      sharedModel: models.ISharedFile;
 }
```
And VSCode is complaining:
```
[{
	"resource": "/workspaces/jupyterlab-universal-extension/node_modules/@jupyterlab/docregistry/lib/registry.d.ts",
	"owner": "typescript",
	"code": "2430",
	"severity": 8,
	"message": "Interface 'ICodeModel' incorrectly extends interface 'IModel'.\n  Types of property 'sharedModel' are incompatible.\n    Property 'source' is missing in type 'ISharedFile' but required in type 'ISharedText'.",
	"source": "ts",
	"startLineNumber": 353,
	"startColumn": 15,
	"endLineNumber": 353,
	"endColumn": 25,
	"relatedInformation": [
		{
			"startLineNumber": 93,
			"startColumn": 5,
			"endLineNumber": 93,
			"endColumn": 11,
			"message": "'source' is declared here.",
			"resource": "/workspaces/jupyterlab-universal-extension/node_modules/@jupyter/ydoc/lib/api.d.ts"
		}
	]
}
```

So, unless I'm missing something (and I could well be missing something) it's not an incompatible-package issue.
Giving up for the night as I am fresh out of ideas...

@fcollonval
Copy link
Member

Thanks for all the information, I opened engageLively/jupyterlab-universal-extension#1 to be able to comment directly on your code to help you improve it.

@JasonWeill JasonWeill added documentation and removed status:Needs Triage Applied to new issues that need triage labels Feb 23, 2023
@JasonWeill
Copy link
Contributor

Leaving this open as a documentation enhancement — we have an API, and we have extension documentation, but we don't have our own best practices document yet.

@krassowski
Copy link
Member

To add here: we do not have a separated stable API subset for extensions; we could consider freezing a subset of API which is critical for most extensions. This could be also helpful for security hardening (by only exposing a subset of our API in a hypothetical future hardened mode)

@rickmcgeer
Copy link
Author

rickmcgeer commented Feb 24, 2023 via email

@rickmcgeer
Copy link
Author

rickmcgeer commented Feb 24, 2023 via email

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