Skip to content

Commit

Permalink
fix: [DEVXBUGS-11661] error is shown if disttags are used in package.…
Browse files Browse the repository at this point in the history
…json (#337)
  • Loading branch information
alex-gilin authored Sep 1, 2024
1 parent 5acbaa3 commit 16c974d
Show file tree
Hide file tree
Showing 8 changed files with 175 additions and 63 deletions.
4 changes: 2 additions & 2 deletions packages/npm-dependencies-validation/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@
"dependencies": {
"fs-extra": "10.0.0",
"lodash": "4.17.21",
"semver": "7.3.5"
"semver": "^7.6.3"
},
"devDependencies": {
"@types/fs-extra": "^9.0.13",
"@types/semver": "^7.3.9",
"@types/semver": "^7.5.8",
"type-fest": "^2.11.1"
}
}
17 changes: 15 additions & 2 deletions packages/npm-dependencies-validation/src/depIssuesFinder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
PackageJsonDeps,
SemVerRange,
} from "./types";
import { retrieveDistTags } from "./utils/npmUtil";

const NO_ISSUES = undefined;
const PKG_JSON_DEFAULT_DEPS: PackageJsonDeps = {
Expand Down Expand Up @@ -136,11 +137,23 @@ async function validateMismatchDep(opts: {
return NO_ISSUES;
}

if (!satisfies(actualVersion, opts.expectedVerRange)) {
let expectedVerRange = opts.expectedVerRange;
let isIssie = false;
if (!satisfies(actualVersion, expectedVerRange)) {
// assume it's a disttag, attempt to get the referenced version
expectedVerRange = await retrieveDistTags(opts);
if (
expectedVerRange === opts.expectedVerRange ||
!satisfies(actualVersion, expectedVerRange)
) {
isIssie = true;
}
}
if (isIssie) {
return {
type: "mismatch" as "mismatch",
name: opts.depName,
expected: opts.expectedVerRange,
expected: expectedVerRange,
actual: actualVersion,
isDev: opts.isDev,
};
Expand Down
59 changes: 58 additions & 1 deletion packages/npm-dependencies-validation/src/utils/npmUtil.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { ChildProcessWithoutNullStreams, spawn } from "child_process";
import { NpmCommandConfig, OutputChannel } from "../types";
import { NpmCommandConfig, OutputChannel, SemVerRange } from "../types";
import { print } from "../logger";
import { EOL } from "os";
import { dirname } from "path";
import { noop } from "lodash";

export function getNPM(): string {
return /^win/.test(process.platform) ? "npm.cmd" : "npm";
Expand Down Expand Up @@ -43,3 +46,57 @@ function executeSpawn(
const { commandArgs, cwd } = config;
return spawn(getNPM(), [...commandArgs, ...additionalArgs], { cwd });
}

function runNpmCommand(args: string[], cwd: string): Promise<string> {
let output = "";
const outputChannel: Partial<OutputChannel> = {
appendLine: (data: string) => (output += data),
show: noop,
};
return invokeNPMCommand(
{ commandArgs: args, cwd },
outputChannel as OutputChannel
)
.catch(() => output)
.then(() => output);
}

// this is in memory cache for repo's disttags metadata that used to resolve the expected version range
const distTagsMap = new Map<string, Map<string, string>>();

export async function retrieveDistTags(opts: {
depPkgJsonPath: string;
depName: string;
expectedVerRange: SemVerRange;
}): Promise<string | SemVerRange> {
const { depName, expectedVerRange } = opts;
if (!distTagsMap.has(depName)) {
const output = await runNpmCommand(
["dist-tags", "ls", depName],
dirname(opts.depPkgJsonPath)
);
const distTags = new Map<string, string>();
distTagsMap.set(depName, distTags);
output
.split(EOL)
.map((line) => line.trim())
.filter((line) => {
return line.includes(":") && !/npm\s+[warn|err]/i.test(line);
})
.forEach((line) => {
const [tag, version] = line.split(":");
distTags.set(tag, version.trim());
});
}
return distTagsMap.get(depName)!.get(expectedVerRange) ?? expectedVerRange;
}

// TODO: consider to uncomment it to clear distTagsMap if it gets too big
// // using a large interval to reduce memory usage
// /* istanbul ignore next -- TCO of testing this setInterval is too high*/
// setInterval(() => {
// distTagsMap.clear();
// // without .unref(), the interval would keep the Node.js process alive indefinitely.
// // By calling .unref(), you allow the process to exit naturally if no other events are pending,
// // even if the interval is still set to run periodically.
// }, 15 * 60 * 1000).unref();

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
"name": "mismatch-dep",
"version": "1.0.0",
"dependencies": {
"mismatched": "3.3.3"
"mismatched": "3.3.3",
"webpack": "legacy"
},
"devDependencies": {
"range-parser": "~1.2.1"
Expand Down
38 changes: 37 additions & 1 deletion packages/npm-dependencies-validation/test/utils/npmUtil.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { resolve } from "path";
import { noop } from "lodash";
import { createSandbox, SinonSpy } from "sinon";
import { OutputChannel } from "../../src/types";
import { getNPM, invokeNPMCommand } from "../../src/utils/npmUtil";
import {
getNPM,
invokeNPMCommand,
retrieveDistTags,
} from "../../src/utils/npmUtil";
import { npmSpawnTestTimeout } from "../config";

describe("npmUtil unit test", () => {
Expand Down Expand Up @@ -67,4 +71,36 @@ describe("npmUtil unit test", () => {
expect(getNPM()).to.be.equal("npm");
});
});

describe("retrieveDistTags", () => {
it("not cached, disttags found", async () => {
expect(
await retrieveDistTags({
depPkgJsonPath: __dirname,
depName: "webpack",
expectedVerRange: "legacy",
})
).be.equal("1.15.0");
});

it("cached, disttags not found", async () => {
expect(
await retrieveDistTags({
depPkgJsonPath: __dirname,
depName: "webpack",
expectedVerRange: "webpack-1",
})
).be.equal("webpack-1");
});

it("not existed, disttags not found", async () => {
expect(
await retrieveDistTags({
depPkgJsonPath: __dirname,
depName: "not-existing",
expectedVerRange: "next",
})
).be.equal("next");
});
});
});
4 changes: 2 additions & 2 deletions packages/vscode-dependencies-validation/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@
},
"dependencies": {
"@sap-devx/npm-dependencies-validation": "^1.8.3",
"@vscode-logging/wrapper": "1.0.1",
"@vscode-logging/wrapper": "2.0.0",
"fs-extra": "10.0.0",
"jsonc-parser": "3.0.0",
"lodash": "^4.17.21",
Expand All @@ -81,7 +81,7 @@
"devDependencies": {
"@types/fs-extra": "^9.0.13",
"@types/proxyquire": "^1.3.28",
"@vscode-logging/types": "0.1.4",
"@vscode-logging/types": "2.0.0",
"jest-mock-vscode": "^0.1.3",
"proxyquire": "2.1.3",
"vscode-uri": "^3.0.3"
Expand Down
Loading

0 comments on commit 16c974d

Please sign in to comment.