Make local-dev-lib a peerdependancy#34
Merged
Merged
Conversation
brandenrodgers
approved these changes
Nov 7, 2023
Contributor
brandenrodgers
left a comment
There was a problem hiding this comment.
Thanks for the thorough explanation 🙏
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Previously, both
hubspot-cliandcli-libused thecli-lib/configmodule to interact with thehubspot.config.ymlfile. Theconfigmodule stores information about the config file in a global variable called__config. We recently introducedhubspot-local-dev-libas a dependency to the CLI. Many modules withinlocal-dev-libalso need to interact with config files, and aslocal-dev-libis itself a port ofcli-lib, it also provides its own config module with its own instance of__config. Having two separate instances of__configcauses bugs, so in an attempt to remedy this, we addedlocal-dev-libas a dep tocli-liband deferred to its instance of__config. Unfortunately things were not so simple.hubspot-cli's dependancies onhubspot-local-dev-liblook something like this:node_modules/@hubspot/local-dev-libnode_modules/@hubspot/clli-lib/node_modules/local-dev-libThis works, but only if both
hubspot-cliandcli-libuse the same version of@hubspot-local-dev-lib. There are many ways for versions between the two to get out of whack, resulting in two separate__configinstances and causing bugs. Our solution is to switchlocal-dev-libto apeerDependencywithin the CLI. Peer dependencies are a feature of npm that allow you to stipulate that package A requires package B, but will not install the dependency on its own. Instead, any package C that has a dependency on package A must also have a dependency on package B. By using them, we can assure that package A (cli-lib) uses the same package B (local-dev-lib) as package C (hubspot-cli).The only side effect is that now, any package with a dependency on
cli-libmust also explicitly specify a dependency onlocal-dev-libin itspackage.json. We will have to do a major release to make this changeSome more info on
peerDependencies, since we didn't know much about them prior to running into this problemWho to Notify
@brandenrodgers