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

WIP: feat(angular): add groovy in angular #772

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ls-infra
Copy link
Contributor

@ls-infra ls-infra commented Oct 24, 2024

as discussed in #763 , adding groovy editor to angular to support multiple lang mode like https://microsoft.github.io/monaco-editor/ , (this will be a good starting point as well to attract angular devs to contribute to enhance the wrapper together.)

but i am getting below error ,
my first guess will be that https://github.com/TypeFox/monaco-languageclient/blob/main/verify/angular/package.json#L23 is overriding the original monaco-editor modules ?

image

thanks

@ls-infra
Copy link
Contributor Author

hi , @kaisalmen , any idea for the above screenshot compile error ?

i added https://github.com/TypeFox/monaco-languageclient/pull/772/files#diff-6ccdef4ee1898bb434934b3a9d897e4abe51c9a6359ff77ebf055eb16bc1b5a3 but it does not seem like it will fix it .

thanks,

@kaisalmen
Copy link
Collaborator

Hi @ls-infra you can re-use the default editor worker loading instructions exported by the examples. The pre-compiled version of the wrapper is not required. I pushed the proposed changes here:
https://github.com/TypeFox/monaco-languageclient/tree/feature/angular-groovy

It is one commit on top of yours (commit changes)

@ls-infra
Copy link
Contributor Author

the code completion still not working in groovy(i used that docker compose ) but working in json
image
image

for the groovy connection to ws is green 101 but the JSONRPC WS message is not triggering when typing in editor .

image

i tried to run the npm run dev to see the groovy demo in /packages/examples/src/groovy
got below error , this below is reproducible in the main branch as well.

✘ [ERROR] Failed to resolve entry for package "@typefox/monaco-editor-react". The package may have incorrect main/module/exports specified in its package.json. [plugin vite:dep-scan]

@kaisalmen
Copy link
Collaborator

i tried to run the npm run dev to see the groovy demo in /packages/examples/src/groovy
got below error , this below is reproducible in the main branch as well.

I don't see this error. The groovy example works on main. Did you try to start fresh (clean node_modules, etc.)? There is npm run reset:repo that performs git clean on the repo.

There is now one other multi-languageclient example:
https://github.com/TypeFox/monaco-languageclient/blob/main/packages/examples/src/multi/twoLanguageClients.ts

But it uses one editor and switches the code. It may be that there is a problem when you start two wrappers with two different languageClientWrapper configurations. I haven't tested this, yet. There could something go wrong in the global monaco-editor "wiring". Can you only use one editor for now?

@ls-infra
Copy link
Contributor Author

the http://localhost:20001/packages/examples/groovy.html works ,
but the angular one is still not work with below errors :
.angular/cache/18.2.10/ng-mlc/vite/deps/resources/package.nls.json 404 (Not Found)
.angular/cache/18.2.10/ng-mlc/vite/deps/resources/dark_modern.json 404 (Not Found)

i just added the d1a1dc7 ,
seems like related , but still not working ,

thanks

@kaisalmen
Copy link
Collaborator

@ls-infra we can't unfortunately use @codingame/esbuild-import-meta-url-plugin even though angular now uses vite/esbuild. I haven't a way to make that work, yet. Production builds are fine.

@ls-infra
Copy link
Contributor Author

ls-infra commented Nov 3, 2024

@codingame/esbuild-import-meta-url-plugin is not part of the change in this PR . it is in the exiting code .
do you mean we should remove it ?

the build is fine but the error related to package.nls.json and dark_modern.json is in run time not build time .
and this two error is blocking the angular working with the Groovy lang server .

any idea i need to change to fix this ?
thx

@kaisalmen
Copy link
Collaborator

@codingame/esbuild-import-meta-url-plugin is not part of the change in this PR . it is in the exiting code .
do you mean we should remove it ?

No, the vite dev server uses esbuild and the plugin is used to overcome a problem with import.meta.url during dev (runtime!). This problem does not occur in production when rollup is used.

The angular dev server also uses vite and esbuild, but I tried to use that plugin in that past, but it doesn't work. Therefore we have this problem. With custom webpack configuration the problems were worse.

Investigations with these kind of problems is time consuming and have not found a solution, yet

@ls-infra
Copy link
Contributor Author

ls-infra commented Nov 5, 2024

does it mean that the angular wrapper can not work with Groovy lang server until further investigation on vite dev server .
but the angular wrapper works with the JSON lang server tho,
i wonder what is the difference .
thx

@kaisalmen
Copy link
Collaborator

does it mean that the angular wrapper can not work with Groovy lang server until further investigation on vite dev server .

It is probably related to the different/ kind of resources that are being loaded. Does the problem exist If only thegroovy ls is used?

@ls-infra
Copy link
Contributor Author

ls-infra commented Nov 6, 2024

the vanilla js with Groovy LSP works well (https://github.com/TypeFox/monaco-languageclient/blob/main/packages/examples/src/groovy/client/main.ts) ,
and the angular with JSON LSP works well (https://github.com/TypeFox/monaco-languageclient/blob/main/verify/angular/src/app/app.component.ts#L31)

just the angular with Groovy LSP does not work well and have issue as shown in the PR.

thx

@kaisalmen
Copy link
Collaborator

@ls-infra I need to dig into this, but did not yet find the time. I hope that will be possible next week.

@kaisalmen
Copy link
Collaborator

I would like to see this working as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants