Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 4 additions & 1 deletion sources/Engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,12 @@ export class Engine {
}

case `NoSpec`: {
if (typeof locator.reference === `function`)
if (result.devEnginesValue)
fallbackDescriptor.range = result.devEnginesValue.range;
else if (typeof locator.reference === `function`)
fallbackDescriptor.range = await locator.reference();


if (process.env.COREPACK_ENABLE_AUTO_PIN === `1`) {
const resolved = await this.resolveDescriptor(fallbackDescriptor, {allowTags: true});
if (resolved === null)
Expand Down
3 changes: 2 additions & 1 deletion sources/commands/Base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ export abstract class BaseCommand extends Command<Context> {
throw new UsageError(`Couldn't find a project in the local directory - please specify the package manager to pack, or run this command from a valid project`);

case `NoSpec`:
if (lookup.devEnginesValue) return [lookup.devEnginesValue];
throw new UsageError(`The local project doesn't feature a 'packageManager' field nor a 'devEngines.packageManager' field - please specify the package manager to pack, or update the manifest to reference it`);

default: {
return [lookup.range ?? lookup.getSpec()];
return [lookup.devEnginesValue ?? lookup.getSpec()];
}
}
} else {
Expand Down
4 changes: 4 additions & 0 deletions sources/commands/deprecated/Prepare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ export class PrepareCommand extends Command<Context> {
throw new UsageError(`Couldn't find a project in the local directory - please specify the package manager to pack, or run this command from a valid project`);

case `NoSpec`:
if (lookup.devEnginesValue) {
specs.push(lookup.devEnginesValue);
break;
}
throw new UsageError(`The local project doesn't feature a 'packageManager' field - please specify the package manager to pack, or update the manifest to reference it`);

default: {
Expand Down
38 changes: 24 additions & 14 deletions sources/specUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) {
return pm;
}

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

if (pm) {
if (!pm.startsWith?.(`${name}@`))
Expand All @@ -113,8 +113,9 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) {
return pm;
}


return `${name}@${version ?? `*`}`;
return {spec: `${name}@${version ?? `*`}`, name, version, toString() {
return this.spec;
}};
}

return pm;
Expand All @@ -123,14 +124,15 @@ function parsePackageJSON(packageJSONContent: CorepackPackageJSON) {
export async function setLocalPackageManager(cwd: string, info: PreparedPackageManagerInfo) {
const lookup = await loadSpecAndEnv(cwd);

const range = `range` in lookup && lookup.range;
const projectFound = lookup.type !== `NoProject`;
const range = projectFound && lookup.devEnginesValue;
if (range) {
if (info.locator.name !== range.name || !semverSatisfies(info.locator.reference, range.range)) {
warnOrThrow(`The requested version of ${info.locator.name}@${info.locator.reference} does not match the devEngines specification (${range.name}@${range.range})`, range.onFail);
}
}

const content = lookup.type !== `NoProject`
const content = projectFound
? await fs.promises.readFile(lookup.target, `utf8`)
: ``;

Expand All @@ -151,12 +153,12 @@ interface FoundSpecResult {
type: `Found`;
target: string;
getSpec: (options?: {enforceExactVersion?: boolean}) => Descriptor;
range?: Descriptor & {onFail?: DevEngineDependency[`onFail`]};
devEnginesValue?: Descriptor & {onFail?: DevEngineDependency[`onFail`]};
envFilePath?: string;
}
export type LoadSpecResult =
| {type: `NoProject`, target: string, envFilePath?: string}
| {type: `NoSpec`, target: string, envFilePath?: string}
| {type: `NoSpec`, target: string, envFilePath?: string, devEnginesValue?: FoundSpecResult[`devEnginesValue`]}
| FoundSpecResult;

async function loadEnvFileIfExists(cwd: string): Promise<{env: LocalEnvFile, path: string} | void> {
Expand Down Expand Up @@ -238,18 +240,26 @@ export async function loadSpecAndEnv(initialCwd: string, {envOnly} = {envOnly: f
if (typeof rawPmSpec === `undefined`)
return {type: `NoSpec`, target: selection.manifestPath, envFilePath: localEnv?.path};

debugUtils.log(`${selection.manifestPath} defines ${rawPmSpec} as local package manager`);
const devEnginesValue = selection.data.devEngines?.packageManager?.version && {
name: selection.data.devEngines.packageManager.name,
range: selection.data.devEngines.packageManager.version,
onFail: selection.data.devEngines.packageManager.onFail,
};

if (typeof rawPmSpec === `object` && !semverValid(rawPmSpec.version)) {
debugUtils.log(`${selection.manifestPath} devEngines does not specify a specific version`);
return {type: `NoSpec`, target: selection.manifestPath, envFilePath: localEnv?.path, devEnginesValue};
}

const hasPackageManagerField = typeof rawPmSpec === `string`;
debugUtils.log(`${selection.manifestPath} defines ${rawPmSpec} as local package manager${hasPackageManagerField ? `using packageManager field` : ``}`);

return {
type: `Found`,
target: selection.manifestPath,
envFilePath: localEnv?.path,
range: selection.data.devEngines?.packageManager?.version && {
name: selection.data.devEngines.packageManager.name,
range: selection.data.devEngines.packageManager.version,
onFail: selection.data.devEngines.packageManager.onFail,
},
devEnginesValue,
// Lazy-loading it so we do not throw errors on commands that do not need valid spec.
getSpec: ({enforceExactVersion = true} = {}) => parseSpec(rawPmSpec, path.relative(initialCwd, selection.manifestPath), {enforceExactVersion}),
getSpec: ({enforceExactVersion = true} = {}) => parseSpec(`${rawPmSpec}`, path.relative(initialCwd, selection.manifestPath), {enforceExactVersion}),
};
}
101 changes: 52 additions & 49 deletions tests/main.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,23 +272,6 @@ it(`should ignore the packageManager field when found within a node_modules vend
});

describe(`should handle invalid devEngines values`, () => {
it(`throw on missing version`, async () => {
await xfs.mktempPromise(async cwd => {
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
devEngines: {
packageManager: {
name: `yarn`,
},
},
});

await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
exitCode: 1,
stderr: `Invalid package manager specification in package.json (yarn@*); expected a semver version\n`,
stdout: ``,
});
});
});
it(`throw on invalid version`, async () => {
await xfs.mktempPromise(async cwd => {
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
Expand Down Expand Up @@ -380,8 +363,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`, () => {
it(`should accept if package.json#packageManager field matches`, async () => {
await xfs.mktempPromise(async cwd => {
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
devEngines: {
Expand All @@ -390,18 +373,19 @@ describe(`should accept range in devEngines only if a specific version is provid
version: `6.x`,
},
},
packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`,
});
await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({
exitCode: 1,
stderr: `Invalid package manager specification in package.json (pnpm@6.x); expected a semver version\n`,
stdout: ``,
exitCode: 0,
stderr: ``,
stdout: `6.6.2\n`,
});

// No version should also work
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
devEngines: {
packageManager: {
name: `pnpm`,
version: `6.x`,
},
},
packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`,
Expand All @@ -411,20 +395,60 @@ describe(`should accept range in devEngines only if a specific version is provid
stderr: ``,
stdout: `6.6.2\n`,
});
});
});

// No version should also work
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as PortablePath), {
it(`should accept without a package.json#packageManager field`, async () => {
process.env.AUTH_TYPE = `COREPACK_NPM_TOKEN`;
process.env.TEST_INTEGRITY = `valid`;

await xfs.mktempPromise(async cwd => {
// When no user version is specified, range versions in devEngines should still cause error
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
devEngines: {
packageManager: {
name: `pnpm`,
version: `^1.0.0`,
},
},
packageManager: `pnpm@6.6.2+sha224.eb5c0acad3b0f40ecdaa2db9aa5a73134ad256e17e22d1419a2ab073`,
});
await expect(runCli(cwd, [`pnpm`, `--version`])).resolves.toMatchObject({

// Without user-specified version, should still fail due to range version in devEngines
await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({
exitCode: 0,
stderr: ``,
stdout: `6.6.2\n`,
stdout: `pnpm: Hello from custom registry\n`,
});
});
});

it(`should pin a specific if COREPACK_ENABLE_AUTO_PIN is set`, async () => {
process.env.AUTH_TYPE = `COREPACK_NPM_TOKEN`;
process.env.TEST_INTEGRITY = `valid`;
process.env.COREPACK_ENABLE_AUTO_PIN = `1`;

await xfs.mktempPromise(async cwd => {
const devEngines = {
packageManager: {
name: `pnpm`,
version: `^1.0.0`,
},
};

await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
devEngines,
});

// Without user-specified version, should still fail due to range version in devEngines
await expect(runCli(cwd, [`pnpm`, `--version`], true)).resolves.toMatchObject({
exitCode: 0,
stderr: expect.stringContaining(`local project doesn't define a 'packageManager' field`),
stdout: `pnpm: Hello from custom registry\n`,
});

await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json` as Filename))).resolves.toMatchObject({
packageManager: expect.stringMatching(/^pnpm@1\.9998\.9999\+sha512\.[0-9a-z]{128}$/),
devEngines,
});
});
});
Expand Down Expand Up @@ -1824,24 +1848,3 @@ describe(`allow range versions in devEngines.packageManager.version when user sp
});
}
});

it(`should still validate devEngines.packageManager.version format when no user version specified`, async () => {
await xfs.mktempPromise(async cwd => {
// When no user version is specified, range versions in devEngines should still cause error
await xfs.writeJsonPromise(ppath.join(cwd, `package.json` as Filename), {
devEngines: {
packageManager: {
name: `npm`,
version: `^6.14.2`,
},
},
});

// Without user-specified version, should still fail due to range version in devEngines
await expect(runCli(cwd, [`npm`, `--version`])).resolves.toMatchObject({
exitCode: 1,
stderr: expect.stringContaining(`Invalid package manager specification in package.json (npm@^6.14.2); expected a semver version`),
stdout: ``,
});
});
});