[monaco-graphql] Cannot use the monaco-editor-webpack features toggle

[bboure]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

I'm trying to use the monaco-editor-webpack-plugin in combination with monaco-graphql and I am trying to disable some features using the new features flag.

However, the disabling of the flag does not work.

Here is my webpack config

new MonacoWebpackPlugin({
      languages: ['json', 'typescript', 'graphql'],
      features: ['!readOnlyMessage'],
      customLanguages: [
        {
          entry: undefined,
          label: 'graphql',
          worker: {
            id: 'graphql',
            entry: require.resolve('monaco-graphql/esm/graphql.worker.js'),
          },
        },
      ],
    }),

I traced the issue and it turns out that monaco-graphql imports edcore.main here which in turns imports all the features by default through editor.all.js

Expected Behavior

the features flag should be respected.

I think the solution is to only import from monaco-editor/esm/vs/editor/editor.api anywhere in monaco-graphql.

Especially in initializeMode.ts. It should be possible since the only thing we need is language

Edit: importing from monaco-editor/esm/vs/editor/editor.api is actually what the doc recommends/requires when using the webpack plugin.

Anything else (features, language workers, etc) should be imported/initialized by the developer in their own project through import 'monaco-editor`.

Steps To Reproduce

No response

Environment

• GraphiQL Version:
• OS:
• Browser:
• Bundler:
• react Version:
• graphql Version:

Anything else?

No response

[bboure]

The workaround is to play with overrides, which works but is not ideal

new NormalModuleReplacementPlugin(
      /^\.\/monaco-editor$/,
      'monaco-editor/esm/vs/editor/editor.api',
),

[acao]

This is connected to #3381 and I think requires reverting this PR for now

#3285

(update: we are not able to automatically revert #3285, so we will have to manually revert these changes)

[acao]

@bboure i think this workaround may also break json completion for variables, and a few other imports from monaco-editor that don't point to editor.api

[bboure]

I saw that issue and I was going to mention it too. It's the same root issue.

The workaround seems to work was long as the json worker is loaded with the WebPack plugin

[bboure]

And reverting the PR will not solve my issue since the full monaco-editor package (default) is imported, which has the same effect.

In my project I had to change all imports to editor.api, otherwise, the fully-featured editor is always loaded and my Webpack config is ignored completely.

There could be an escape hatch for custom cases like mine.

[acao]

we originally imported from editor.api directly, but at some point that was changed. i think there may also be new changes in monaco-editor exports causing some of these issues

[acao]

These seem to be the minimal core features we need for our mode to work. We could import 0 of these features, but then users would need to manually enable completion, hover, etc. What do you think?

edcore imports editor.all which imports all language features, including ones we don't use

[bboure]

This looks good and it makes sense I think. Users using this plugin will probably want these features anyway.

But I'm wondering if it's necessary. Most users will probably already import monaco-editor in their own project in one way or another, except in some rare cases, which will load all these features already.
Unless they are trying to be specific with the features and import only a few ones, which is what I'm trying to do.

It would be great if monaco-graphql only imported what it needs. For what I can see and unless I missed something, in this case it's languages, Uri and a few TS types, which should all be provided by editor.api already.

With that said, changing it now might be a breaking change to anyone not importing monaco-editor explicitely, so I'm good with the proposed solution.

Maybe in the next version bump, we can change it?

Note it still does not fix #3381 which is also a breaking change. But at this point there is no easy fix to everything I think

[acao]

if we only import editor.api, then we do not get completion, formatting, or other features provided by the mode
this will already be a breaking change for users, so I'm trying to make it less severe at least, and to prevent "mode doesn't work" bug reports when users learn they have to manually import completion/etc features

[acao]

perhaps we could have a monaco-graphql/lite import that only imports editor.api, and requires opt in to each of these features? I can already think of how to shift things around to implement it

[bboure]

👍 opt-out with monaco-graphql/light would be a great solution.