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

Disallow autofix for banned specifier types. #216

Open
RachelScodes opened this issue May 16, 2024 · 2 comments
Open

Disallow autofix for banned specifier types. #216

RachelScodes opened this issue May 16, 2024 · 2 comments

Comments

@RachelScodes
Copy link

RachelScodes commented May 16, 2024

Description

I think it's a common case that repos want to enforce a convention of explicit versioning. So if we have a rule that is banning a given specifier type, I think that needs manual attention or to be fixed via prompt. I don't think it can or should be auto-fixed

Suggested Solution 1

If a rule has both isBanned: true AND a specifier type: it should be prompt fix only. Nice to have is outputting the values that matched the banned specifier type

Sample code in the config:

// syncpackrc.js
const config = {
  versionGroups: [{
      label: 'Explicit versions or version ranges must be used for all dependencies. "latest" or wildcard ("*") are not allowed.',
      packages: ["**"],
      dependencies: ['**'],
      dependencyTypes: ['**'],
      specifierTypes: ['latest'],
      isBanned: true,
  }]
}

current output:

= Explicit versions or version ranges must be used for all dependencies. "latest" or wildcard ("*") are not allowed. 
✘ type-fest banned package.json > dependencies [Banned]
✘ typescript banned package.json > overrides [Banned]
     2 ✓ can be auto-fixed

proposed output with remediation instructions and why it didn't pass linting:

= Explicit versions or version ranges must be used for all dependencies. "latest" or wildcard ("*") are not allowed. 
✘ type-fest banned package.json > dependencies [Banned]
  Banned specifier type ('latest' || '*') <- why it is banned, and what the bad values are
✘ typescript banned package.json > overrides [Banned]
  Banned specifier type ('latest' || '*') <- why it is banned, and what the bad values are
     2 ! can be fixed manually using syncpack prompt

Suggested Solution 2

add an explicit config rule key: enforceExplicitVersions. the value is an array of dependency types: ['prod', 'dev', '!local', '**', etc] or an object with versionGroup keys:

Sample code in the config:

// syncpackrc.js
const config = {
  enforceExplicitVersions: ['prod', 'dev', '!local', '**', etc],
  // OR
  explicitVersionsOnly: {
      label: 'Explicit versions or version ranges must be used for all dependencies. "latest" or wildcard ("*") are not allowed.',
      packages: ["**"],
      dependencies: ['**'],
      dependencyTypes: ['**']
      // does not need specifierTypes or isBanned because this is scoped to banning the latest specifier type anyway
  },
}

proposed output:

Versions
= Explicit versions only ============================ (if no "label" provided or using the array of dependencyTypes)
= Explicit versions or version ranges must be used for all dependencies. "latest" or wildcard ("*") are not allowed. (used value from label)
✘ type-fest banned package.json > dependencies [Banned]
  Implicit version used ('latest'). Must select an explicit version or range
✘ typescript banned package.json > overrides [Banned]
  Implicit version used ('*'). Must select an explicit version or range
     2 ! can be fixed manually using syncpack prompt
@JamieMason
Copy link
Owner

Thanks again @RachelScodes it's a good idea. I've been thinking on and off about whether to expose config for each status code to choose whether to error, warn, or stay silent. What you're describing could be another flavour/use case of that. Thanks for a great issue, will use this when thinking about ways to handle this scenario and others like it.

(I see you're at Netflix by the way, are you using syncpack there? I'd been curious as I see referrals from an internal Netflix URL in this repo's Traffic Insights).

@andschub-ms
Copy link

andschub-ms commented Jul 16, 2024

@JamieMason Example 4 in your docs seems to suggest this config (range: "") would enforce fixed version, although I copied the example and tried it in practice and it allows "*" version. I had to follow @RachelScodes 's example 1 which is properly blocking the ambiguous version range (*). I would suggest double-checking this example to see if this situation repros. https://jamiemason.github.io/syncpack/guide/semver-groups/

It would also be good in the public docs to document what values the "range" key can take. I had to look up in the typescript code, and it's still not clear what is the behavior of an empty string "" value, what it allows or disallows.

export type SemverRange = '' | '*' | '>' | '>=' | '.x' | '<' | '<=' | '^' | '~' | 'workspace:';

Thank you.

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