Address PR comments

This commit is contained in:
James McMeeking 2026-04-02 13:03:01 +01:00
parent 5690e55d99
commit 6f976f6a2b
No known key found for this signature in database
GPG key ID: C69A11061EE15228
3 changed files with 62 additions and 30 deletions

View file

@ -22,23 +22,29 @@ async function scanRushAddCommand(args) {
return []; return [];
} }
const packageSpecs = extractRushAddPackageSpecs(args); const parsedSpecs = extractRushAddPackageSpecs(args)
.map((spec) => parsePackageSpec(spec))
.filter((spec) => spec !== null);
const resolvedVersions = await Promise.all(
parsedSpecs.map(async (parsed) => {
const exactVersion = await resolvePackageVersion(parsed.name, parsed.version);
return {
parsed,
exactVersion,
};
}),
);
const changes = []; const changes = [];
for (const resolved of resolvedVersions) {
for (const spec of packageSpecs) { if (!resolved.exactVersion) {
const parsed = parsePackageSpec(spec);
if (!parsed) {
continue;
}
const exactVersion = await resolvePackageVersion(parsed.name, parsed.version);
if (!exactVersion) {
continue; continue;
} }
changes.push({ changes.push({
name: parsed.name, name: resolved.parsed.name,
version: exactVersion, version: resolved.exactVersion,
type: "add", type: "add",
}); });
} }

View file

@ -8,8 +8,9 @@ import { reportCommandExecutionFailure } from "../_shared/commandErrors.js";
*/ */
export async function runRushCommand(args) { export async function runRushCommand(args) {
try { try {
const env = mergeSafeChainProxyEnvironmentVariables(process.env); const env = normalizeProxyEnvironmentVariables(
normalizeProxyEnvironmentVariables(env); mergeSafeChainProxyEnvironmentVariables(process.env),
);
const result = await safeSpawn("rush", args, { const result = await safeSpawn("rush", args, {
stdio: "inherit", stdio: "inherit",
@ -27,37 +28,44 @@ export async function runRushCommand(args) {
* lowercase or npm/yarn-specific environment variables. * lowercase or npm/yarn-specific environment variables.
* *
* @param {Record<string, string>} env * @param {Record<string, string>} env
* @returns {Record<string, string>}
*/ */
function normalizeProxyEnvironmentVariables(env) { function normalizeProxyEnvironmentVariables(env) {
if (env.HTTPS_PROXY && !env.HTTP_PROXY) { const normalized = {
env.HTTP_PROXY = env.HTTPS_PROXY; ...env,
};
if (normalized.HTTPS_PROXY && !normalized.HTTP_PROXY) {
normalized.HTTP_PROXY = normalized.HTTPS_PROXY;
} }
if (env.HTTP_PROXY && !env.http_proxy) { if (normalized.HTTP_PROXY && !normalized.http_proxy) {
env.http_proxy = env.HTTP_PROXY; normalized.http_proxy = normalized.HTTP_PROXY;
} }
if (env.HTTPS_PROXY && !env.https_proxy) { if (normalized.HTTPS_PROXY && !normalized.https_proxy) {
env.https_proxy = env.HTTPS_PROXY; normalized.https_proxy = normalized.HTTPS_PROXY;
} }
if (env.HTTP_PROXY && !env.npm_config_proxy) { if (normalized.HTTP_PROXY && !normalized.npm_config_proxy) {
env.npm_config_proxy = env.HTTP_PROXY; normalized.npm_config_proxy = normalized.HTTP_PROXY;
} }
if (env.HTTPS_PROXY && !env.npm_config_https_proxy) { if (normalized.HTTPS_PROXY && !normalized.npm_config_https_proxy) {
env.npm_config_https_proxy = env.HTTPS_PROXY; normalized.npm_config_https_proxy = normalized.HTTPS_PROXY;
} }
if (env.HTTP_PROXY && !env.NPM_CONFIG_PROXY) { if (normalized.HTTP_PROXY && !normalized.NPM_CONFIG_PROXY) {
env.NPM_CONFIG_PROXY = env.HTTP_PROXY; normalized.NPM_CONFIG_PROXY = normalized.HTTP_PROXY;
} }
if (env.HTTPS_PROXY && !env.NPM_CONFIG_HTTPS_PROXY) { if (normalized.HTTPS_PROXY && !normalized.NPM_CONFIG_HTTPS_PROXY) {
env.NPM_CONFIG_HTTPS_PROXY = env.HTTPS_PROXY; normalized.NPM_CONFIG_HTTPS_PROXY = normalized.HTTPS_PROXY;
} }
if (env.HTTPS_PROXY && !env.YARN_HTTPS_PROXY) { if (normalized.HTTPS_PROXY && !normalized.YARN_HTTPS_PROXY) {
env.YARN_HTTPS_PROXY = env.HTTPS_PROXY; normalized.YARN_HTTPS_PROXY = normalized.HTTPS_PROXY;
} }
return normalized;
} }

View file

@ -5,11 +5,13 @@ describe("runRushCommand", () => {
let runRushCommand; let runRushCommand;
let safeSpawnMock; let safeSpawnMock;
let mergeCalls; let mergeCalls;
let mergeResultEnv;
let nextSpawnStatus; let nextSpawnStatus;
let nextSpawnError; let nextSpawnError;
beforeEach(async () => { beforeEach(async () => {
mergeCalls = []; mergeCalls = [];
mergeResultEnv = null;
nextSpawnStatus = 0; nextSpawnStatus = 0;
nextSpawnError = null; nextSpawnError = null;
safeSpawnMock = mock.fn(async () => { safeSpawnMock = mock.fn(async () => {
@ -32,6 +34,10 @@ describe("runRushCommand", () => {
namedExports: { namedExports: {
mergeSafeChainProxyEnvironmentVariables: (env) => { mergeSafeChainProxyEnvironmentVariables: (env) => {
mergeCalls.push(env); mergeCalls.push(env);
if (mergeResultEnv) {
return mergeResultEnv;
}
return { return {
...env, ...env,
HTTPS_PROXY: "http://localhost:8080", HTTPS_PROXY: "http://localhost:8080",
@ -96,4 +102,16 @@ describe("runRushCommand", () => {
assert.strictEqual(res.status, 1); assert.strictEqual(res.status, 1);
}); });
it("does not mutate merged env object", async () => {
mergeResultEnv = {
HTTPS_PROXY: "http://localhost:8080",
};
await runRushCommand(["install"]);
assert.deepStrictEqual(mergeResultEnv, {
HTTPS_PROXY: "http://localhost:8080",
});
});
}); });