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 [email protected] and [email protected] #544

Merged
merged 5 commits into from
Sep 28, 2023
Merged

Conversation

kaisalmen
Copy link
Collaborator

@kaisalmen kaisalmen commented Sep 27, 2023

Changes:

  • Update all dependencies:
  • Removed dynamic imports:
    • initServices add both language and model service always and they are therefore a direct dependency of monaco-languageclient. Without them using this lib doesn't really make sense.
    • The examples package define the extra services and default-extensions the examples require in their dependencies

TODO:

  • Update README
  • Use final version of @codingame/monaco-vscode-api as soon as it released.

@kaisalmen kaisalmen force-pushed the mva-1.82.0 branch 2 times, most recently from 5da5529 to b7abbe8 Compare September 28, 2023 08:21
@kaisalmen kaisalmen marked this pull request as ready for review September 28, 2023 08:26
@kaisalmen kaisalmen requested a review from CGNonofr as a code owner September 28, 2023 08:26
@kaisalmen
Copy link
Collaborator Author

@CGNonofr it is working nicely after I adapted the vite / experimental node config

- Updated service dependencies
- Updated repository urls
@kaisalmen
Copy link
Collaborator Author

@CGNonofr
Copy link
Collaborator

@CGNonofr it is working nicely after I adapted the vite / experimental node config

Wonderful! Don't you want me to wait for the monaco-vscode-api final release before approving it?

@kaisalmen
Copy link
Collaborator Author

Wonderful! Don't you want me to wait for the monaco-vscode-api final release before approving it?

Yes, I also want/force-me to update the README on this branch. 🙂

@kaisalmen
Copy link
Collaborator Author

@CGNonofr updated to 1.82.2
Next version is available: https://www.npmjs.com/package/monaco-languageclient/v/6.5.0-next.1

The README is basically ready. Should we remove the monaco-editor-core section from the troubleshooting section. This no longer relevant. WDYT?

- Fixed exports
- Examples: Removed unneeded views service dependency
@kaisalmen
Copy link
Collaborator Author

@CGNonofr this is now ready to be merged. Further README clarification don't block this, I think.

packages/client/package.json Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@CGNonofr CGNonofr self-requested a review September 28, 2023 13:01
@kaisalmen kaisalmen merged commit 79ecd5a into main Sep 28, 2023
@kaisalmen kaisalmen deleted the mva-1.82.0 branch September 28, 2023 13:02
@kaisalmen kaisalmen restored the mva-1.82.0 branch September 28, 2023 13:02
@kaisalmen kaisalmen deleted the mva-1.82.0 branch September 28, 2023 13:02
@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Sep 28, 2023

@CGNonofr I have not released a final version, because I discovered a nasty problem in monaco-editor-wrapper and here is the solution:
https://github.com/TypeFox/monaco-components/blob/next-mlc/packages/examples/src/langium/config/wrapperLangiumVscode.ts#L42-L43
https://github.com/TypeFox/monaco-components/blob/next-mlc/packages/monaco-editor-wrapper/src/editorAppBase.ts#L212-L221
When you load an extension for a language (Langium Web Worker LS) you now have to await the loading of the theme before you load the language extension. Otherwise the theme does not work. This behaviour changed with [email protected]. It was no problem before. Are you aware of this change in behaviour?

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Sep 29, 2023

@CGNonofr the problem is that sometimes the loading of extensions (e.g. themes) happens before and sometimes after. It seems that something that has been awaited before (1.81.x) and now not (1.82.x). Maybe something needs to be adjusted in monaco-vscpde-api. I haven't looked into the code. It is anyway good I have the other mechanism above in place.

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