feat(versionGroups): mark specific dependencies for removal

[Aghassi]

Description

There are some cases were you may not want certain dependencies introduced into the codebase (for example, say your company uses Typescript, you wouldn't want people trying to introduce Flow). Having a way to have a disallow list would be nice.

Suggested Solution

Have a list in the config of disallowed dependencies to prevent them being added anywhere in the repo.

Help Needed

Mostly want to know if this fits the goal of this project before looking to add this feature, or if it's not the job of this project

[JamieMason]

Hey @Aghassi, this should be possible now with the versionGroups. Similar to pinVersion we could add a property to the group which indicates that the dependencies belonging to that group should be disallowed 👍

[Aghassi]

@JamieMason awesome thanks! Anyway I can help with this?

[JamieMason]

Sure, thanks a lot.

Putting each dependency into a semver/version group happens here, but you won't need to change this
https://github.com/JamieMason/syncpack/blob/dea167556635f015f0feb407207b0d2e9470ed0e/src/lib/get-input/get-instances.ts

The logic for finding out if a given version group is valid or not happens here

syncpack/src/bin-list/list-version-groups.ts

Lines 24 to 32 in dea1675

	const pinnedVersion = versionGroup.pinVersion;
	const hasPinnedVersion = isNonEmptyString(pinnedVersion);
	const versions = instances.map(({ version }) => version);
	const uniques = Array.from(new Set(versions));
	const hasMismatches = versions.some(
	(version, i) =>
	(hasPinnedVersion && version !== pinnedVersion) ||
	(i > 0 && version !== versions[i - 1]),
	);

The removal could probably happen around here:

syncpack/src/bin-fix-mismatches/fix-mismatches.ts

Lines 27 to 29 in dea1675

	if (version !== nextVersion) {
	root[dependencyType][name] = nextVersion;
	}

The harder question is deciding where this should be handled - throwing it in with version mismatches would be convenient
as far as implementing it goes, but it might not be the best idea as far as it being clear to users. I've not thought
about this much yet but as I throw these notes together – it's something that wants thinking through so the API is clear
etc.

The rules surface in a few different places too, like in list, list-mismatches, and fix-mismatches.

Hope this is helpful and makes sense, thanks again David.

EDIT: maybe adding "isAllowed: false" as a new prop returned from /src/bin-list/list-version-groups.ts would be a good approach, then use that value in the other areas

[JamieMason]

I've got a working version in this branch, could you give a try please @Aghassi and let me know if it works in your repo.

[JamieMason]

Released in 7.2.1

[Aghassi]

Yep yep, let me upgrade to v7 and come back to this and fill you in

[Aghassi]

@JamieMason just keeping you updated, I just landed the 7.x upgrade internally. I am at a conference this week, but will be trying the new features next week

[Aghassi]

So I have two hurdles and maybe I'm doing it wrong so feel free to correct me

When I enable resolutions, workspaces, and overrides, I'm not getting a failure when I make a resolution in one package.json that is definitely different from a resolution in another package.json when using list-mismatches.

EDIT: ignore versionGroups, see next comment
In addition, I tried using versionGroups with isBanned and I got same thing. No failure.

versionGroups: [
    {
      isBanned: true,
      dependencies: ['react'],
      packages: ['**'],
    },
  ],

I'm curious if I'm misunderstanding the usage or if there is a good way to debug.

My exact command for testing is npx syncpack list-mismatches --config ./syncpack.config.js and my config has a glob set that filters out a bunch of paths I don't care about so I don't pass --source.

EDIT

My config looks like this

module.exports = {
  dev: true,
  // Regex to filter what dependencies we care about.
  filter,
  indent: '  ',
  peer: true,
  prod: true,
  resolutions: true,
  overrides: true,
  workspace: true,
  semverRange: '',
  sortAz: [
    'contributors',
    'dependencies',
    'devDependencies',
    'keywords',
    'peerDependencies',
    'scripts',
  ],
  sortFirst: ['name', 'description', 'version', 'author'],
  source: paths,
  versionGroups: [
    {
      isBanned: true,
      dependencies: ['react'],
      packages: ['**'],
    },
  ],
}

[Aghassi]

Oh ignore me if a dependency is ignored and also in the is banned the ignored wins out.

The silent success threw me off.

[Aghassi]

The messaging could be cleaner but bans do work :)

x  react remove this dependency

I would just change the error to be something more useful. I think it would be nice to say react is ban from use within the code base and then in the config isBanned can take a reason that we can customize and surface to the user so we can say "go to this url to learn more why"