graphql / graphiql

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

[Monaco-graphql] Monaco-graphql fails to build with latest versions of Vite, monaco-editor and monaco-graphql. #3639

Open skalidindi opened 4 months ago

skalidindi commented 4 months ago

Is there an existing issue for this?

Current Behavior

I get the following error when trying to run npm run build on the sample project provided below. It is a barebones skeleton React Vite starter app which imports monaco-editor and monaco-graphql in main.tsx and runs initializeMode. This was working in a previous version of monaco-editor 0.48.

Error: error during build: [vite]: Rollup failed to resolve import "monaco-editor/esm/vs/editor/contrib/hover/browser/hover" from "/Users/skalidindi/oss/vite-monaco-editor-build-bug/node_modules/monaco-graphql/esm/full.js". This is most likely unintended because it can break your application at runtime. If you do want to externalize this module explicitly add it to build.rollupOptions.external at viteWarn (file:///Users/skalidindi/oss/vite-monaco-editor-build-bug/node_modules/vite/dist/node/chunks/dep-C1-ZB6nQ.js:65876:17) at onwarn (file:///Users/skalidindi/oss/vite-monaco-editor-build-bug/node_modules/@vitejs/plugin-react-swc/index.mjs:203:9) at onRollupWarning (file:///Users/skalidindi/oss/vite-monaco-editor-build-bug/node_modules/vite/dist/node/chunks/dep-C1-ZB6nQ.js:65909:5) at onwarn (file:///Users/skalidindi/oss/vite-monaco-editor-build-bug/node_modules/vite/dist/node/chunks/dep-C1-ZB6nQ.js:65571:7) at file:///Users/skalidindi/oss/vite-monaco-editor-build-bug/node_modules/rollup/dist/es/shared/node-entry.js:18514:13 at Object.logger [as onLog] (file:///Users/skalidindi/oss/vite-monaco-editor-build-bug/node_modules/rollup/dist/es/shared/node-entry.js:20162:9) at ModuleLoader.handleInvalidResolvedId (file:///Users/skalidindi/oss/vite-monaco-editor-build-bug/node_modules/rollup/dist/es/shared/node-entry.js:19104:26) at file:///Users/skalidindi/oss/vite-monaco-editor-build-bug/node_modules/rollup/dist/es/shared/node-entry.js:19062:26

Repo: https://github.com/skalidindi/vite-monaco-editor-build-bug

Expected Behavior

I expect the error to not occur and the build to pass.

Steps To Reproduce

  1. Clone provided repo
  2. Run npm install
  3. Run npm run build

Environment

Anything else?

No response

RoccoC commented 3 months ago

This is also failing in my project, which uses Webpack, not Vite.

Looks like this is due to a recent change in monaco-editor-core where /vs/editor/contrib/hover/browser/hover was renamed to /vs/editor/contrib/hover/browser/hoverController: https://github.com/microsoft/vscode/pull/210219/files#diff-952bfc0bbfb137401f2692dfbf091b8cca1506a573b26242a0233b2845574b9d

I was able to work-around this by adding an alias to my webpack config:

const config = {
  // ...
  resolve: {
    alias: {
      'monaco-editor/esm/vs/editor/contrib/hover/browser/hover': 'monaco-editor/esm/vs/editor/contrib/hover/browser/hoverController',
    },
  },
};

I suspect that the monaco-editor-core hover/hoverController is an internal module and not intended for direct import by external packages, but I don't know enough about the monaco-editor ecosystem to know for sure. That said, I supose one way to address this would be to update the hover import in monaco-graphql to refer to the new file and to also update its peerDependency on monaco-editor to match. Happy to raise a PR if this is the right approach, but I'll leave that up to the maintainers. 🙂

acao commented 3 months ago

A PR would be welcome if you've tracked down a solution!

ThomasLaSalmonie commented 1 month ago

This workaround work only for monaco-editor >= "0.49.0" et monaco-editor < "0.52.0"

There is currently a lot of movements in renaming files directly in the vscode repo This one is already in the monaco-editor: "0.52.0" renaming and extract some code from hoverController => contentHoverController2 https://github.com/microsoft/vscode/commit/e4843ae75e2942ba8249849e62f8a39c053e227f

And maybe in the next release of monaco-editor: renaming contentHoverController2 => contentHoverController https://github.com/microsoft/vscode/commit/ef4eae349572d302002d10368f5f06d4d9e7b28e

Also I didn't dig deep into the code modification of each commit/release from vscode repo, but my use case of using monaco-graphql is working with those workarounds