support dual versions #2
Conversation
| errors = externalMismatch.validate(ws.get("pkg-1")!, ws, rootWorkspace, {}); | ||
| expect(errors.length).toEqual(0); | ||
| }); | ||
|
|
There was a problem hiding this comment.
We should add a test case to validate which version is used when adding an unsupported version in the case where 2 or more allowed versions exist.
e.g.
'allowedDependencyVersions' {
'foo': [
'^1.0.0',
'^2.0.0'
]
}
yarn add foo@^3.0.0 ... does it clamp to the lowest version number, the highest one, or the most commonly used between the allowed versions?
There was a problem hiding this comment.
yep, added a test case, should be the most commonly used one.
| @@ -1,4 +1,4 @@ | |||
| import internalMismatch from "../EXTERNAL_MISMATCH"; | |||
There was a problem hiding this comment.
Interesting to see the naming discrepancy. Increases the diff, which might be problematic if we did end up sticking to a fork if the contribution to the main lib isn't accepted.
| expect(errors.length).toEqual(0); | ||
| }); | ||
|
|
||
| it("should error if the range is the range is outside allowedDependencyVersions and running fix should clamp it to the most commonly used one", () => { |
There was a problem hiding this comment.
grammar nit: "if the range is the range is outside" looks like duplication. Rephrase please.
packages/cli/src/checks/utils.ts
Outdated
| @@ -120,7 +124,7 @@ function weakMemoize<Arg, Ret>(func: (arg: Arg) => Ret): (arg: Arg) => Ret { | |||
| export let getMostCommonRangeMap = weakMemoize(function getMostCommonRanges( | |||
| allPackages: Map<string, Package> | |||
| ) { | |||
| let dependencyRangesMapping = new Map<string, {[key: string]: number}>(); | |||
| let dependencyRangesMapping = new Map<string, { [key: string]: number }>(); | |||
There was a problem hiding this comment.
We may as well avoid formatting changes that aren't related to the tangible changes of this PR just to reduce diff.
There was a problem hiding this comment.
yeah, I cleared all those unrelated changes to reduce diff
| @@ -15,7 +15,7 @@ type ErrorType = { | |||
| }; | |||
|
|
|||
| export default makeCheck<ErrorType>({ | |||
| validate: (workspace, allWorkspace) => { | |||
| validate: (workspace, allWorkspace, rootWorkspace, options) => { | |||
There was a problem hiding this comment.
What does rootWorkspace do here? I don't see it referenced further down?
There was a problem hiding this comment.
That's the validate function signature.
let errors = check.validate( workspace, allWorkspaces, rootWorkspace, options );
| if ( | ||
| mostCommonRange !== undefined && | ||
| mostCommonRange !== range && | ||
| !(allowedVersions && allowedVersions.includes(range)) && |
There was a problem hiding this comment.
Just to summarise, it'll error anything that isn't most common or in allowedVersions? When fixing it'll fix to most common?
cheesehary
force-pushed
the
support-dual-versions
branch
from
0f1ba60 to
df2ca33
Compare
|
Dont forget to open a PR back upstream |
|
Late to the party, but it seems like we may need to evolve this a little further first. There seems to be unexpected changes to the |
Support dual package versions. This can be configured in root package.json