Skip to content

Commit

Permalink
[cli] Fix minor libdef install (#4224)
Browse files Browse the repository at this point in the history
* add working change

* expose pkgVersionMatch func for testing

* add test cases
  • Loading branch information
Brianzchen committed Jan 5, 2022
1 parent 36c4bdf commit 0523004
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 2 deletions.
50 changes: 50 additions & 0 deletions cli/src/lib/npm/__tests__/npmLibDefs-test.js
Expand Up @@ -10,6 +10,7 @@ import {
findNpmLibDef,
getScopedPackageName,
parseSignedCodeVersion,
pkgVersionMatch,
} from '../npmLibDefs';
import * as cacheRepoUtils from '../../cacheRepoUtils';

Expand Down Expand Up @@ -299,6 +300,55 @@ describe('npmLibDefs', () => {
});
});

describe('pkgVersionMatch', () => {
it('matches minor libdef', () => {
expect(pkgVersionMatch('^8.5.1', '8.5.x')).toBe(true);
expect(pkgVersionMatch('~8.5.1', '8.5.x')).toBe(true);
});

it('matches major libdef', () => {
expect(pkgVersionMatch('^8.5.1', '8.x.x')).toBe(true);
expect(pkgVersionMatch('~8.5.1', '8.x.x')).toBe(true);
});

it('will not match a lower libdef', () => {
expect(pkgVersionMatch('^8.5.1', '8.4.x')).toBe(false);
expect(pkgVersionMatch('~8.5.1', '8.4.x')).toBe(false);
});

it('will not match a lower libdef by major value', () => {
expect(pkgVersionMatch('^8.5.1', '7.x.x')).toBe(false);
expect(pkgVersionMatch('~8.5.1', '7.x.x')).toBe(false);
});

it('will not match a greater libdef by minor value', () => {
expect(pkgVersionMatch('^8.5.1', '8.6.x')).toBe(false);
expect(pkgVersionMatch('~8.5.1', '8.6.x')).toBe(false);
});

it('will not match a greater libdef by major value', () => {
expect(pkgVersionMatch('^8.5.1', '9.x.x')).toBe(false);
expect(pkgVersionMatch('~8.5.1', '9.x.x')).toBe(false);
});

it('matches lowest range when version has >=', () => {
expect(pkgVersionMatch('>=8.5.1', '8.x.x')).toBe(true);
expect(pkgVersionMatch('>=8.5.1', '8.5.x')).toBe(true);
});

it('will not match any greater libdef when version has >=', () => {
expect(pkgVersionMatch('>=8.5.1', '9.x.x')).toBe(false);
});

it('matches explicit libdef with major range libdef', () => {
expect(pkgVersionMatch('8.5.1', '8.x.x')).toBe(true);
});

it('matches explicit libdef with minor range libdef', () => {
expect(pkgVersionMatch('8.5.1', '8.5.x')).toBe(true);
});
});

describe('getInstalledNpmLibDefs', () => {
const FIXTURE_ROOT = path.join(BASE_FIXTURE_ROOT, 'getInstalledNpmLibDefs');

Expand Down
23 changes: 21 additions & 2 deletions cli/src/lib/npm/npmLibDefs.js
Expand Up @@ -254,7 +254,10 @@ function validateVersionNumPart(part: string, partName: string): number {
return num;
}

function pkgVersionMatch(pkgSemverRaw: string, libDefSemverRaw: string) {
export function pkgVersionMatch(
pkgSemverRaw: string,
libDefSemverRaw: string,
): boolean {
// The package version should be treated as a semver implicitly prefixed by
// `^` or `~`. Depending on whether or not the minor value is defined.
// i.e.: "foo_v2.2.x" is the same range as "~2.2.x"
Expand Down Expand Up @@ -313,7 +316,23 @@ function pkgVersionMatch(pkgSemverRaw: string, libDefSemverRaw: string) {
const libDefUpper = getRangeUpperBound(libDefRange);

const pkgBelowLower = semver.gtr(libDefLower, pkgSemver);
const pkgAboveUpper = semver.ltr(libDefUpper, pkgSemver);
// semver.coerce explanation:
// We compare a libdef version against a package version
// to check if it matches the range, if pkgAboveUpper would be true
// that means it doesn't match.
//
// Mismatches occur when for example libdef is defined as is 8.5.x
// which makes it's upper <8.6.0
// pkgSemver is ^8.5.1 who's range will reach a max of <9.0.0
// therefore libDefUpper is less than maximum pkgSemver range.
//
// coerce will transform any semver passed in to an explicit
// version, in this case, ^8.5.1 becomes 8.5.1 which allows
// 8.6.0 (libdef) to be above 8.5.1 (pkg).
const pkgAboveUpper = semver.ltr(
libDefUpper,
semver.coerce(pkgSemver)?.version ?? pkgSemver,
);
if (pkgBelowLower || pkgAboveUpper) {
return false;
}
Expand Down

0 comments on commit 0523004

Please sign in to comment.