TypeFox / monaco-components

Monaco Editor and Language Client Wrapper, plus Monaco Editor React Component
MIT License
42 stars 13 forks source link

UI Flickering after updating manually the content (code) of the editor #46

Closed rhumbertgz closed 11 months ago

rhumbertgz commented 1 year ago

Hello,

The UserConfig property of the MonacoEditorReactComp allows us to initialise the content of the editor using wrapperConfig / editorAppConfig / code property.

For example:

const userConfig: UserConfig = {
            htmlElement: rootElem,
            wrapperConfig: {
                ...
                editorAppConfig: {
                    $type: 'classic',
                    languageId: 'typescript',
                    useDiffEditor: false,
                    theme: 'vs-light',
                    code: content,
                },
            },
};
<MonacoEditorReactComp
                    ref={editorRef}
                    userConfig={userConfig}
/>

I would like to push an external update to the current editor code, let's consider a button that add a new console.log('Hello') to the editor.

My first attempt was using the useState hook:

const [content, setContent ] = useState(initialCode);

And then call the setContent inside of the onClickHandler of the button, but this cause a restart of the editor getting a flickering in the UI.

Then I found that I can access the current value of the editor as follow:

const editor = editorRef.current?.getEditorWrapper().getEditor();
const currentCode=  editor?.getValue();

and I tried to update the value of the editor inside of a useEffect with content as dependency. In this case, I initialised the wrapperConfig / editorAppConfig / code property with an empty string

editor?.setValue(content);

That approach was worst fully crashing the editor.

My third attempt was to try to update it via the updateModel of the MonacoEditorWrapper but it raised an error so the editor was restarted again.

My code

const wrapper = editorRef.current?.getEditorWrapper();
wrapper?.updateModel({code: code});

The new error

No service init on restart
wrapper.ts:85 Starting monaco-editor (15)
editorAppClassic.ts:101 Init of MonacoConfig was completed.
editorAppBase.ts:109 Uncaught (in promise) Error: You cannot update the editor model, because the regular editor is not configured.
    at EditorAppClassic.updateModel (editorAppBase.ts:109:35)
    at MonacoEditorLanguageClientWrapper.updateModel (wrapper.ts:132:31)
    at index.tsx:79:22
    at commitHookEffectListMount (react-dom.development.js:23150:26)
    at commitPassiveMountOnFiber (react-dom.development.js:24931:11)
    at commitPassiveMountEffects_complete (react-dom.development.js:24891:9)
    at commitPassiveMountEffects_begin (react-dom.development.js:24878:7)
    at commitPassiveMountEffects (react-dom.development.js:24866:3)
    at flushPassiveEffectsImpl (react-dom.development.js:27039:3)
    at flushPassiveEffects (react-dom.development.js:26984:14)

When I looked at the editorAppBase.ts Line 109 the exception is raised because the editor is null/undefined.

editorAppBase.ts

async updateModel(modelUpdate: ModelUpdate): Promise<void> {
        if (!this.editor) {
            return Promise.reject(new Error('You cannot update the editor model, because the regular editor is not configured.'));
        }

        this.updateAppConfig(modelUpdate);
        await this.updateEditorModel();
}

I updated my code as follows, but although the error is gone, the editor still is restarted.

const wrapper = editorRef.current?.getEditorWrapper();

if (wrapper?.getEditor()) {
     wrapper?.updateModel({code: code});
}

In the bellow gif you can see the flickering issue using my first approach. Due to the quality of the gif only on the four addition of the console.logyou can see the flickering of the strings.

Sep-07-2023 10-31-12

Does anyone know the right way to update the code of the editor in a programatically/manual way?

Here is the code of my demo. It is basically a modification of the reactTs.ts example available in monaco-components repository

reactTs.ts

import React from 'react';
import ReactDOM from 'react-dom/client';
import EditorDemo from './EditorDemo.js';

const comp = <EditorDemo />
const rootElem = document.getElementById('root')!;
const root = ReactDOM.createRoot(rootElem);
root.render(comp);

EditorDemo.ts

import React, { useState } from 'react';
import { MonacoEditorReactComp } from '@typefox/monaco-editor-react';
import { UserConfig } from 'monaco-editor-wrapper';

import 'monaco-editor/esm/vs/basic-languages/typescript/typescript.contribution.js';
import 'monaco-editor/esm/vs/language/typescript/monaco.contribution.js';

import { buildWorkerDefinition } from 'monaco-editor-workers';
buildWorkerDefinition('../../../../node_modules/monaco-editor-workers/dist/workers', import.meta.url, false);

const EditorDemo: React.FC = () => {
    const logMessage = `console.log('hello')`
    const [content, setContent] = useState(logMessage);

    const rootElem = document.getElementById('root')!;
    const userConfig: UserConfig = {
        htmlElement: rootElem,
        wrapperConfig: {
            serviceConfig: {
                enableKeybindingsService: true,
                debugLogging: true
            },
            editorAppConfig: {
                $type: 'classic',
                languageId: 'typescript',
                useDiffEditor: false,
                theme: 'vs-dark',
                code: content
            }
        }
    };

    const addConsoleMessage = () => {
        setContent(`${content}\n${logMessage}`);
    };

    return (
        <>
            <button onClick={addConsoleMessage}>
                Click me!
            </button>
            <MonacoEditorReactComp
                userConfig={userConfig}
                style={{
                    'paddingTop': '5px',
                    'height': '80vh'
                }}
            />
        </>

    );
};

export default EditorDemo;
kaisalmen commented 12 months ago

Hi @rhumbertgz thank you for reporting this. Using updateModel is the correct approach. The underlying monaco-editor-wrapper does not require a restart in this case.

It may be the case that componentDidUpdate in monaco-editor-react has an unidentified problem in the mustReInit detection logic. I will try to re-produce the problem with your example.

rhumbertgz commented 12 months ago

@kaisalmen thanks!

kaisalmen commented 12 months ago

@rhumbertgz I have reproduced your issue and the restart detection logic is bad at one point as I suspected. Will work on a fix now...

rhumbertgz commented 12 months ago

@kaisalmen thanks for the update 👌

kaisalmen commented 12 months ago

@rhumbertgz #47 is open. There is still a thing that needs to be fixed.

kaisalmen commented 11 months ago

@rhumbertgz #47 is ready for review now. The fixes and others are included in a new preview versions: https://www.npmjs.com/package/monaco-editor-wrapper/v/3.1.0-next.0