Implement review comments. Language client automatic flags are option…

…al and another flag for worker dispose was introduced
TypeFox · Mar 5, 2025 · d1db034 · d1db034
1 parent 80eae38
commit d1db034
Show file tree

Hide file tree

Showing 14 changed files with 108 additions and 129 deletions.
diff --git a/packages/examples/src/clangd/client/config.ts b/packages/examples/src/clangd/client/config.ts
@@ -31,9 +31,6 @@ export const createWrapperConfig = async (config: {
         htmlContainer: config.htmlContainer,
         logLevel: LogLevel.Debug,
         languageClientConfigs: {
-            automaticallyInit: true,
-            automaticallyStart: true,
-            automaticallyDispose: true,
             configs: {
                 LANGUAGE_ID: {
                     name: 'Clangd WASM Language Server',

diff --git a/packages/examples/src/eclipse.jdt.ls/client/main.ts b/packages/examples/src/eclipse.jdt.ls/client/main.ts
@@ -47,9 +47,6 @@ export const runEclipseJdtLsClient = () => {
             monacoWorkerFactory: configureDefaultWorkerFactory
         },
         languageClientConfigs: {
-            automaticallyInit: true,
-            automaticallyStart: true,
-            automaticallyDispose: true,
             configs: {
                 java: {
                     connection: {

diff --git a/packages/examples/src/groovy/client/main.ts b/packages/examples/src/groovy/client/main.ts
@@ -43,9 +43,6 @@ const wrapperConfig: WrapperConfig = {
         monacoWorkerFactory: configureDefaultWorkerFactory
     },
     languageClientConfigs: {
-        automaticallyInit: true,
-        automaticallyStart: true,
-        automaticallyDispose: true,
         configs: {
             groovy: {
                 clientOptions: {

diff --git a/packages/examples/src/json/client/wrapperWs.ts b/packages/examples/src/json/client/wrapperWs.ts
@@ -44,9 +44,6 @@ export const buildJsonClientUserConfig = (htmlContainer?: HTMLElement): WrapperC
             monacoWorkerFactory: configureDefaultWorkerFactory
         },
         languageClientConfigs: {
-            automaticallyInit: true,
-            automaticallyStart: true,
-            automaticallyDispose: true,
             configs: {
                 json: {
                     clientOptions: {

diff --git a/packages/examples/src/langium/langium-dsl/config/classicConfig.ts b/packages/examples/src/langium/langium-dsl/config/classicConfig.ts
@@ -49,9 +49,6 @@ export const setupLangiumClientClassic = async (langiumWorker: Worker): Promise<
             }
         },
         languageClientConfigs: {
-            automaticallyInit: true,
-            automaticallyStart: true,
-            automaticallyDispose: true,
             configs: {
                 langium: {
                     clientOptions: {

diff --git a/packages/examples/src/langium/langium-dsl/config/extendedConfig.ts b/packages/examples/src/langium/langium-dsl/config/extendedConfig.ts
@@ -74,9 +74,6 @@ export const setupLangiumClientExtended = async (langiumWorker: Worker): Promise
             monacoWorkerFactory: configureDefaultWorkerFactory
         },
         languageClientConfigs: {
-            automaticallyInit: true,
-            automaticallyStart: true,
-            automaticallyDispose: true,
             configs: {
                 langium: {
                     clientOptions: {

diff --git a/packages/examples/src/langium/statemachine/config/wrapperStatemachineConfig.ts b/packages/examples/src/langium/statemachine/config/wrapperStatemachineConfig.ts
@@ -16,15 +16,15 @@ import { configureDefaultWorkerFactory } from 'monaco-editor-wrapper/workers/wor
 import statemachineLanguageConfig from './language-configuration.json?raw';
 import responseStatemachineTm from '../syntaxes/statemachine.tmLanguage.json?raw';
 
-export const createLangiumGlobalConfig = async (params: {
+export const createLangiumGlobalConfig = (params: {
     languageServerId: string,
     useLanguageClient: boolean,
     text?: string,
     worker?: Worker,
     messagePort?: MessagePort,
     messageTransports?: MessageTransports,
     htmlContainer: HTMLElement
-}): Promise<WrapperConfig> => {
+}): WrapperConfig => {
     const extensionFilesOrContents = new Map<string, string | URL>();
     extensionFilesOrContents.set(`/${params.languageServerId}-statemachine-configuration.json`, statemachineLanguageConfig);
     extensionFilesOrContents.set(`/${params.languageServerId}-statemachine-grammar.json`, responseStatemachineTm);
@@ -38,9 +38,6 @@ export const createLangiumGlobalConfig = async (params: {
     }
 
     const languageClientConfigs: LanguageClientConfigs | undefined = params.useLanguageClient && params.worker ? {
-        automaticallyInit: true,
-        automaticallyStart: true,
-        automaticallyDispose: true,
         configs: {
             statemachine: {
                 clientOptions: {

diff --git a/packages/examples/src/langium/statemachine/main-react.tsx b/packages/examples/src/langium/statemachine/main-react.tsx
@@ -13,7 +13,7 @@ import text from '../../../resources/langium/statemachine/example.statemachine?r
 import { disableElement } from '../../common/client/utils.js';
 
 export const runStatemachineReact = async () => {
-    const wrapperConfig = await createLangiumGlobalConfig({
+    const wrapperConfig = createLangiumGlobalConfig({
         languageServerId: 'react',
         useLanguageClient: true,
         text,

diff --git a/packages/examples/src/langium/statemachine/main.ts b/packages/examples/src/langium/statemachine/main.ts
@@ -44,7 +44,7 @@ const startEditor = async () => {
     });
 
     // the configuration does not contain any text content
-    const langiumGlobalConfig = await createLangiumGlobalConfig({
+    const langiumGlobalConfig = createLangiumGlobalConfig({
         languageServerId: 'first',
         useLanguageClient: true,
         worker: stateMachineWorkerPort,
@@ -63,7 +63,7 @@ const startEditor = async () => {
 
     // start the second wrapper without any languageclient config
     // => they share the language server and both text contents have different uris
-    const langiumGlobalConfig2 = await createLangiumGlobalConfig({
+    const langiumGlobalConfig2 = createLangiumGlobalConfig({
         languageServerId: 'second',
         useLanguageClient: false,
         text: textMod,

diff --git a/packages/examples/src/multi/twoLanguageClients.ts b/packages/examples/src/multi/twoLanguageClients.ts
@@ -58,9 +58,6 @@ print("Hello Moon!")
             monacoWorkerFactory: configureDefaultWorkerFactory
         },
         languageClientConfigs: {
-            automaticallyInit: true,
-            automaticallyStart: true,
-            automaticallyDispose: true,
             configs: {
                 json: createJsonLanguageClientConfig(),
                 python: createPythonLanguageClientConfig()
@@ -76,7 +73,7 @@ print("Hello Moon!")
             disableElement('button-flip', false);
             disableElement('checkbox-extlc', true);
 
-            const externalLc = (document.getElementById('checkbox-extlc')! as HTMLInputElement).checked;
+            const externalLc = (document.getElementById('checkbox-extlc') as HTMLInputElement).checked;
             wrapperConfig.languageClientConfigs!.automaticallyInit = !externalLc;
             wrapperConfig.languageClientConfigs!.automaticallyStart = !externalLc;
             wrapperConfig.languageClientConfigs!.automaticallyDispose = !externalLc;
@@ -88,7 +85,7 @@ print("Hello Moon!")
             }
 
             // init language clients after start
-            if (externalLc === true) {
+            if (externalLc) {
                 wrapper.initLanguageClients();
                 await wrapper.startLanguageClients();
             }
@@ -101,7 +98,13 @@ print("Hello Moon!")
         disableElement('button-dispose', true);
         disableElement('button-start', false);
 
+        const externalLc = (document.getElementById('checkbox-extlc')! as HTMLInputElement).checked;
+
         await wrapper.dispose();
+
+        if (externalLc) {
+            wrapper.disposeLanguageClients();
+        }
     });
     document.querySelector('#button-flip')?.addEventListener('click', async () => {
         currentText = currentText === textJson ? textPython : textJson;

diff --git a/packages/examples/src/python/client/config.ts b/packages/examples/src/python/client/config.ts
@@ -101,9 +101,6 @@ export const createWrapperConfig = (): PythonAppConfig => {
         htmlContainer: configParams.htmlContainer,
         logLevel: LogLevel.Debug,
         languageClientConfigs: {
-            automaticallyInit: true,
-            automaticallyStart: true,
-            automaticallyDispose: true,
             configs: {
                 python: {
                     name: 'Python Language Server Example',

diff --git a/packages/wrapper-react/src/index.tsx b/packages/wrapper-react/src/index.tsx
@@ -28,57 +28,67 @@ export const MonacoEditorReactComp: React.FC<MonacoEditorProps> = (props) => {
     const wrapperRef = useRef<MonacoEditorLanguageClientWrapper>(new MonacoEditorLanguageClientWrapper());
     const containerRef = useRef<HTMLDivElement>(null);
 
-    const destroyMonaco = async () => {
-        try {
-            await wrapperRef.current.dispose();
-        } catch {
-            // The language client may throw an error during disposal.
-            // This should not prevent us from continue working.
-        }
-    };
+    useEffect(() => {
+        const disposeMonaco = async () => {
+            try {
+                await wrapperRef.current.dispose();
+            } catch {
+                // The language client may throw an error during disposal, but we want to continue anyway
+            }
+        };
 
-    const initMonaco = async () => {
-        if (containerRef.current) {
-            wrapperConfig.htmlContainer = containerRef.current;
-            await wrapperRef.current.init(wrapperConfig);
-        } else {
-            throw new Error('No htmlContainer found! Aborting...');
-        }
-    };
+        const initMonaco = async () => {
+            if (containerRef.current) {
+                wrapperConfig.htmlContainer = containerRef.current;
+                await wrapperRef.current.init(wrapperConfig);
+            } else {
+                throw new Error('No htmlContainer found! Aborting...');
+            }
+        };
 
-    const startMonaco = async () => {
-        if (containerRef.current) {
-            try {
-                wrapperRef.current.registerTextChangeCallback(onTextChanged);
-                await wrapperRef.current.start();
-                onLoad?.(wrapperRef.current);
-            } catch (e) {
-                if (onError) {
-                    onError(e);
-                } else {
-                    throw e;
+        const startMonaco = async () => {
+            if (containerRef.current) {
+                try {
+                    wrapperRef.current.registerTextChangeCallback(onTextChanged);
+                    await wrapperRef.current.start();
+                    onLoad?.(wrapperRef.current);
+                } catch (e) {
+                    if (onError) {
+                        onError(e);
+                    } else {
+                        throw e;
+                    }
                 }
+            } else {
+                throw new Error('No htmlContainer found! Aborting...');
             }
-        } else {
-            throw new Error('No htmlContainer found! Aborting...');
-        }
-    };
+        };
 
-    useEffect(() => {
         (async () => {
-            if (!wrapperRef.current.isStopping()) {
-                await destroyMonaco();
+            if (!wrapperRef.current.isDisposing()) {
+                await disposeMonaco();
                 await initMonaco();
                 await startMonaco();
             }
         })();
     }, [wrapperConfig, onTextChanged, onLoad, onError]);
 
-    useEffect(()=>{
-        (async () => {
-            await destroyMonaco();
-        })();
-    },[]);
+    useEffect(() => {
+        // exact copy of the above function, to prevent declaration in useCallback
+        const disposeMonaco = async () => {
+            try {
+                await wrapperRef.current.dispose();
+            } catch {
+                // The language client may throw an error during disposal, but we want to continue anyway
+            }
+        };
+
+        return () => {
+            (async () => {
+                await disposeMonaco();
+            })();
+        };
+    }, []);
 
     return (
         <div