Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Use monaco-languageclient instead of monaco-editor #22

Closed
wants to merge 23 commits into from

Conversation

fcollonval
Copy link
Member

@fcollonval fcollonval commented Nov 4, 2018

This PR builds from the nice work of @juliandolby in #12 in an attempt to reach a new stable step.

The major addition to #12:

Major knowns issues:

  • Websocket connection is not secured (to check)
  • Socket connection is never closed - even when the file editor is closed
    But a new websocket is created each time a file is (re)opened
    => we multiply the number of server instance (for the Python LS example)
    & => reopening a previously opened file results in an unusable editor
    Note: Can we get inspiration from the terminal code?

Issue linked to the Python Language Server: Some file crashes the pyls subprocess. This requires the all jupyterlab to be restarted. It has something to do with the server response size.

/closes #12

@dhirschfeld dhirschfeld mentioned this pull request Nov 14, 2018
@gnestor
Copy link
Contributor

gnestor commented Nov 15, 2018

@fcollonval Thanks so much for taking the lead on this!! We are having an in-person JupyterLab team meeting 11/26-30 and my goal is to get this working and lay down a foundation for long-term LSP support in Jupyter.

Please let me know what I can do to help in the meantime 👍

@fcollonval
Copy link
Member Author

fcollonval commented Nov 15, 2018

Hey @gnestor , thanks for the feedback and interest.

So questions and comments:

Related to monaco languageclient

To pack monaco-languageclient, the following entry needs to be specified in the webpack.config.js to link to a fake vscode api.
https://github.com/fcollonval/jupyterlab-monaco/blob/cce20dd782999f5f05ff1418a5b1016a0a169345/webpack.config.js#L30

The trouble comes from the fact that I need to specify that entry also in the webpack.config.js of JupyterLab to be able to install the extension. I don't know if that step can be avoided or if we can somehow provide some options when jupyterlab is rebuilding with this extension activated.

Related issues in theia and monaco-languageclient;
TypeFox/monaco-languageclient#24
eclipse-theia/theia#2589 (comment)

Another mandatory setting (for the extension only) is enabling skipLibCheck. I don't understand the consequences of this.

Related to the python LSP server extension

Currently, I used simple websocket from tornado. I'm even not inheritating from IPythonHandler. I quickly look at the websocket for console and notebook. But when I try using WebSocketMixin and IPythonHandler like in
https://github.com/jupyter/notebook/blob/b8b66332e2023e83d2ee04f83d8814f567e01a4e/notebook/terminal/handlers.py#L22
I got 404 reply. So if you know how to handle properly websocket connection, your knowledge will be more than welcomed.

Next step

But as I felt like pushing integration further, I tried replacing IEditorServices by one based on monaco and LSP (see #23).

Conclusion

If we can find a solution to enable classical jupyterlab extension (i.e. without the need to modify webpack.config.js file), this PR can be merged for me as it's usable.

@gnestor
Copy link
Contributor

gnestor commented Nov 19, 2018

Thanks for those details, @fcollonval.

Regarding the webpack issue:

First, @blink1073 is there any new way for an extension to provide any webpack config of its own to jupyterlab?

If not, then we may want to consider moving this work (your PR and jupyterlab-monaco as a whole) into jupyterlab core.

Lastly, to better understand the problem, I assume vscode-base-languageclient and other dependencies of monaco-languageclient are depending on vscode (which isn't exactly usable in the browser) and monaco-languageclient/lib/vscode-compatibility provides a shim that makes it usable? If so, I guess that we could fork these packages, modify them to use monaco-languageclient/lib/vscode-compatibility for vscode, publish them, and then depend on the forked packages in the meantime. Not an ideal solution but could work...

@blink1073
Copy link
Contributor

We were trying to avoid exposing Webpack, keeping it as an implementation detail. We do however expose generic loaders: https://github.com/jupyterlab/jupyterlab/blob/125637729d75b19bdc8177e41f4d8310a09dd229/dev_mode/webpack.config.js#L131

@fcollonval
Copy link
Member Author

Thanks for those feedbacks.

@gnestor
It sounds that the latest solution (forking the packages) is the way to go?

I looked deeper to the dependencies:

@gnestor
Copy link
Contributor

gnestor commented Nov 27, 2018

Hi @fcollonval! The JupyterLab team is together in person this week so let's get this PR merged! I created a Gitter room for us to chat about it: https://gitter.im/jupyterlab-monaco/Lobby

cc @ian-r-rose @afshin @saulshanabrook

First things first, I need to get this running locally. Do you have a branch of JupyterLab that this PR works with? I tried simply adding:

resolve: {
  alias: {
    vscode: require.resolve('monaco-languageclient/lib/vscode-compatibility')
  }
}

to webpack.config.js in my staging directory (which for me is /Users/grant/anaconda/share/jupyter/lab/staging) and I'm getting the following:

$ webpack
ModuleNotFoundError: Module not found: Error: Can't resolve 'vscode' in '/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/vscode-base-languageclient/lib'
    at factory.create (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/webpack/lib/Compilation.js:535:10)
    at factory (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/webpack/lib/NormalModuleFactory.js:397:22)
    at resolver (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/webpack/lib/NormalModuleFactory.js:130:21)
    at asyncLib.parallel (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/webpack/lib/NormalModuleFactory.js:224:22)
    at /Users/grant/anaconda/share/jupyter/lab/staging/node_modules/neo-async/async.js:2825:7
    at /Users/grant/anaconda/share/jupyter/lab/staging/node_modules/neo-async/async.js:6886:13
    at normalResolver.resolve (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/webpack/lib/NormalModuleFactory.js:214:25)
    at doResolve (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/Resolver.js:184:12)
    at hook.callAsync (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/Resolver.js:238:5)
    at _fn0 (eval at create (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:15:1)
    at resolver.doResolve (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/UnsafeCachePlugin.js:37:5)
    at hook.callAsync (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/Resolver.js:238:5)
    at _fn0 (eval at create (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:15:1)
    at hook.callAsync (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/Resolver.js:238:5)
    at _fn0 (eval at create (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:12:1)
    at resolver.doResolve (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/DescriptionFilePlugin.js:42:38)
    at hook.callAsync (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/Resolver.js:238:5)
    at _fn41 (eval at create (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:381:1)
    at resolver.doResolve (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/ModuleKindPlugin.js:23:37)
    at hook.callAsync (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/Resolver.js:238:5)
    at _fn0 (eval at create (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:15:1)
    at hook.callAsync (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/Resolver.js:238:5)
    at _fn0 (eval at create (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:15:1)
    at args (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/forEachBail.js:30:14)
    at hook.callAsync (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/Resolver.js:238:5)
    at _fn0 (eval at create (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:15:1)
    at resolver.doResolve (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/UnsafeCachePlugin.js:37:5)
    at hook.callAsync (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/Resolver.js:238:5)
    at _fn0 (eval at create (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:15:1)
    at hook.callAsync (/Users/grant/anaconda/share/jupyter/lab/staging/node_modules/enhanced-resolve/lib/Resolver.js:238:5)

@fcollonval
Copy link
Member Author

Hey @gnestor

I realized this morning that @juliandolby has updated to the latest monaco-languageclient package on another branch. And to avoid the vscode package trouble, he included it directly as devDependency
juliandolby@862b81b

Although this is probably the easiest hack, this means including vscode in JupyterLab: i.e. problably shipping far too much stuff. Nota: I have not test this.

Another though: this extension is already bundling the editor separately (due to worker constraints). Would it work/be interesting to bundle with webpack monaco-languageclient and vscode-base-languageclient and load that bundle in the extension through fileloader?

@cgpu
Copy link

cgpu commented May 23, 2020

@fcollonval I know it's been a loong time since this PR, but I am wondering since it hasn't been merged, if the functionality has been moved to another extension. If so could you share what is available to have the Monaco editor in JupyterLab?

@fcollonval
Copy link
Member Author

Hey @cgpu, this was a trial but nothing went further on my side. You definitely should look at jupyterlab-lsp altough this is using CodeMirror.

@fcollonval fcollonval closed this May 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants