Skip to content

fix: handle projects with devEngines ranges gracefully #730

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: handle projects with devEngines ranges gracefully #730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 32 additions & 28 deletions sources/specUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,47 +77,51 @@ function warnOrThrow(errorMessage: string, onFail?: DevEngineDependency[`onFail`
console.warn(`! Corepack validation warning: ${errorMessage}`);
}
}
function parsePackageJSON(packageJSONContent: CorepackPackageJSON) {
const {packageManager: pm} = packageJSONContent;
if (packageJSONContent.devEngines?.packageManager != null) {
const {packageManager} = packageJSONContent.devEngines;

if (typeof packageManager !== `object`) {
console.warn(`! Corepack only supports objects as valid value for devEngines.packageManager. The current value (${JSON.stringify(packageManager)}) will be ignored.`);
return pm;
function parsePackageJSON({devEngines, packageManager}: CorepackPackageJSON) {
const spec = {
packageManager,
enforceExactVersion: true,
};

if (devEngines?.packageManager != null) {
const {packageManager: pm} = devEngines;

if (typeof pm !== `object`) {
console.warn(`! Corepack only supports objects as valid value for devEngines.packageManager. The current value (${JSON.stringify(pm)}) will be ignored.`);
return spec;
}
if (Array.isArray(packageManager)) {
if (Array.isArray(pm)) {
console.warn(`! Corepack does not currently support array values for devEngines.packageManager`);
return pm;
return spec;
}

const {name, version, onFail} = packageManager;
const {name, version, onFail} = pm;
if (typeof name !== `string` || name.includes(`@`)) {
warnOrThrow(`The value of devEngines.packageManager.name ${JSON.stringify(name)} is not a supported string value`, onFail);
return pm;
return spec;
}
if (version != null && (typeof version !== `string` || !semverValidRange(version))) {
warnOrThrow(`The value of devEngines.packageManager.version ${JSON.stringify(version)} is not a valid semver range`, onFail);
return pm;
return spec;
}

debugUtils.log(`devEngines.packageManager defines that ${name}@${version} is the local package manager`);

if (pm) {
if (!pm.startsWith?.(`${name}@`))
warnOrThrow(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the "devEngines.packageManager" field set to ${JSON.stringify(name)}`, onFail);

else if (version != null && !semverSatisfies(pm.slice(packageManager.name.length + 1), version))
warnOrThrow(`"packageManager" field is set to ${JSON.stringify(pm)} which does not match the value defined in "devEngines.packageManager" for ${JSON.stringify(name)} of ${JSON.stringify(version)}`, onFail);

return pm;
if (packageManager) {
if (!packageManager.startsWith?.(`${name}@`))
warnOrThrow(`"packageManager" field is set to ${JSON.stringify(packageManager)} which does not match the "devEngines.packageManager" field set to ${JSON.stringify(name)}`, onFail);
else if (version != null && !semverSatisfies(packageManager.slice(pm.name.length + 1), version))
warnOrThrow(`"packageManager" field is set to ${JSON.stringify(packageManager)} which does not match the value defined in "devEngines.packageManager" for ${JSON.stringify(name)} of ${JSON.stringify(version)}`, onFail);
return spec;
}


return `${name}@${version ?? `*`}`;
return {
enforceExactVersion: semverValid(version),
packageManager: `${name}@${version ?? `*`}`,
};
}

return pm;
return spec;
}

export async function setLocalPackageManager(cwd: string, info: PreparedPackageManagerInfo) {
Expand Down Expand Up @@ -233,11 +237,11 @@ export async function loadSpec(initialCwd: string): Promise<LoadSpecResult> {
process.env = selection.localEnv;
}

const rawPmSpec = parsePackageJSON(selection.data);
if (typeof rawPmSpec === `undefined`)
const {enforceExactVersion, packageManager} = parsePackageJSON(selection.data);
if (typeof packageManager === `undefined`)
return {type: `NoSpec`, target: selection.manifestPath};

debugUtils.log(`${selection.manifestPath} defines ${rawPmSpec} as local package manager`);
debugUtils.log(`${selection.manifestPath} defines ${packageManager} as local package manager`);

return {
type: `Found`,
Expand All @@ -249,6 +253,6 @@ export async function loadSpec(initialCwd: string): Promise<LoadSpecResult> {
onFail: selection.data.devEngines.packageManager.onFail,
},
// Lazy-loading it so we do not throw errors on commands that do not need valid spec.
getSpec: () => parseSpec(rawPmSpec, path.relative(initialCwd, selection.manifestPath)),
getSpec: () => parseSpec(packageManager, path.relative(initialCwd, selection.manifestPath), {enforceExactVersion}),
};
}
24 changes: 16 additions & 8 deletions tests/main.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,8 @@ it(`should use hash from "packageManager" even when "devEngines" defines a diffe
});
});

describe(`should accept range in devEngines only if a specific version is provided`, () => {
it(`either in package.json#packageManager field`, async () => {
describe(`should accept range in devEngines only if`, () => {
it(`either is fully defined in package.json#packageManager field`, async () => {
await xfs.mktempPromise(async cwd => {
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
devEngines: {
Expand All @@ -387,18 +387,22 @@ describe(`should accept range in devEngines only if a specific version is provid
version: `6.x`,
},
},
packageManager: `[email protected]+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`,
});
await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({
exitCode: 1,
stderr: `Invalid package manager specification in package.json ([email protected]); expected a semver version\n`,
stdout: ``,
exitCode: 0,
stderr: ``,
stdout: `6.6.2\n`,
});
});
});

it(`has no packageManager.version at all`, async () => {
await xfs.mktempPromise(async cwd => {
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
devEngines: {
packageManager: {
name: `pnpm`,
version: `6.x`,
},
},
packageManager: `[email protected]+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`,
Expand All @@ -408,16 +412,20 @@ describe(`should accept range in devEngines only if a specific version is provid
stderr: ``,
stdout: `6.6.2\n`,
});
});
});

// No version should also work
it(`does not use package.json#packageManager at all`, async () => {
await xfs.mktempPromise(async cwd => {
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
devEngines: {
packageManager: {
name: `pnpm`,
version: `~6.6.0`,
},
},
packageManager: `[email protected]+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`,
});

await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({
exitCode: 0,
stderr: ``,
Expand Down