monaco: upgrade @theia/monaco-editor-core

[vince-fugnitto]

Feature Description:

The issue tracks the upgrade of @theia/monaco-editor-core which pulls a newer version of the monaco-editor built from theia-ide/vscode. The current version of monaco matches 1.72.3 while our supported API is currently at 1.78.0 and soon to be 1.79.0.

I propose we use the latest tagged release of VS Code and benefit from monaco updates, UI/UX improvements and increased compatibility.

Steps

• release a new version of @theia/monaco-editor-core
• adapt the new editor version to the framework
• extensive testing and open an upstream pull-request

Documentation

Information related to performing the monaco uplift:

• https://github.com/eclipse-theia/theia/tree/master/packages/monaco#monaco-uplifts

[vince-fugnitto]

Unfortunately it does not seem like I will have the bandwidth to continue working on the upgrade.
I can certainly help the author with how to build @theia/monaco-editor-core, and testing.

[vince-fugnitto]

@rschnekenbu has expressed interest in performing the uplift 👍

Edit: do you mind commenting Remy so I can assign the issue to you?

[rschnekenbu]

@vince-fugnitto, can you please assign me this ticket?

[tsmaeder]

Some changes which do not show up in compilation:

1. the front-end command "setContext" was renamed to "_setContext" and the original command is now defined in the extensions host.
2. We get two constructor invocations of "StandaloneCodeEditorService", leading to two instances and erroneous tracking of the active editor, which in turn leads to typing not working in the editor. Maybe this is because the StandaloneCodeEditorService is create eagerly?, see

standaloneCodeEditoService.ts#registerSingleton(ICodeEditorService, StandaloneCodeEditorService, false/eager) 

[tsmaeder]

Investigating the trouble with the double instantiation of the StandaloneCodeEditorService: when the user types in the editor, the typing action is eventually delegated to EditorHandlerCommand#runCommand in the coreCommands.ts file from VS Code. The runCommand function will fetch and ICodeEditorService form the global StandaloneServices. But this service is not the instance of MonacoEditorService we would like to use, but a naked instance of StandaloneEditorService. This service will try to find the focused editor by going through all the editors it manages and ask whether the editor has focus. But since this service does not know the Theia editors (The MonacoEditorService manages those), it will never find a focused editor and will ignore the typed character.
So why is this happening? We try to override the ICodeEditorService in various places, for example in the constructor of MonacoEditorProvider, but also in the postConstruct method in MonacoFrontendApplicationContribution. Sadly, none of these attempts to override the services will work. If we look at the code in the StandaloneServices module in VS Code, as soon as we call initialize the first time, the set of services becomes fixed and subsequent calls to initialize with overrides will be silently ignored. That does not sound so bad, but unfortunately, calling StandaloneServices.get calls initialize with no overrides. And we do a lot of those calls in @postConstruct methods of various Theia objects.
We also use an instantiator with service overrides when creating Monaco editors. While this overrides the services in the editor, it does not affect the global services registry.
One change the has recently occurred in VS Code is that services like ICodeEditorService are now created eagerly: so if a service is requested, it is immediately instantiated. Before, you would get back a proxy that would defer instantiation until later. I believe this will eagerly create the transitive closure of all dependencies of a service as soon as it's accessed the first time, unless the dependencies are marked for lazy instantiation.
I am not entirely sure yet why things fail now and did not with the older version of VS Code, but I am convinced that in order to make things like command execution work properly, we will need to able to override the global StandaloneServices properly.

[tsmaeder]

I've prototyped an approach that would call this code from index.js:

    export function init(container: Container): void {

        // eslint-disable-next-line @typescript-eslint/no-explicit-any
        function getCodeEditorService(args: any[]): unknown {
            return container.get(MonacoEditorService);
        };

        StandaloneServices.initialize({
            // eslint-disable-next-line @typescript-eslint/no-explicit-any
            [ICodeEditorService.toString()]: new SyncDescriptor(getCodeEditorService as unknown as new (args: any[]) => unknown),
        });
    }

The idea is to let not override with instances, but with "constructor functions". This should work fine as long as we don't have services we need to inject into the constructor function. Maybe we can annotate the constructor function?

[tsmaeder]

Correction, the above does not really work because "getCodeEditorService is not a constructor" from Reflect.construct(). Sigh!

[tsmaeder]

But this seems to work, complete with service injection:

class MyService {
    constructor(container: Container,
        @IContextKeyService contextKeyService: IContextKeyService,
        @IThemeService themeService: IThemeService) {
        return container.get(MonacoEditorService);
    };
}

export class MonacoInit {
    static init(container: Container): void {

        StandaloneServices.initialize({
            // eslint-disable-next-line @typescript-eslint/no-explicit-any
            [ICodeEditorService.toString()]: new SyncDescriptor(MyService, [container]),
        });
    }
}

[tsmaeder]

So the instantiation seems to work properly now. Next problem: the find/replace dialog layout is broken in the editor.

[tsmaeder]

Turns out we're mapping icon-id's to codicon ids: https://github.com/eclipse-theia/theia/pull/11527/files The problem is that findWidget.css has a new rule that addresses the widget-id (widget-close), but since we map the icon id to close, the css does not get applied to the close icon.

[tsmaeder]

@vince-fugnitto I have a couple of questions about the instructions in https://github.com/theia-ide/vscode/wiki/Publish-%60@theia-monaco-editor-core%60

1. Shouldn't the changes to package.json be done in source and added to the commit that needs to be cherry-picked?
2. ThirdPartNotices.txt seems to be built wrong form me: it only contains two entries that look kind of like a template.

3. publish using npm publish ./out-monaco-editor-core {options}

what are {options}?

[tsmaeder]

I've published @theia/monaco-editor-core 1.83.1 and opened a IP issue: https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/12577

[tsmaeder]

So it turns out that tests fail with the upgraded monaco package. The reason is that we are running tests in nodejs, which cannot easily mix ESM and CJS modules. It seems this is not a problem in the webpacked front-end, but in nodejs, you cannot simply import a ESM module: so far, NodeJS simply loaded the modules from Monaco as CommonJS modules, which does not allow the export keyword. Newer versions of NodeJS can handle ESM modules, but there is a caveat: you have to use dynamic imports to load the files:

Error [ERR_REQUIRE_ESM]: require() of ES Module C:\Users\thomas\code\EclipseSource\theia\node_modules\@theia\monaco-editor-core\esm\vs\editor\standalone\browser\standaloneServices.js from C:\Users\thomas\code\EclipseSource\theia\packages\monaco\lib\browser\monaco-text-model-service.js not supported.
standaloneServices.js is treated as an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which declares all .js files in that package scope as ES modules.
Instead rename standaloneServices.js to end in .cjs, change the requiring code to use dynamic import() which is available in all CommonJS modules, or change "type": "module" to "type": "commonjs" in C:\Users\thomas\code\EclipseSource\theia\node_modules\@theia\monaco-editor-core\package.json to treat all .js files as CommonJS (using .mjs for all ES modules instead).

There are a couple of ways we could resolve this problem:

1. Use dynamic import('...') to load anything from Monaco. The problem here is that this is async: we would have to analyze whether we rely on synchronous import somewhere
2. Fix the Monaco code to not generate export statements. This would probably work for now, but we'd have to maintain this for future Monaco releases.
3. Change Theia to work use ESM modules.

[tsmaeder]

Correction: removing the export statements from the marked.js file in ESM Monaco makes the hover fail in a typescript editor. So that's not without problems either.