-
-
Notifications
You must be signed in to change notification settings - Fork 197
Update to monaco-vscode-api v13 #836
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
Conversation
Updated dependencies
@CGNonofr I see this error when the view service is used in the app playground example in the browser console: and in the "rerender" react component test, but nowhere else in the tests: I don't see this in your deployed demo or anywhere else in the examples here or apart from this one test (I pin-pointed this locally). Any idea why this comes from suddenly? I set |
- Remove pre-build workers from wrapper
@CGNonofr there are some problems with peerDependencies. I think. I will open an issue https://github.com/CodinGame/monaco-vscode-api and hopefully are able to describe it in an understandable way |
I'm tired of that error, it almost as old as monaco itself: microsoft/monaco-editor#4311 but it has no other consequences than an error in the logs 🤷
Are you sure? I can see it in https://monaco-vscode-api.netlify.app/, and also locally (just not in full-workbench mode)
I'm scared! any insight yet? |
@CGNonofr am still digging. Getting there. Btw, pnpm is very helpful here, because it is way faster (due to its cache) and better standard reporting |
Maybe my browser cache was not cleaned |
@CGNonofr I just pushed an update that builds three very simple projects with npm, pnpm and yarn just to check if basic dependencies work. |
aeb4bf5
to
e302952
Compare
@CGNonofr back to the simple dependency approach. I was thinking about providing a node based script we could export with |
@CGNonofr this is now ready for review. The proposed script may come later. |
btw, something can be done at runtime: we store something globally from monaco-vscode-api, so if there is already something in there at load time, throw an error |
That is also a good idea |
I see this as well there. Just tried again with fresh browser and cleared cache. |
@kaisalmen i am trying to try this out in our yarn based project.
|
hmmmm the next4 actually solves it ;) |
@CGNonofr npm dependency resolution with 13.1.0 is a lot quicker again. |
@cdietrich btw, we now have those new verification examples for quick checks (pipeline https://github.com/TypeFox/monaco-languageclient/actions/workflows/peer.yml): |
hmmm
will destorys our multi project setup. |
@cdietrich |
@cdietrich you have to adjust your code, see #836 (comment) Maybe you should wait a bit and use a stable build. The CHANGELOGs are also not yet complete. |
so you assume it should work once you release? it complains about the code that really uses vscode, not in the modules that use monco-editor-wrapper |
@cdietrich have you applied the changes mentioned here: CodinGame/monaco-vscode-api#561 (comment) You early feedback is of course appreciated. I haven't written that down yet ⬆️ and I have to make up my mind if we have to perform a major version upgrade, but the api on this level is not changed, so a x.2.0 release should be fine. |
I am still confused what of that applies to the module that uses Monaco and what to the others |
Whatever you do with the api of monaco-editor itself is not effected. Have you looked at the delta in our examples. The changes are pretty straight-forward and the way the dependencies are used has not really changed. Is your code openely available? |
no code is not available. we have a structure like this
in your pr i e.g dont see any changes to https://github.com/TypeFox/monaco-languageclient/tree/main/packages/examples when i now would update imports on the vscode extension @samir-abis maybe you can explain better what we do maybe it works with the current stuff just accidentially |
@cdietrich There are 86 files changed in this PR and there are many changes in the examples. What are you looking at? |
sry if i was imprecise. i just see changes on stuff that uses monaco. we e.g. have packages/our-language-vscode/src/notebook/our-notebook-kernel.ts with import * as vscode from "vscode" am i supposed to import from some codingame here? |
on our main node_modules folder
on the next version one
|
@cdietrich You always did in the past if you used @CGNonofr this PR is now ready for merge. |
i guess i need help on this from someone else in my team. |
hmmm checked out your branch and i see this so is this one correct: i nguess i need to figure out where the bad one is coming from
so it picks the extension-api one for us |
What does |
in yarn berry it wont find anything. am not sure if there is a command line option to show it |
@cdietrich IMO do yourself a favour and get rid of yarn. Use pnpm if you want a cache or just npm |
Applied changes according to: CodinGame/monaco-vscode-api#561 (comment)