Skip to content

Commit 4be72f6

Browse files
authored
fix(use): do not throw on invalid packageManager (#663)
1 parent bb16184 commit 4be72f6

File tree

5 files changed

+57
-9
lines changed

5 files changed

+57
-9
lines changed

sources/Engine.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -293,19 +293,20 @@ export class Engine {
293293
}
294294

295295
case `Found`: {
296-
if (result.spec.name !== locator.name) {
296+
const spec = result.getSpec();
297+
if (spec.name !== locator.name) {
297298
if (transparent) {
298299
if (typeof locator.reference === `function`)
299300
fallbackDescriptor.range = await locator.reference();
300301

301-
debugUtils.log(`Falling back to ${fallbackDescriptor.name}@${fallbackDescriptor.range} in a ${result.spec.name}@${result.spec.range} project`);
302+
debugUtils.log(`Falling back to ${fallbackDescriptor.name}@${fallbackDescriptor.range} in a ${spec.name}@${spec.range} project`);
302303
return fallbackDescriptor;
303304
} else {
304-
throw new UsageError(`This project is configured to use ${result.spec.name} because ${result.target} has a "packageManager" field`);
305+
throw new UsageError(`This project is configured to use ${spec.name} because ${result.target} has a "packageManager" field`);
305306
}
306307
} else {
307-
debugUtils.log(`Using ${result.spec.name}@${result.spec.range} as defined in project manifest ${result.target}`);
308-
return result.spec;
308+
debugUtils.log(`Using ${spec.name}@${spec.range} as defined in project manifest ${result.target}`);
309+
return spec;
309310
}
310311
}
311312
}

sources/commands/Base.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export abstract class BaseCommand extends Command<Context> {
1919
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`);
2020

2121
default: {
22-
return [lookup.spec];
22+
return [lookup.getSpec()];
2323
}
2424
}
2525
}

sources/commands/deprecated/Prepare.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export class PrepareCommand extends Command<Context> {
4242
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`);
4343

4444
default: {
45-
specs.push(lookup.spec);
45+
specs.push(lookup.getSpec());
4646
}
4747
}
4848
}

sources/specUtils.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ export async function setLocalPackageManager(cwd: string, info: PreparedPackageM
7575
export type LoadSpecResult =
7676
| {type: `NoProject`, target: string}
7777
| {type: `NoSpec`, target: string}
78-
| {type: `Found`, target: string, spec: Descriptor};
78+
| {type: `Found`, target: string, getSpec: () => Descriptor};
7979

8080
export async function loadSpec(initialCwd: string): Promise<LoadSpecResult> {
8181
let nextCwd = initialCwd;
@@ -124,6 +124,7 @@ export async function loadSpec(initialCwd: string): Promise<LoadSpecResult> {
124124
return {
125125
type: `Found`,
126126
target: selection.manifestPath,
127-
spec: parseSpec(rawPmSpec, path.relative(initialCwd, selection.manifestPath)),
127+
// Lazy-loading it so we do not throw errors on commands that do not need valid spec.
128+
getSpec: () => parseSpec(rawPmSpec, path.relative(initialCwd, selection.manifestPath)),
128129
};
129130
}

tests/Use.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,50 @@ describe(`UseCommand`, () => {
7070
});
7171
});
7272
});
73+
74+
describe(`should not care if packageManager is set to an invalid value`, () => {
75+
for (const {description, packageManager} of [
76+
{
77+
description: `when a version range is given`,
78+
packageManager: `[email protected]`,
79+
},
80+
{
81+
description: `when only the pm name is given`,
82+
packageManager: `yarn`,
83+
},
84+
{
85+
description: `when the version is missing`,
86+
packageManager: `yarn@`,
87+
},
88+
{
89+
description: `when the field is not a string`,
90+
packageManager: [],
91+
},
92+
]) {
93+
it(description, async () => {
94+
await xfs.mktempPromise(async cwd => {
95+
await xfs.writeJsonPromise(ppath.join(cwd, `package.json`), {
96+
packageManager,
97+
license: `MIT`, // To avoid warning
98+
});
99+
100+
await expect(runCli(cwd, [`use`, `[email protected]`])).resolves.toMatchObject({
101+
exitCode: 0,
102+
stderr: ``,
103+
stdout: expect.stringMatching(/^Installing yarn@1\.22\.4 in the project\.\.\.\n\nyarn install v1\.22\.4\ninfo No lockfile found\.\n(.*\n)+Done in \d+\.\d+s\.\n$/),
104+
});
105+
106+
await expect(xfs.readJsonPromise(ppath.join(cwd, `package.json`))).resolves.toMatchObject({
107+
packageManager: `[email protected]+sha512.a1833b862fe52169bd6c2a033045a07df5bc6a23595c259e675fed1b2d035ab37abe6ce309720abb6636d68f03615054b6292dc0a70da31c8697fda228b50d18`,
108+
});
109+
110+
await expect(runCli(cwd, [`yarn`, `--version`])).resolves.toMatchObject({
111+
exitCode: 0,
112+
stdout: `1.22.4\n`,
113+
stderr: ``,
114+
});
115+
});
116+
});
117+
}
118+
});
73119
});

0 commit comments

Comments
 (0)