TypeFox / monaco-languageclient

Repo hosts npm packages for monaco-languageclient, vscode-ws-jsonrpc, monaco-editor-wrapper, @typefox/monaco-editor-react and monaco-languageclient-examples
https://www.npmjs.com/package/monaco-languageclient
MIT License
992 stars 171 forks source link

wrapper.dispose() does not get rid of extension #679

Closed abentkamp closed 1 week ago

abentkamp commented 1 week ago

I assume that MonacoEditorLanguageClientWrapper.dispose() is supposed unregister anything that it registered during init and start, right? Then it does not seem to work correctly with extensions that use filesOrContents.

Here is a small example (full reproduction repo here: https://github.com/abentkamp/monacotest3/tree/reinit) :

import { MonacoEditorLanguageClientWrapper, UserConfig } from 'monaco-editor-wrapper';

const extensionFilesOrContents = new Map<string, string | URL>();
extensionFilesOrContents.set('/test.json', "Test");

const userConfig : UserConfig = {
  wrapperConfig: {
    editorAppConfig: {
      $type: 'extended',
      codeResources: {
        main: {
          text: 'print("Hello, World!")',
          uri: '/workspace/hello.py'
        }
      },
      extensions: [{
        config: {
            name: 'myextension',
            publisher: 'me'
        },
        filesOrContents: extensionFilesOrContents
      }]
    }
  }
};

const run = async () => {
  const htmlElement = document.getElementById('monaco-editor-root');

  const wrapper = new MonacoEditorLanguageClientWrapper();
  await wrapper.initAndStart(userConfig, htmlElement);
  await wrapper.dispose();

  const wrapper2 = new MonacoEditorLanguageClientWrapper();
  await wrapper2.initAndStart(userConfig, htmlElement);
  await wrapper2.dispose();
}

run()

I get the following error:

Uncaught (in promise) Error: file 'extension-file://me.myextension/' already exists
    registerFile files.js:307
    registerExtensionFile files.js:762
    registerExtensionFileUrl extensions.js:29
    registerFileUrl extensions.js:122
    init editorAppExtended.ts:93
    start wrapper.ts:100
    initAndStart wrapper.ts:85
    run main.ts:36
    async* main.ts:40
[files.js:307:18](http://localhost:5174/node_modules/@codingame/monaco-vscode-files-service-override/files.js)
    run main.ts:38
    InterpretGeneratorResume self-hosted:1469
    AsyncFunctionThrow self-hosted:856
    (Async: async)
    <anonymous> main.ts:40
abentkamp commented 1 week ago

Okay, I think I found the issue: The call to RegisterLocalExtensionResult.registerFileUrl at

https://github.com/TypeFox/monaco-languageclient/blob/main/packages/wrapper/src/editorAppExtended.ts#L93

returns an IDisposable that needs to be disposed in the EditorAppExtended's disposeApp function.

kaisalmen commented 1 week ago

Hi @abentkamp thank you for spotting this. I didn't had time to investigate that, yet. Do want to propose a PR/fix?

vrama628 commented 1 week ago

I've been running into the same issue -- here's a fix: https://github.com/TypeFox/monaco-languageclient/pull/680

abentkamp commented 1 week ago

Thanks a lot for the PR @vrama628 !