Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update generator-langium web template #1205

Merged
merged 10 commits into from
Oct 27, 2023
Merged

Update generator-langium web template #1205

merged 10 commits into from
Oct 27, 2023

Conversation

kaisalmen
Copy link
Contributor

@kaisalmen kaisalmen commented Sep 27, 2023

We need to wait for monaco-editor-wrapper@3.3.0 (PR54) before we should merge this change. (merged)

This PR fixes #1163 and updates the web template to the latest monaco-editor-wrapper.
It is now possible to use the textmate grammar definition as well as the monarch grammar definitions with monaco-editor. The underlying monaco-vscode-api exposes various vscode service overrides that make this possible. Therefore we now generate two HTML examples and a top-level index page.

The generated output no longer requires express. vite+rollup are used to produce the bundles, so the output can be used/served as is. After the generation/template building is complete the output can be tested using vite's dev server.

Copy link
Contributor

@montymxb montymxb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One apparent bug with the generated package.json, and a few comments, but nothing else that I could find. Seems like the rest is working as intended.

@kaisalmen
Copy link
Contributor Author

kaisalmen commented Sep 29, 2023

@montymxb code is updated to final monaco-editor-wrapper@3.2.1. Bugfix release because I broke awaitExtensionReadiness.

@kaisalmen kaisalmen force-pushed the kaisalmen/issue-1163 branch 2 times, most recently from 423d658 to 0708200 Compare October 6, 2023 19:00
@kaisalmen
Copy link
Contributor Author

Rebased to latest main and updated to latest monaco-editor-wrapper.

@Yokozuna59
Copy link
Contributor

Hi @kaisalmen, It may be a silly question, but what's the difference between classic and vscodeApi? And is there a way to test it out before the next release?

@kaisalmen
Copy link
Contributor Author

kaisalmen commented Oct 7, 2023

@Yokozuna59 it is basically this:

  • classic: Configure monaco-editor as you would when using it directly using Monarch grammar definition
  • Extended: Configure monaco-editor like a VSCode extension using Textmate grammar definition

The extended mode allows all the fancy things from: monaco-vscode-api

There is now another PR in the pipeline for the wrapper that makes the mode separation clearer (terminology and code): TypeFox/monaco-components#54

You can checkout the PR locally and use the generator to try it out. It generates both versions. You can navigate from the index page:
image

@Yokozuna59
Copy link
Contributor

@kaisalmen I see, thanks for illustration.

There is now another PR in the pipeline for the wrapper that makes the mode separation clearer (terminology and code): TypeFox/monaco-components#54

Is there an estimated time when it will be merged? After looking at the changes in that PR, it contains a lot of breaking changes, and it would be hard to reimplement the code to inline it with the next major release.

@kaisalmen
Copy link
Contributor Author

kaisalmen commented Oct 8, 2023

After looking at the changes in that PR, it contains a lot of breaking changes, and it would be hard to reimplement the code to inline it with the next major release.

@Yokozuna59 the changes in monaco-editor-wrapper PR 54 are mostly internal and don't change the external API much. The only change required here is to change from $type from vscode to extended apart from updating the version. next versions are available on npm (see description of #54)

... and it will remove the need for two bundles as more config is not made internally (less work for end-user)

@Yokozuna59
Copy link
Contributor

Hi @kaisalmen, just a simple question, is there a reason why vscode-languageserver isn't a dependency in the template? Both web and vscode templates uses it:

I asked that because when using npm package manager, it works fine and build without issues. But I switched to pnpm it started throwing errors:

> hello-world@0.0.1 build hello-world
> tsc -b tsconfig.json && node esbuild.mjs

src/language/main-browser.ts:6:8 - error TS2307: Cannot find module 'vscode-languageserver/browser.js' or its corresponding type declarations.

6 } from 'vscode-languageserver/browser.js';
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

src/language/main.ts:3:52 - error TS2307: Cannot find module 'vscode-languageserver/node.js' or its corresponding type declarations.

3 import { createConnection, ProposedFeatures } from 'vscode-languageserver/node.js';
                                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~


Found 2 errors.

 ELIFECYCLE  Command failed with exit code 1.

@Yokozuna59
Copy link
Contributor

Yokozuna59 commented Oct 11, 2023

I was able to resolve some of the errors that were thrown when switching to pnpm:

  1. install missing dependencies:

    pnpm add monaco-languageclient@6.5.2 \
      vscode-languageserver@8.0.2 \
      vscode@npm:@codingame/monaco-vscode-api@1.82.4

Note: installing @codingame/monaco-vscode-api is only to resolve the peerDependencies missing issue.

2. Make vscode as external dependency:

This change broke the editor.

// vite.bundle.classic.config.ts
export default defineConfig({
   build: {
       ...,
       rollupOptions: {
           external: ['vscode'],
           ...,
       }
   }
});

These changes don't have any impact when using npm.


But I don't know how to resolve other issues:

> hello-world@0.0.1 build:bundles
> vite --config ./vite.bundle.classic.config.ts build && vite --config ./vite.bundle.vscodeApi.config.ts build

vite v4.4.11 building for production...
✓ 8 modules transformed.
✓ built in 184ms
[vite]: Rollup failed to resolve import "monaco-editor/esm/vs/platform/userDataProfile/common/userDataProfile.js" from "hello-world/node_modules/.pnpm/@codingame+monaco-vscode-api@1.82.4/node_modules/@codingame/monaco-vscode-api/service-override/configuration.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`
error during build:
Error: [vite]: Rollup failed to resolve import "monaco-editor/esm/vs/platform/userDataProfile/common/userDataProfile.js" from "hello-world/node_modules/.pnpm/@codingame+monaco-vscode-api@1.82.4/node_modules/@codingame/monaco-vscode-api/service-override/configuration.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://hello-world/node_modules/.pnpm/vite@4.4.11_@types+node@20.8.4/node_modules/vite/dist/node/chunks/dep-2b82a1ce.js:48205:27)
    at onRollupWarning (file://hello-world/node_modules/.pnpm/vite@4.4.11_@types+node@20.8.4/node_modules/vite/dist/node/chunks/dep-2b82a1ce.js:48237:9)
    at onwarn (file://hello-world/node_modules/.pnpm/vite@4.4.11_@types+node@20.8.4/node_modules/vite/dist/node/chunks/dep-2b82a1ce.js:47965:13)
    at file://hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:24276:13
    at Object.logger [as onLog] (file://hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:25950:9)
    at ModuleLoader.handleInvalidResolvedId (file://hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:24862:26)
    at file://hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:24822:26
 ELIFECYCLE  Command failed with exit code 1.

And this issue occurs for different imports all over @codingame/monaco-vscode-api. I sat all those files as external as workaround, but another issue was thrown:

> hello-world@0.0.1 build:bundles
> vite --config ./vite.bundle.classic.config.ts build && vite --config ./vite.bundle.vscodeApi.config.ts build

vite v4.4.11 building for production...
✓ 2044 modules transformed.
✓ built in 8.85s
"FileType" is not exported by "node_modules/.pnpm/monaco-editor@0.43.0/node_modules/monaco-editor/esm/vs/platform/files/common/files.js", imported by "node_modules/.pnpm/@codingame+monaco-vscode-api@1.82.4/node_modules/@codingame/monaco-vscode-api/vscode/src/vs/workbench/common/editor.js".
file: /hello-world/node_modules/.pnpm/@codingame+monaco-vscode-api@1.82.4/node_modules/@codingame/monaco-vscode-api/vscode/src/vs/workbench/common/editor.js:7:9
5: import { IInstantiationService } from 'monaco-editor/esm/vs/platform/instantiation/common/instantiation.js';
6: import { Registry } from 'monaco-editor/esm/vs/platform/registry/common/platform.js';
7: import { FileType } from 'monaco-editor/esm/vs/platform/files/common/files.js';
            ^
8: import { Schemas } from 'monaco-editor/esm/vs/base/common/network.js';
9: import { isErrorWithActions, createErrorWithActions } from 'monaco-editor/esm/vs/base/common/errorMessage.js';
error during build:
RollupError: "FileType" is not exported by "node_modules/.pnpm/monaco-editor@0.43.0/node_modules/monaco-editor/esm/vs/platform/files/common/files.js", imported by "node_modules/.pnpm/@codingame+monaco-vscode-api@1.82.4/node_modules/@codingame/monaco-vscode-api/vscode/src/vs/workbench/common/editor.js".
    at error (file:///hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:2287:30)
    at Module.error (file:///hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:13745:16)
    at Module.traceVariable (file:///hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:14175:29)
    at ModuleScope.findVariable (file:///hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:12615:39)
    at FunctionScope.findVariable (file:///hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:7124:38)
    at ChildScope.findVariable (file:///hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:7124:38)
    at ReturnValueScope.findVariable (file:///hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:7124:38)
    at ChildScope.findVariable (file:///hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:7124:38)
    at TrackingScope.findVariable (file:///hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:7124:38)
    at BlockScope.findVariable (file:///hello-world/node_modules/.pnpm/rollup@3.29.4/node_modules/rollup/dist/es/shared/node-entry.js:7124:38)
 ELIFECYCLE  Command failed with exit code 1

I also tried to install monaco-editor@0.43.0, but it has no effect.

@kaisalmen
Copy link
Contributor Author

@Yokozuna59 thank you very much for your feedback. I will answer your feedback soon.
It is already clead to me that this PR should wait for monaco-editor-wrapper@3.3.0 as it may already solve some of your issues and also it addresses some shortcoming I identified myself.

@kaisalmen
Copy link
Contributor Author

vscode-languageserver isn't a dependency in the template

You are correct. It works with npm, because it is a transitive dependency of langium, but it should be declared here.

@Yokozuna59
Copy link
Contributor

@kaisalmen Thanks for your fast response.

I guess the template .gitignore file has missing attributes.

static/bundle/
static/monaco-editor-workers/
static/worker/

@kaisalmen kaisalmen force-pushed the kaisalmen/issue-1163 branch from da600ff to 437504a Compare October 13, 2023 20:18
@kaisalmen
Copy link
Contributor Author

@montymxb this ready now. It is based on the final version 3.3.0 of monaco-editor-wrapper
@Yokozuna59 This version is now fully working with pnpm out of the box.

@kaisalmen
Copy link
Contributor Author

@msujew this is ready. It needs approval from a core maintainer. 🙂

@kaisalmen kaisalmen force-pushed the kaisalmen/issue-1163 branch from 39dbfff to ee77476 Compare October 25, 2023 06:33
@kaisalmen
Copy link
Contributor Author

@msujew the rebase updates vscode-languageserver and vscode-languageclient libraries to 9.0.1.

Important: This will only work properly with a new langium release or a locally linked version. If you just check out and build it will fail, because Langium 2.0.x references another version that breaks the compilation.

@spoenemann
Copy link
Contributor

It needs approval from a core maintainer. 🙂

All committers listed here are allowed to approve and merge PRs:
https://projects.eclipse.org/projects/ecd.langium/who

Copy link
Contributor

@montymxb montymxb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:octocat: approved! Generated project is good (although until the new langium release comes out, the generator will have a small mismatch).

@kaisalmen kaisalmen merged commit 1ec85f1 into main Oct 27, 2023
3 checks passed
@kaisalmen kaisalmen deleted the kaisalmen/issue-1163 branch October 27, 2023 07:43
@msujew msujew added this to the v2.1.0 milestone Nov 1, 2023
@msujew msujew added the yeoman Yeoman generator related issue label May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
yeoman Yeoman generator related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Browser template should be updated
5 participants