Skip to content

Dependency issues with v13 / questions #563

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

Closed
kaisalmen opened this issue Jan 27, 2025 · 6 comments · Fixed by #564
Closed

Dependency issues with v13 / questions #563

kaisalmen opened this issue Jan 27, 2025 · 6 comments · Fixed by #564
Labels

Comments

@kaisalmen
Copy link
Collaborator

@codingame/monaco-vscode-extension-api has @types/vscode as peerDependency. This should be devDependency.

Are you aware there is a dependency loop from @codingame/monaco-vscode-api => @codingame/monaco-vscode-files-service-override => @codingame/monaco-vscode-api and other packages?

Is it still necessary to redefine monaco-editor on this level or could you use @codingame/monaco-vscode-editor-api everywhere? I no longer do this on monaco-languageclient level (TypeFox/monaco-languageclient#836).

Would this be possible with vscode as well or is the global package rewrite required?

Btw, we currently cannot prevent

  "dependencies": {
    "@codingame/monaco-vscode-api": "13.0.0",
    "@codingame/monaco-vscode-model-service-override": "13.0.0",
    "@codingame/monaco-vscode-extensions-service-override": "^10"
  }

to install both versions? I always thought that the closest version of @codingame/monaco-vscode-api drives the deduplication. But, it seems the problem comes from vscode@npm:@codingame/[email protected] being the previous dependency to @codingame/monaco-vscode-extensions-service-override. So, we can only fix this from now on (>=13) and the old workaround for monaco-languageclient should be kept in place for past versions.

Does it make sense to define only define @codingame/monaco-vscode-api as peer dependency and everything else either as regular or optional dependency to remove complexity?

@kaisalmen
Copy link
Collaborator Author

@CGNonofr
Copy link
Contributor

@codingame/monaco-vscode-extension-api has @types/vscode as peerDependency. This should be devDependency.

devDependencies have no consequences for published packages, so might as well delete it. Why is it an issue?

Are you aware there is a dependency loop from @codingame/monaco-vscode-api => @codingame/monaco-vscode-files-service-override => @codingame/monaco-vscode-api and other packages?

Yes, is it an issue?

Is it still necessary to redefine monaco-editor on this level or could you use @codingame/monaco-vscode-editor-api everywhere? I no longer do this on monaco-languageclient level (TypeFox/monaco-languageclient#836).

some dependencies, like monaco-graphql or monaco-vim (and probably others) are importing from monaco-editor

Would this be possible with vscode as well or is the global package rewrite required?

Others packages need to be able to import from the main package so we can only have 1 working alias. What would be the point though, as the whole point of this package is to be able to run libraries relying of the vscode api, like vscode-languageclient

Does it make sense to define only define @codingame/monaco-vscode-api as peer dependency and everything else either as regular or optional dependency to remove complexity?

Isn't it already the case? I'm not sure to follow

Or are you referring to the type dependency expressed as optional peerDependencies you want to replace by optionalDependencies?

@kaisalmen
Copy link
Collaborator Author

kaisalmen commented Jan 27, 2025

Why is it an issue?

It is a missing peerDependency when you install @codingame/monaco-vscode-extension-api that needs to contained depending on the package manager

Yes, is it an issue?

Was there a loop already before v12/v13? npm takes ages to install the packages, pnpm and yarn are a lot faster (cache).

some dependencies, like monaco-graphql or monaco-vim (and probably others) are importing from monaco-editor

Ok, makes sense. Doing it differently on monaco-languageclient level is no problem, I think.

whole point of this package is to be able to run libraries relying of the vscode api, like vscode-languageclient

Yes, I know. I was just wondering if there is another way.

Or are you referring to the type dependency expressed as optional peerDependencies you want to replace by optionalDependencies?

Yes, only one peerDependency (@codingame/monaco-vscode-api) and everything else simpler; either via dependency or optionalDependency. Is that better or is the better the way it is?

@CGNonofr
Copy link
Contributor

CGNonofr commented Jan 27, 2025

It is a missing peerDependency when you install @codingame/monaco-vscode-extension-api that needs to contained depending on the package manager

What do you suggest then? the issue is that the package includes the proposed api types. Maybe we should include the vscode types directly also, instead of expecting the user to install @types/vscode, what do you think?

Was there a loop already before v12/v13? npm takes ages to install the packages, pnpm and yarn are a lot faster (cache).

yes. It was a dependency and not a peerDependency though

Is that better or is the better the way it is

I don't know... having a dependency only for the type is really not something common, maybe we can just remove them 🤷

@kaisalmen
Copy link
Collaborator Author

instead of expecting the user to install @types/vscode, what do you think?

yes, maybe that is what is best, because usually a pure @types package is a devDependency

I don't know... having a dependency only for the type is really not something common, maybe we can just remove them 🤷

Less complexity is likely better. If you have @codingame/monaco-vscode-api defined as peerDepenendency that should be sufficient for dependent projects. For all other packages do it in the least complex way that provides what is needed (with respect to less maintenance efforts for you).

I will remove all the peerDependencies in monaco-languageclient and dependent project. I will just go back to regular use dependencies. There is always a package manager which does not enforce what I wanted to express in the package definitions (e.g. you can't install "rivaling" packages with npm, but with pnpm you can. This is frustrating). Therefore I will go back to simple dependencies. I will update the PR.

Copy link

🎉 This issue has been resolved in version 13.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging a pull request may close this issue.

2 participants