aglinxinyuan commented on code in PR #4997:
URL: https://github.com/apache/texera/pull/4997#discussion_r3212865677


##########
frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.ts:
##########
@@ -228,53 +224,121 @@ export class CodeEditorComponent implements 
AfterViewInit, SafeStyle, OnDestroy
     }
   }
 
+  /**
+   * Lazily start the global monaco-vscode-api wrapper. The vscode API 
services are
+   * a process-wide singleton in v10; calling start() twice would throw, so we 
share
+   * a single Promise across every CodeEditorComponent instance.
+   */
+  private static async ensureVscodeApiStarted(): Promise<void> {
+    if (CodeEditorComponent.apiWrapperStarted) {
+      return;
+    }
+    CodeEditorComponent.apiWrapperStartPromise ??= (async () => {
+      const apiConfig: MonacoVscodeApiConfig = {
+        $type: "extended",
+        viewsConfig: { $type: "EditorService" },
+        userConfiguration: {
+          json: JSON.stringify({ "workbench.colorTheme": "Default Dark Modern" 
}),
+        },
+        // Wire up the workers monaco-vscode-api spawns at runtime (editor,
+        // extension host, textmate). Each `new Worker(new URL(...))` literal
+        // points at a thin trampoline in `./workers/` that just re-imports the
+        // codingame-shipped worker entry. Two reasons for the indirection:
+        //   1. webpack 5 only treats `new Worker(new URL("./relative", 
import.meta.url))`
+        //      as a worker entry point (so it bundles the dep tree into a 
chunk);
+        //      `new URL("@codingame/...", import.meta.url)` would just emit a
+        //      static asset whose own relative imports 404 at runtime.
+        //   2. the test pipeline (esbuild via @angular/build:unit-test) 
resolves
+        //      `new URL(spec, import.meta.url)` literally relative to the 
source
+        //      file, so the spec needs to point at a real on-disk file. The
+        //      trampolines satisfy both bundlers.
+        monacoWorkerFactory: () => {
+          const env = getEnhancedMonacoEnvironment();
+          env.getWorker = (_workerId: string, label: string): Worker => {
+            switch (label) {
+              case "editorWorkerService":
+                return new Worker(new URL("./workers/editor.worker", 
import.meta.url), { type: "module" });
+              case "extensionHostWorkerMain":
+                return new Worker(new URL("./workers/extension-host.worker", 
import.meta.url), { type: "module" });
+              case "TextMateWorker":
+                return new Worker(new URL("./workers/textmate.worker", 
import.meta.url), { type: "module" });
+              default:
+                throw new Error(`No worker configured for label: ${label}`);
+            }
+          };
+        },
+      };
+      await new MonacoVscodeApiWrapper(apiConfig).start();
+
+      // Load AND fully activate the default language extensions. Each module
+      // exports a `whenReady()` that resolves after its TextMate grammar /
+      // configuration files are registered with the host — without waiting,
+      // the editor opens with every token rendered as the default `mtk1`
+      // class (no syntax colours). Dynamic `import(...)` is used so the
+      // Angular build pipeline doesn't tree-shake the side-effect imports.
+      const extensions = await Promise.all([
+        import("@codingame/monaco-vscode-python-default-extension"),
+        import("@codingame/monaco-vscode-java-default-extension"),
+      ]);
+      await Promise.all(extensions.map(ext => ext.whenReady?.()));
+
+      CodeEditorComponent.apiWrapperStarted = true;
+    })();
+    return CodeEditorComponent.apiWrapperStartPromise;
+  }
+
   /**
    * Create a Monaco editor and connect it to MonacoBinding.
    * @private
    */
   private initializeMonacoEditor() {
     const fileSuffix = this.getFileSuffixByLanguage(this.language);
-    const userConfig: UserConfig = {
-      wrapperConfig: {
-        editorAppConfig: {
-          $type: "extended",
-          codeResources: {
-            main: {
-              text: this.code?.toString() ?? "",
-              uri: `in-memory-${this.currentOperatorId}.${fileSuffix}`,
-            },
-          },
-          userConfiguration: {
-            json: JSON.stringify({
-              "workbench.colorTheme": "Default Dark Modern",
-            }),
-          },
+    const editorAppConfig: EditorAppConfig = {
+      codeResources: {
+        modified: {
+          text: this.code?.toString() ?? "",
+          uri: `in-memory-${this.currentOperatorId}${fileSuffix}`,
         },
       },
     };
 
-    // optionally, configure python language client.
-    // it may fail if no valid connection is established, yet the failure 
would be ignored.
     const languageServerWebsocketUrl = getWebsocketUrl(
       "/python-language-server",
       this.config.env.pythonLanguageServerPort
     );
-    if (this.language === "python") {
-      userConfig.languageClientConfig = {
-        languageId: this.language,
-        options: {
-          $type: "WebSocketUrl",
-          url: languageServerWebsocketUrl,
-        },
-      };
-    }
 
-    // init monaco editor, optionally with attempt on language client.
-    from(this.editorWrapper.initAndStart(userConfig, 
this.editorElement.nativeElement))
+    const startEditor = async (): Promise<MonacoEditor | undefined> => {
+      await CodeEditorComponent.ensureVscodeApiStarted();
+      this.editorApp = new EditorApp(editorAppConfig);
+      await this.editorApp.start(this.editorElement.nativeElement);
+
+      // optionally, configure python language client.
+      // it may fail if no valid connection is established, yet the failure 
would be ignored.
+      if (this.language === "python") {
+        const lcConfig: LanguageClientConfig = {
+          languageId: this.language,
+          connection: {
+            options: {
+              $type: "WebSocketUrl",
+              url: languageServerWebsocketUrl,
+            },
+          },
+          clientOptions: {
+            documentSelector: [this.language],
+          },
+        };
+        this.languageClientWrapper = new LanguageClientWrapper(lcConfig);
+        await this.languageClientWrapper.start();
+      }
+      return this.editorApp.getEditor();
+    };
+
+    from(startEditor())
       .pipe(
         timeout(LANGUAGE_SERVER_CONNECTION_TIMEOUT_MS),
-        switchMap(() => of(this.editorWrapper.getEditor())),
-        catchError(() => of(this.editorWrapper.getEditor())),
+        // Language-server connection may time out (or fail) — fall back to 
the editor
+        // that was already mounted before `languageClientWrapper.start()` was 
awaited.
+        catchError(() => of(this.editorApp?.getEditor())),
         filter(isDefined),
         untilDestroyed(this)
       )

Review Comment:
   Good catch — fixed in 51b4fe5546.
   
   Pulled the timeout out of the rxjs pipeline and into a tight `Promise.race` 
around `languageClientWrapper.start()` only. The editor mount path 
(`ensureVscodeApiStarted()` + `EditorApp.start()`) is now always awaited to 
completion, and the rxjs pipeline is just 
`from(startEditor()).pipe(filter(isDefined), untilDestroyed(this))`. The unused 
`timeout` / `catchError` / `of` rxjs imports went with it.



##########
frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.ts:
##########
@@ -228,53 +224,121 @@ export class CodeEditorComponent implements 
AfterViewInit, SafeStyle, OnDestroy
     }
   }
 
+  /**
+   * Lazily start the global monaco-vscode-api wrapper. The vscode API 
services are
+   * a process-wide singleton in v10; calling start() twice would throw, so we 
share
+   * a single Promise across every CodeEditorComponent instance.
+   */
+  private static async ensureVscodeApiStarted(): Promise<void> {
+    if (CodeEditorComponent.apiWrapperStarted) {
+      return;
+    }
+    CodeEditorComponent.apiWrapperStartPromise ??= (async () => {
+      const apiConfig: MonacoVscodeApiConfig = {
+        $type: "extended",
+        viewsConfig: { $type: "EditorService" },
+        userConfiguration: {
+          json: JSON.stringify({ "workbench.colorTheme": "Default Dark Modern" 
}),
+        },
+        // Wire up the workers monaco-vscode-api spawns at runtime (editor,
+        // extension host, textmate). Each `new Worker(new URL(...))` literal
+        // points at a thin trampoline in `./workers/` that just re-imports the
+        // codingame-shipped worker entry. Two reasons for the indirection:
+        //   1. webpack 5 only treats `new Worker(new URL("./relative", 
import.meta.url))`
+        //      as a worker entry point (so it bundles the dep tree into a 
chunk);
+        //      `new URL("@codingame/...", import.meta.url)` would just emit a
+        //      static asset whose own relative imports 404 at runtime.
+        //   2. the test pipeline (esbuild via @angular/build:unit-test) 
resolves
+        //      `new URL(spec, import.meta.url)` literally relative to the 
source
+        //      file, so the spec needs to point at a real on-disk file. The
+        //      trampolines satisfy both bundlers.
+        monacoWorkerFactory: () => {
+          const env = getEnhancedMonacoEnvironment();
+          env.getWorker = (_workerId: string, label: string): Worker => {
+            switch (label) {
+              case "editorWorkerService":
+                return new Worker(new URL("./workers/editor.worker", 
import.meta.url), { type: "module" });
+              case "extensionHostWorkerMain":
+                return new Worker(new URL("./workers/extension-host.worker", 
import.meta.url), { type: "module" });
+              case "TextMateWorker":
+                return new Worker(new URL("./workers/textmate.worker", 
import.meta.url), { type: "module" });
+              default:
+                throw new Error(`No worker configured for label: ${label}`);
+            }
+          };
+        },
+      };
+      await new MonacoVscodeApiWrapper(apiConfig).start();
+
+      // Load AND fully activate the default language extensions. Each module
+      // exports a `whenReady()` that resolves after its TextMate grammar /
+      // configuration files are registered with the host — without waiting,
+      // the editor opens with every token rendered as the default `mtk1`
+      // class (no syntax colours). Dynamic `import(...)` is used so the
+      // Angular build pipeline doesn't tree-shake the side-effect imports.
+      const extensions = await Promise.all([
+        import("@codingame/monaco-vscode-python-default-extension"),
+        import("@codingame/monaco-vscode-java-default-extension"),
+      ]);
+      await Promise.all(extensions.map(ext => ext.whenReady?.()));
+
+      CodeEditorComponent.apiWrapperStarted = true;
+    })();
+    return CodeEditorComponent.apiWrapperStartPromise;
+  }

Review Comment:
   Fixed in 51b4fe5546.
   
   Wrapped the IIFE body in a try/catch and reset `apiWrapperStartPromise = 
undefined` before re-throwing, so a later open re-runs the start sequence 
instead of inheriting the rejection. `apiWrapperStarted` only flips to `true` 
on the success path.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to