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
Avoid circular dependencies in SemVer #957
Conversation
A circular dependency between the Range and Comparator classes can cause issues for consumers who are trying to bundle extensions with vscode-languageclient as a dependency. See related GitHub issue from [npm/node-semver](https://github.com/npm/node-semver) - [[BUG] the package isn't compatible with Rollup due to require cycle microsoft#381](npm/node-semver#381)
@samrose3 I see the problem but I am not a fan of merging this in since it puts me at the mercy of the semver directory layout. IMO this should be addressed in semver directly. Wouldn't it make more sense to provide a PR there? |
I did look into making the fix in It does look like these are officially supported function paths provided by Related discussion around circular dep issues in |
Hi @dbaeumer! Just following up on this 👋 Given that accessing the For more context, we are using I have tested this change and verified that it only pulls in the small parts of Any other concerns I can help address? I've tried resolving the circular dep issue within |
Just checking in, wanting to poke this along if possible 😄 any reconsideration to using the individual function imports for SemVer rather than the full package? This would avoid the circular dep occurring in SemVer and improve the bundle size for those bundling |
I looked at the semver documentation and they indeed advocate using the functions directly. Hope that keep that stable as well :-). |
Problem
Consumers using bundlers like Rollup or Webpack are hitting a circular dependency error coming from npm/node-semver. This is causing the
SemVer,satisfies
function to always returnfalse
when bundled, resulting in the following error:What this PR is doing
A circular dependency between the Range and Comparator classes can cause issues for consumers who are trying to bundle extensions with
vscode-languageclient
as a dependency.See related GitHub issue from npm/node-semver
Using Rollup to bundle VS Code extension
yalc
Verified the build output of
client/lib/node/main.js
is correctly importing only the needed functions, avoiding the circular dependency: