imguolao / monaco-vue

Use monaco-editor loaded from CDN in Vue 2&3, no need to bundling.
https://imguolao.github.io/monaco-vue/
MIT License
211 stars 21 forks source link

It is not possible to load a custom monaco instance with `await import` #33

Closed JeanJPNM closed 10 months ago

JeanJPNM commented 11 months ago

I was trying to use the version of monaco-editor from my node_modules folder and bundle it using vite to only import the languages that the component uses, however, I couldn't find a way to use the dynamically imported monaco instance with the Editor component. ``:

monaco.ts

import editorWorker from "monaco-editor/esm/vs/editor/editor.worker?worker";
import tsWorker from "monaco-editor/esm/vs/language/typescript/ts.worker?worker";

// based on
// https://github.com/vitejs/vite/discussions/1791#discussioncomment-321046
// the other workers were removed since the editor
// only needs the typescript language server
self.MonacoEnvironment = {
  getWorker(_, label) {
    if (label === "typescript" || label === "javascript") {
      return new tsWorker();
    }
    return new editorWorker();
  },
};

export * from "monaco-editor";
<template>
  <Editor v-model:value="valueRef"></Editor>
</template>

<script setup>
import Editor from "@guolao/vue-monaco-editor";
import { onMounted, ref } from "vue";

const valueRef = ref("");

onMounted(async () => {
  const monaco = await import("./monaco");

  // the editor already loaded its own monaco instance
  // so calling loader.config({monaco}) won't work
});
</script>
imguolao commented 11 months ago

Maybe you should find a solution for packing webworker in the vite docs, Editor component loading monaco from a CDN, there are not support packing monaco from local floder, it is should be done by vite or rollup.

JeanJPNM commented 10 months ago

But the problem is not in packing the webworkers or importing them. The problem is that I can't tell @monaco-editor/loader, and by consequence, your editor, that I'm loading my own monaco instance and that it should not load its own from the CDN.

Because of the way the library is written, calling loader.config({ monaco }) will not cause any sort of update in the component, because it already loaded its own monaco instance.

I haven't found a workaround that doesn't involve making my own version of this library and passing a monaco ref via inject.

I here is a demonstration of the issue

imguolao commented 10 months ago

You're wanna to dynamically switch monaco-editor versions via the @monaco-editor/loader, but it doesn't work, right?

I can see what your saying, but we hadn't considered this senario.

As you can see from the following lines of code, the loader only loads once, repeating settings doesn't reload:

function init() {
  const state = getState(({ monaco, isInitialized, resolve }) => ({ monaco, isInitialized, resolve }));

  // look here, it is only executeds once.
  if (!state.isInitialized) {
    setState({ isInitialized: true });

    // something...
  }

  return makeCancelable(wrapperPromise);
}
imguolao commented 10 months ago

The principle of the @monaco-editor/loader is very simple: dynamically create a script element to loading monaco-editor.

The problem now is that @monaco-editor/loader is only exectued once. Maybe you can create a pr to fix this problem or fork this repo and rewrite to your needs.

JeanJPNM commented 10 months ago

After experimenting I found a possible solution, and I would like to see what you think of it.

changing useMonaco.ts:

import type { Nullable, MonacoEditor } from '../types'
import { InjectionKey, ShallowRef, inject, onMounted, provide, shallowRef } from 'vue-demi'
import loader from '@monaco-editor/loader'

interface MonacoContext {
  monacoRef?: ShallowRef<MonacoEditor | null>
}
const key: InjectionKey<MonacoContext> = Symbol()

// export this in index.ts
export function provideMonaco(context: MonacoContext) {
  provide(key, context)
}

// maybe export this too
export function injectMonaco() {
  return inject<MonacoContext>(key, () => ({}), true)
}

export function useMonaco() {
  // this declaration has been scope to
  // not modify the original monacoRef
  // declaration type, since this one may be undefined
  {
    const { monacoRef } = injectMonaco()
    if (monacoRef) return { monacoRef, unload() {} }
  }
  const monacoRef = shallowRef<Nullable<MonacoEditor>>(loader.__getMonacoInstance())

  let promise: ReturnType<(typeof loader)['init']>

  onMounted(() => {
    // the instance has already been loaded
    if (monacoRef.value) return

    promise = loader.init()
    promise
      .then(monacoInstance => (monacoRef.value = monacoInstance))
      .catch(error => {
        if (error?.type !== 'cancelation') {
          console.error('Monaco initialization error:', error)
        }
      })
  })

  // monaco mount
  const unload = () => promise?.cancel()

  return {
    monacoRef,
    unload,
  }
}

And then it would be used like this:

<script setup>
import Editor, { provideMonaco } from '@guolao/vue-monaco-editor';
import { onMounted, shallowRef } from 'vue'

const monacoRef = shallowRef()
provideMonaco({ monacoRef })
onMounted(async () => {
  monacoRef.value = await import("./my_monaco_module")
});
</script>

<template>
  <Editor language="typescript" /> 
</template>

This change is backwards-compatible, but I'm not very familiar with the provide/inject API to know if I should just export the key or if it's okay to export provideMonaco and injectMonaco.

imguolao commented 10 months ago

After experimenting I found a possible solution, and I would like to see what you think of it.

changing useMonaco.ts:

import type { Nullable, MonacoEditor } from '../types'
import { InjectionKey, ShallowRef, inject, onMounted, provide, shallowRef } from 'vue-demi'
import loader from '@monaco-editor/loader'

interface MonacoContext {
  monacoRef?: ShallowRef<MonacoEditor | null>
}
const key: InjectionKey<MonacoContext> = Symbol()

// export this in index.ts
export function provideMonaco(context: MonacoContext) {
  provide(key, context)
}

// maybe export this too
export function injectMonaco() {
  return inject<MonacoContext>(key, () => ({}), true)
}

export function useMonaco() {
  // this declaration has been scope to
  // not modify the original monacoRef
  // declaration type, since this one may be undefined
  {
    const { monacoRef } = injectMonaco()
    if (monacoRef) return { monacoRef, unload() {} }
  }
  const monacoRef = shallowRef<Nullable<MonacoEditor>>(loader.__getMonacoInstance())

  let promise: ReturnType<(typeof loader)['init']>

  onMounted(() => {
    // the instance has already been loaded
    if (monacoRef.value) return

    promise = loader.init()
    promise
      .then(monacoInstance => (monacoRef.value = monacoInstance))
      .catch(error => {
        if (error?.type !== 'cancelation') {
          console.error('Monaco initialization error:', error)
        }
      })
  })

  // monaco mount
  const unload = () => promise?.cancel()

  return {
    monacoRef,
    unload,
  }
}

And then it would be used like this:

<script setup>
import Editor, { provideMonaco } from '@guolao/vue-monaco-editor';
import { onMounted, shallowRef } from 'vue'

const monacoRef = shallowRef()
provideMonaco({ monacoRef })
onMounted(async () => {
  monacoRef.value = await import("./my_monaco_module")
});
</script>

<template>
  <Editor language="typescript" /> 
</template>

This change is backwards-compatible, but I'm not very familiar with the provide/inject API to know if I should just export the key or if it's okay to export provideMonaco and injectMonaco.

There are some problems with your suggestion.

  1. Using props to pass monacoRef is better than using provide & inject api.
  2. When using both @monaco-editor/loader and await import to danamically load two different monaco-editor over http, no way to known which https request will return result first (Race Condition).
  3. If we have solved the previously mentioned problem and changed the monacoRef variable correctly. And then, we still need to destroy the created editor instance and create a new editor instance. It will bring many breaking changes,causing me need a lot of time to test the change.

This reverses my idea of making monaco-editor easier to use.

imguolao commented 10 months ago

I'm not sure I'm misunderstanding what you are saying. If you only wanna package the monaco-editor file from your local node_modules, just use vite can solve it.

If you are wanna load two different version of the monaco-editor. I don't known why.

But any way, I have a suggestion that you can fork this repo and rewrite it to suit your needs, because this repo just simply wraps the monaco-editor in Vue, very seay.

JeanJPNM commented 10 months ago

When using both @monaco-editor/loader and await import to danamically load two different monaco-editor over http, no way to known which https request will return result first (Race Condition).

Alright, I guess I should have explained better:

I decided to use provide/inject because the component that is responsible for loading the monaco instance may be much higher up in the component tree than the component that is instantiating the editor (like a nuxt layout).

As a consequence of using provide/inject, the useMonaco hook can check if a parent component provided its own monaco instance ref, and if it does, then useMonaco returns it without invoking loader.init() and thus, without creating a race condition.

JeanJPNM commented 10 months ago

But any way, I have a suggestion that you can fork this repo and rewrite it to suit your needs, because this repo just simply wraps the monaco-editor in Vue, very seay.

Yeah, it's probably better if I just make a fork that fits my needs