graphql / graphiql

GraphiQL & the GraphQL LSP Reference Ecosystem for building browser & IDE tools.
MIT License
16.09k stars 1.72k forks source link

[monaco-graphql] Unable to use TypeScript language in combination #3381

Open RedMser opened 1 year ago

RedMser commented 1 year ago

After loading monaco-graphql, I am unable to create a monaco editor instance that uses the typescript language. Monaco only tries loading the regular editor worker, and not the TS LSP worker anymore.

Commenting out the import statement for monaco-graphql makes TypeScript work again.

It's possible that this is a build pipeline issue, but even in that case I would appreciate some pointers that would allow me to further investigate and fix the issue.

For reproduction and screenshots: https://github.com/graphql/graphiql/issues/3381#issuecomment-1659135207

acao commented 1 year ago

@RedMser how are you loading the language workers, with a webpack or vite plugin? or manually?

is the issue just with types or at runtime as well?

RedMser commented 1 year ago

I'm using webpack through angular, manually loaded using code similar to this: https://github.com/miki995/ngx-monaco-editor-v2/blob/master/projects/editor/src/lib/base-editor.ts#L46

It's a runtime problem, as in the monaco editor in my app looks like it has language set to plain text (no error squiggles, no syntax highlight, etc.) even though it's typescript. Works fine when I remove the import.

I'll try debugging this myself a bit, but maybe it can be reproduced in one of the examples on this repo? I haven't tried yet...

RedMser commented 1 year ago

Aha! @acao I can reproduce it in the examples/monaco-graphql-webpack example.

Here's my branch so you can take a better look: https://github.com/graphql/graphiql/compare/main...RedMser:graphiql:repro-3381

I noticed that I got an eslint warning in this project:

monaco-editor imports all languages; use monaco-graphql/esm/monaco-editor instead to import only json and graphql languages

So I assume that not including TypeScript by default is intentional. I just don't know why this propagates through the entire project the moment you import monaco-graphql...

acao commented 1 year ago

it was an eslint rule added by @B2o5T , but this change should only impact linting, and the related change should only impact types

acao commented 1 year ago

I will look into this bug this week, hoping we can find a solution that avoids importing all languages by default, but that doesn't cause this regression either

acao commented 1 year ago

@RedMser I had to import these client side to get typescript working with the vite plugin. configuring languageWorkers in the vite plugin only loaded the typescript language worker, but did not import the client side modes

import 'monaco-editor/esm/vs/basic-languages/typescript/typescript.contribution';
import 'monaco-editor/esm/vs/editor/contrib/peekView/browser/peekView';
import 'monaco-editor/esm/vs/editor/contrib/parameterHints/browser/parameterHints';
import 'monaco-editor/esm/vs/editor/contrib/hover/browser/hover';
import 'monaco-editor/esm/vs/language/typescript/monaco.contribution';

update: I found that enabling typescript in the webpack monaco editor plugin settings led to this error, so I had to use the above approach and manually enable the typescript worker as such:

 new MonacoWebpackPlugin({
          // you can add other languages here as needed
          languages: ['json', 'graphql'],
          filename: 'static/[name].worker.js',
          // this is not in the plugin readme, but saves us having to override
          // MonacoEnvironment.getWorkerUrl or similar.
          customLanguages: [
            {
              label: 'graphql',
              worker: {
                id: 'graphql',
                entry: 'monaco-graphql/esm/graphql.worker.js',
              },
            },
            {
              label: 'typescript',
              worker: {
                id: 'typescript',
                entry: 'monaco-editor/esm/vs/language/typescript/ts.worker.js',
              },
            },
          ],
        })

here is an example with the working typescript mode:

image

acao commented 1 year ago

here are a few examples @RedMser , with both vite and webpack (next.js)

https://github.com/graphql/graphiql/pull/3384

RedMser commented 1 year ago

Thank you for the quick update! I'll take a look and let you know if this resolves the problem for me.

RedMser commented 1 year ago

@acao I moved from the AMD loader to an ESM loader, tried out the imports you listed, and got TypeScript working together with monaco-graphql loaded. I didn't try setting up a GraphQL schema or anything yet, so no idea if the graphql part works, but at least everything loads properly now. Thank you! :)

So the PR you linked will, when merged, basically just end up reducing the bundle size (by not including all basic languages for example)?