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

Allow multiple versions of a particular dependency #41

Closed
ecraig12345 opened this issue Jul 29, 2020 · 10 comments
Closed

Allow multiple versions of a particular dependency #41

ecraig12345 opened this issue Jul 29, 2020 · 10 comments

Comments

@ecraig12345
Copy link

Description

This request pertains to the list-mismatches and fix-mismatches commands.

In some cases, it's necessary to temporarily have different versions of a particular dependency present in the same repo, for example when experimenting with new things or moving code into a monorepo.

Example:

  • foo is a package within the monorepo
  • Several packages in the monorepo (including foo) depend on typescript and react
  • For most packages, their typescript and react versions must match, but foo is experimental and needs newer versions (it may also need newer versions of other packages)
  • Variant: foo could have a few related packages (like foo-utilities) which should share these different dependency versions

Existing solutions

  • Ignore all dependencies of foo by using a custom --source glob which excludes it: e.g. --source "{apps/*,packages/!(foo),packages/sub-group/*,scripts,.}/package.json" (from the real monorepo where I want this feature)
    • Disadvantages: allows accidental variation of other deps
  • Ignore all dependencies on typescript with --filter "^(?!^typescript$).*$"
    • Disadvantages: removes all restrictions on typescript

These options also share the following disadvantages:

  • Not terribly intuitive, at least in the exclusion/negation case
  • Awkward to type on the command line and/or must be duplicated across multiple package.json scripts
  • Doesn't scale well if there are related packages which share alternate versions

Suggested Solutions

There are multiple possible ways to solve this, most simply by adding another command line option, but a package.json and/or config file option would be easier to use and more capable.

1. Command line option

Simplest implementation would be to add a command line option listing packages for which dependencies don't need to match, something like --ignore <package-name-regex>.

This is not my preference (at least as the only solution) because:

  • It's too permissive:
    • Allows any deps to vary, not just the (probably) one or two that you care about
    • If ignoring multiple package names, there's no enforcement that the deps match within them, which is likely what would be desired
  • Typing potentially-complicated regexes on the command line (or duplicating them across multiple scripts in package.json) is annoying.

2. package.json (and/or config file) options

A better approach would be to add support for reading configuration from the monorepo root package.json (and/or a config file, but only using package.json would be easier). This allows easily specifying alternates xin the form of key/value mappings, which I think is the most logical form but would be very awkward to specify on the command line.

There are various ways the option could be set up. Alternatives can be found in the collapsed section below, but here's what I'd suggest (though I'm not at all set on the names).

Using the same example from above, this would allow foo and foo-utilities to depend on any version of typescript and react, but all other packages must still depend on a matching version. variationGroups takes an array in case there are multiple groups of packages which should share alternate versions.

{
  "syncpack": {
    "variationGroups": [
      {
        "packages": ["foo", "foo-utilities"],
        "dependencies": ["typescript", "react"]
      }
    ]
  }
}

Other solutions considered

Click to expand

These were other options I considered but discarded for various reasons. (Again in all these cases, naming is not final.)

2a. List groups of monorepo packages to consider separately

Essentially, do two sub-runs: against foo plus foo-utilities, and against everything else.

{
  "syncpack": {
    "variationGroups": [
      ["foo", "foo-utilities"]
    ]
  }
}

Pros: Probably the simplest to set up and to implement. (Could potentially even be set up as a command line option.)

Cons: It's likely that the variation group and the main group should still share versions of some dependencies, and this does nothing to enforce that.

2b. Mapping between monorepo package and deps which may vary

foo and foo-utilities may depend on any version of typescript and react, but all other packages must still depend on a matching version.

{
  "syncpack": {
    // Mapping from monorepo package to dep list
    "allowedVariations": {
      "foo": ["typescript", "react"],
      "foo-utilities": ["typescript", "react"]
    },
    // OR the inverse:
    // Mapping from dep to monorepo package list
    "allowedVariations": {
      "typescript": ["foo", "foo-utilities"],
      "react": ["foo", "foo-utilities"]
    }
  }
}

Pros: Simple to set up in some ways.

Cons: Requires duplication if there are additional packages which are related to foo; no notion of groups which should share dep versions

2c. Mapping from monorepo package to allowed semver specs

For particular packages, list the semver specs which are allowed. This is similar to allowedAlternateVersions from rush (which my team's monorepo previously used).

{
  "syncpack": {
    "allowedVersions": {
      "typescript": ["3.7.2", "^3.9.0"]
    }
  }
}

Pros: No duplication required for multiple packages to share an alternate version; very specific

Cons: Additional place to manually change when upgrading versions; no notion of grouping; seems a bit incongruous with syncpack's other options (which don't rely on explicitly set semver specs)

Note for any package.json/config file-based solution

If any file-based config support is added for version alternates, it would probably make sense to also add support for overrideable defaults for the other options. For example, instead of having to specify --prod --dev --source "{apps/*,packages/!(foo),packages/sub-group/*,scripts,.}/package.json" for every syncpack command, you could do this:

{
  "syncpack": {
    "prod": true,
    "dev": true,
    "source": "{apps/*,packages/!(foo),packages/sub-group/*,scripts,.}/package.json"
  }
}

(Granted the long source glob would not be needed once this new config setting is added, but it would still be nice to have the other common options specified in a single place.)

Help Needed

I can probably help implement this.

@JamieMason
Copy link
Owner

Hi @ecraig12345,
I appreciate the time and effort you've put into this, it's refreshing to receive an issue with such detail and thought put into it, so thank you for that.

I know you've spoken about this but I just want to confirm – for the versions of typescript and react within foo and foo-utilities, is the intention to:

  • (A) continue checking the versions of typescript and react within foo and foo-utilities for mismatches, but only ensure that they match within foo and foo-utilities, regardless of what they might be in the rest of the repo.
  • (B) exclude the instances of typescript and react within foo and foo-utilities from mismatch checking altogether – they could mismatch the rest of the repo and/or mismatch each other and still be considered valid.

If A, your preferred shape looks a good fit as it allows for multiple partitions/contexts/scopes and, within them, suggests there's a relationship between the members of packages.

{
  "syncpack": {
    "mismatchContexts": [
      {
        "packages": ["foo", "foo-utilities"],
        "dependencies": ["typescript", "react"]
      }
    ]
  }
}

If B, I'd prefer to use a structure which couldn't be misinterpreted as describing a relationship between the packages, even if that means a little repetition. Roughly something like:

{
  "syncpack": {
    "allowedMismatches": {
      "foo": ["typescript", "react"],
      "foo-utilities": ["typescript", "react"]
    }
  }
}

Let me know what you think. Like you, I'm not set on the names mismatchContexts, allowedMismatches, or the shapes we've come up with so far – feel free to challenge anything at all I've said and let's see what we can come up with.

Thanks again for your time, appreciate it.

Jamie

@ecraig12345
Copy link
Author

Thanks for the response! Option A is what I meant.

@ecraig12345
Copy link
Author

I'm not sure about the name mismatchContexts since it could be misinterpreted to mean that mismatches are allowed among versions of dependencies within packages. Maybe alternateContexts or alternateGroups?

@JamieMason
Copy link
Owner

JamieMason commented Aug 6, 2020

I'm not sure about the name mismatchContexts since it could be misinterpreted to mean that mismatches are allowed among versions of dependencies within packages. Maybe alternateContexts or alternateGroups?

yeah good point, I think you're right. I wanted "mismatch" to be in there somewhere to indicate that this config only applies to that feature, but I think you're right that it could be confusing. alternateGroups, alternateVersionGroups, or versionGroups could work possibly, it's a tricky one. We can think about that in the background.

I am away this weekend so this may take a couple of weeks before I have the time.

Taking a quick look at the code here...

Mismatched dependencies are collected here (they use generators to allow support for future interactive modes):

export function* getMismatchedDependencies(
types: DependencyType[],
wrappers: SourceWrapper[],
): Generator<InstalledPackage> {
const iterator = getDependencies(types, wrappers);
for (const installedPackage of iterator) {
const { installations } = installedPackage;
const len = installations.length;
if (len > 1) {
for (let i = 1; i < len; i++) {
if (installations[i].version !== installations[i - 1].version) {
yield installedPackage;
break;
}
}
}
}
}

This is read by both list-mismatches:

const iterator = getMismatchedDependencies(dependencyTypes, wrappers);
const mismatches = Array.from(iterator).filter(({ name }) => name.search(filter) !== -1);

and fix-mismatches:

const iterator = getMismatchedDependencies(dependencyTypes, wrappers);
const mismatches = Array.from(iterator).filter(({ name }) => name.search(filter) !== -1);

I'm not sure as I'd need more time to look at it properly, but it might be possible to pass more arguments into getMismatchedDependencies and have it all handled in there.

@JamieMason
Copy link
Owner

FYI I'm planning to split this up so we add support for a syncpack config first (via cosmiconfig), then return to this issue to extend that with what we want here.

Branch: feat/config-file

@JamieMason
Copy link
Owner

JamieMason commented Aug 23, 2020

The config file work is released as of 5.2.5, which unblocks this issue.

@ecraig12345
Copy link
Author

Great, thanks so much for all your work on this!

@JamieMason
Copy link
Owner

Released in 5.6.7. I've tested for as much as I can think of, but please let me know if you run into any problems.

@ecraig12345
Copy link
Author

Great, thank you!!! Haven't gotten to try it out yet but I'll let you know if there are issues.

@JasonGore
Copy link

Yes, thanks for the quick turnaround! I gave it a try and think I may have found an issue.

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

No branches or pull requests

3 participants