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

Move monaco to peer dependencies #429

Merged
merged 2 commits into from
Oct 26, 2021

Conversation

HKalbasi
Copy link
Contributor

Fix #351

Since monaco-editor-webpack-plugin has monaco-editor as a peer dependency, people will install monaco-editor in addition to react-monaco-editor and it will bloat the bundle with two different monaco-editor (happened in this PR) and with possible other problems due to version incompatibility. With peer dependency it forces users to install a monaco-editor equal to version needed by this library, and install a compatible monaco-editor-webpack-plugin.

@domoritz
Copy link
Member

Can you explain why moving to a peer dependency actually solves the issue? My understanding is that e.g. yarn resolve/de-duplicate dependencies already. Is this worth requiring every user of this library to explicitly add monaco-editor as a dependency?

@HKalbasi
Copy link
Contributor Author

My understanding is that e.g. yarn resolve/de-duplicate dependencies already.

De-duplicating dependencies in semver incompatible cases (monaco 0.27.0 and monaco 0.29.0 in this case) is wrong and in my experience in that PR yarn doesn't do that.

Is this worth requiring every user of this library to explicitly add monaco-editor as a dependency?

Users of monaco-webpack-plugin (the majority of users) should already do this, because monaco-editor is a peer dependency of it (and they will get warning if they don't add it directly). In current state, these might happen:

  • They don't add monaco-editor directly and install a correct version of monaco-webpack-plugin by chance, there will be a yarn warning but everything will be ok.
  • They don't add monaco-editor directly and install an incompatible version of monaco-webpack-plugin, there will be a yarn warning and potential hard to find problems.
  • They add monaco-editor directly and install corresponding version of monaco-webpack-plugin based on peer dependency, there will be duplicate version of monaco-editor in bundle (unless they are semver equal, by chance) and webpack plugin can be incompatible as well.

With peer dependency, these constraints will be enforced by package manager. So it will be a benefit to monaco-webpack-plugin users. For others, it depends, but it may have no benefit for them and so unnecessary annoyance for them.

@Resetand
Copy link

Resetand commented Oct 26, 2021

react-monaco-editor should use monaco-editor as a peer-dependency, it is a common pattern for such wrappers, to avoid duplication.
In my case i am using this libraries

  • monaco-editor
  • monaco-yaml
  • react-monaco-editor

I rely on some features from monaco core library, so i should install monaco explicitly. and keep track version match between libraries dependencies, to avoid duplicates.

@domoritz domoritz merged commit f863931 into react-monaco-editor:master Oct 26, 2021
@Resetand
Copy link

When can I expect this version to be published?

@domoritz
Copy link
Member

Try react-monaco-editor@0.46.0

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.

Remove star dependency on monaco-editor?
3 participants