React problems + lsp (langium)

[cdietrich]

Hi, with the newest versions i dont get my react based app running anymore. the wrapper based does though

here is the code:
https://github.com/cdietrich/monaco-editor-react-new

do you have an idea? is there an easy way to figure what the client communicates with the language server

[kaisalmen]

is there an easy way to figure what the client communicates with the language server

Hi @cdietrich easiest way is to pass you own readers and writers, like here:
https://github.com/TypeFox/monaco-languageclient/blob/main/packages/examples/src/langium/statemachine/main.ts#L36-L40
They are passed on to the configuration function.

I will have a look later. I am figuring out test issues right now.

[cdietrich]

have the feeling the worker is not getting messages at all...

[cdietrich]

hmmm

=> which codepath is intended to start the client?

[cdietrich]

hmmm when patching it ls gets called but the styling is still wired. need to check that one as well

[kaisalmen]

@cdietrich do you see the Worker appear in the left pane above (screenshot ⬆️)?

#859 makes init/start more robust/predictable

I will take a look at your example now.

[cdietrich]

yes the worker is there.
just pushed a fix for the styling.
here is what i patched:

[kaisalmen]

@cdietrich have you tested an unbundled version of the worker? I see that it is started locally, too. But I don't know if it is doing anything.

[ahmed-s-fatahallah]

Hello @kaisalmen
after checking @cdietrich issue I found out that his issue is when the wrapper code resources property changes the useEffect will rerun and the cleanup function will call destroyMonaco function before initing a new editor but because destroyMonaco is an async function and react's useEffect cleanup function cannot be an async function and only run code inside of it synchronically, hence it won'tt wait for destroyMonaco function to finish executing and resolve the promise so the init will be called before the destroyMonaco finish executing which will result in this error.

his solution is simple we just need to make sure that the old editor has finished disposing before re-initing a new one this can be achieved by just adding the destroyMonaco and await it for it before calling initMonaco again like this

 (async () => {
            await destroyMonaco();
            await initMonaco();
            await startMonaco();
        })();

we also will leave the cleanup function as it is to make sure that if the react component unmounts the editor instance will be disposed

I have opened an MR with the fix can you please check it out ?

Also, @cdietrich can you try this solution and confirm if it works for you?

Thanks and sorry for this bug

[cdietrich]

hmmm
i see this logged

hook.js:608 Error: Trying to add a disposable to a DisposableStore that has already been disposed of. The added object will be leaked!
    at _DisposableStore.add (lifecycle.js:239:32)
    at ExtHostOutputService2.doCreateOutputChannel (extHostOutput.js:131:43)

and language server instances leaking .... (in the other example that toggles back and forth) at https://github.com/cdietrich/my-monaco-editor-react-example/tree/origin/cd/otherFilesExperiments_createModelReference_multi-newer-600)

[cdietrich]

the the task queue approach from #872 i dont see that problem

[ahmed-s-fatahallah]

hmmm i see this logged

hook.js:608 Error: Trying to add a disposable to a DisposableStore that has already been disposed of. The added object will be leaked!
    at _DisposableStore.add (lifecycle.js:239:32)
    at ExtHostOutputService2.doCreateOutputChannel (extHostOutput.js:131:43)

and language server instances leaking .... (in the other example that toggles back and forth) at https://github.com/cdietrich/my-monaco-editor-react-example/tree/origin/cd/otherFilesExperiments_createModelReference_multi-newer-600)

@cdietrich This warning is due to the cleanup function in the useEffect is run each time the wrapper config change which tries to destroy the monacoInstance while It has not finished executing yet the newly added destroyMonaco is run which result in this warning so I have separated the cleanup function in a separate useEffect that has an empty dependencies array to make sure that the clean up function will only run before the component unmounts

useEffect(()=>{
        return ()=>{
            try {
                wrapperRef.current.dispose();
            } catch {
                // The language client may throw an error during disposal.
                // This should not prevent us from continue working.
            }
        };
    },[]);

and I have updated #873

can you check it out, please

Thanks

[cdietrich]

can confirm it looks better