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 to monaco-editor 0.40.0 and fitting monaco-vscode-api #512

Closed
kaisalmen opened this issue Jul 21, 2023 · 21 comments
Closed

Update to monaco-editor 0.40.0 and fitting monaco-vscode-api #512

kaisalmen opened this issue Jul 21, 2023 · 21 comments
Assignees

Comments

@kaisalmen
Copy link
Collaborator

kaisalmen commented Jul 21, 2023

Started with using monaco-vscode-api@1.80.0-next.3 based on CodinGame/monaco-vscode-api#146

Implementing changes on branch: https://github.com/TypeFox/monaco-languageclient/tree/monaco-0.40

@kaisalmen kaisalmen self-assigned this Jul 21, 2023
@kaisalmen
Copy link
Collaborator Author

@CGNonofr I updated to 0.80.0-next.5 and now resolution of onig.wasm fails with webpack and also during dev with vite. The relative resolution does not work.
Is the extensions service now required?

@CGNonofr
Copy link
Collaborator

@CGNonofr I updated to 0.80.0-next.5 and now resolution of onig.wasm fails with webpack and also during dev with vite. The relative resolution does not work.

What is the error? We should add a webpack demo here

Is the extensions service now required?

Yes but it's included by default

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Jul 26, 2023

What is the error? We should add a webpack demo here

We have that, see packages/verify/webpack. It is build when you execute npm run build. That is the issue:
https://github.com/TypeFox/monaco-languageclient/actions/runs/5669420808/job/15362085854
image

The other error regarding extensions service is still there as well. I deleted node-modules and package-lock file beforehand.

@CGNonofr
Copy link
Collaborator

Ok I understand the issue with the first one, will have a look

Regarding the extensions service issue, I've just reproduced 👍

@kaisalmen
Copy link
Collaborator Author

Regarding the extensions service issue, I've just reproduced 👍

The feeling of relief when one is able to reproduce a problem 😆

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Jul 26, 2023

@CGNonofr FYI, I just released a new next version based on the current progress (monaco-editor@0.40.0 / monaco-vscode-api@1.80.0-next.6):
https://www.npmjs.com/package/monaco-languageclient/v/6.3.0-next.0

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Jul 27, 2023

@CGNonofr 1.80.1-next.0 works, but I needed to init the quickAccessService with empty configuration. If I want to configure it like you do in your demo I get the folowing error once I add import { isEditorPartVisible } from 'vscode/service-override/views':
image

Once I remove the import, the problem is gone. Is this expected?

@kaisalmen
Copy link
Collaborator Author

New next version is available:
https://www.npmjs.com/package/monaco-languageclient/v/6.3.0-next.1

@CGNonofr
Copy link
Collaborator

CGNonofr commented Aug 4, 2023

@CGNonofr 1.80.1-next.0 works, but I needed to init the quickAccessService with empty configuration. If I want to configure it like you do in your demo I get the folowing error once I add import { isEditorPartVisible } from 'vscode/service-override/views': image

Once I remove the import, the problem is gone. Is this expected?

Views service override shouldn't be imported if editor service override is. don't import isEditorPartVisible if you don't use the service override, just return false

@kaisalmen
Copy link
Collaborator Author

Ok, got it. I didn't realize you used the views service in the demo.

@kaisalmen
Copy link
Collaborator Author

@CGNonofr I found a nice way forward: 95e0b69

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Aug 4, 2023

@CGNonofr I was just wondering if there is now still the need to override registerBuiltinFeatures. The only thing we did not include was:

this.registerFeature(new ProgressFeature(this));
this.registerFeature(new NotebookDocumentSyncFeature(this));

WDYT?

@CGNonofr
Copy link
Collaborator

CGNonofr commented Aug 4, 2023

Probably not indeed...

@kaisalmen
Copy link
Collaborator Author

This lib shrinks further....
Then this hack becomes obsolete as well, right?
https://github.com/TypeFox/monaco-languageclient/blob/main/packages/client/src/monaco-language-client.ts#L56-L71

@kaisalmen
Copy link
Collaborator Author

@CGNonofr with 1.80.2 when I enable the view service and have everything configured except for terminal and markers service I still get this (when I use the editor service instead everything is fine). Is this expected behaviour? What do I have to do to run the views service successfully?

image

@kaisalmen
Copy link
Collaborator Author

I will push an update in a bit to #513

@kaisalmen
Copy link
Collaborator Author

@CGNonofr Official release 6.3.0 is now available:
https://www.npmjs.com/package/monaco-languageclient/v/6.3.0

@CGNonofr
Copy link
Collaborator

CGNonofr commented Aug 4, 2023

@CGNonofr with 1.80.2 when I enable the view service and have everything configured except for terminal and markers service I still get this (when I use the editor service instead everything is fine). Is this expected behaviour? What do I have to do to run the views service successfully?

image

If you use the view service override, you have to render the editor part or there will be no activeGroup, leading to this error

@kaisalmen
Copy link
Collaborator Author

you have to render

Can you explain this or show me using your demo how this is realized?

@CGNonofr
Copy link
Collaborator

CGNonofr commented Aug 5, 2023

you have to render

Can you explain this or show me using your demo how this is realized?

I'm talking about https://github.com/CodinGame/monaco-vscode-api/blob/339a825ce83b5a88ec2100135c3068d76fe62bfc/demo/src/setup.ts#L86

@kaisalmen
Copy link
Collaborator Author

Thank you

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

No branches or pull requests

2 participants