Skip to content

Commit afe40b1

Browse files
authored
Allow catalogs to work with descriptors without resolvers (#6930)
## What's the problem this PR addresses? <!-- Describe the rationale of your PR. --> <!-- Link all issues that it closes. (Closes/Resolves #xxxx.) --> Resolves #6928 ## How did you fix it? <!-- A detailed description of your implementation. --> I modified the catalog plugin to skip binding a descriptor when the descriptor referenced by the catalog protocol is not supported by any resolver. Since an error occurs when binding a descriptor that has no supporting resolver, I added a check before calling `resolver.bindDescriptor`. Even if the catalog plugin doesn't resolve the descriptor, an error will still be properly raised during the final resolution step if no resolver can handle the descriptor. ## Checklist <!--- Don't worry if you miss something, chores are automatically tested. --> <!--- This checklist exists to help you remember doing the chores when you submit a PR. --> <!--- Put an `x` in all the boxes that apply. --> - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). <!-- See https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released for more details. --> <!-- Check with `yarn version check` and fix with `yarn version check -i` --> - [x] I have set the packages that need to be released for my changes to be effective. <!-- The "Testing chores" workflow validates that your PR follows our guidelines. --> <!-- If it doesn't pass, click on it to see details as to what your PR might be missing. --> - [x] I will check that all automated PR checks pass before the PR gets reviewed.
1 parent aa71c69 commit afe40b1

File tree

4 files changed

+105
-0
lines changed

4 files changed

+105
-0
lines changed

.yarn/versions/f352ecbe.yml

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
releases:
2+
"@yarnpkg/cli": patch
3+
"@yarnpkg/plugin-catalog": patch
4+
5+
declined:
6+
- "@yarnpkg/plugin-compat"
7+
- "@yarnpkg/plugin-constraints"
8+
- "@yarnpkg/plugin-dlx"
9+
- "@yarnpkg/plugin-essentials"
10+
- "@yarnpkg/plugin-init"
11+
- "@yarnpkg/plugin-interactive-tools"
12+
- "@yarnpkg/plugin-nm"
13+
- "@yarnpkg/plugin-npm-cli"
14+
- "@yarnpkg/plugin-pack"
15+
- "@yarnpkg/plugin-patch"
16+
- "@yarnpkg/plugin-pnp"
17+
- "@yarnpkg/plugin-pnpm"
18+
- "@yarnpkg/plugin-stage"
19+
- "@yarnpkg/plugin-typescript"
20+
- "@yarnpkg/plugin-version"
21+
- "@yarnpkg/plugin-workspace-tools"
22+
- "@yarnpkg/builder"
23+
- "@yarnpkg/core"
24+
- "@yarnpkg/doctor"

packages/acceptance-tests/pkg-tests-specs/sources/features/catalogs.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,34 @@
11
import {PortablePath, xfs} from '@yarnpkg/fslib';
22
import {yarn, fs as fsUtils} from 'pkg-tests-core';
33

4+
const CUSTOM_PROTOCOL_PLUGIN = `
5+
module.exports = {
6+
name: 'plugin-custom-protocol',
7+
factory: function(require) {
8+
const {structUtils} = require('@yarnpkg/core');
9+
10+
return {
11+
default: {
12+
hooks: {
13+
reduceDependency(dependency, project) {
14+
if (!dependency.range.startsWith('custom-protocol:')) {
15+
return dependency;
16+
}
17+
18+
const version = dependency.range.slice('custom-protocol:'.length);
19+
20+
return structUtils.makeDescriptor(
21+
structUtils.makeIdent(dependency.scope, dependency.name),
22+
\`npm:\${version}\`
23+
);
24+
}
25+
}
26+
}
27+
};
28+
}
29+
};
30+
`;
31+
432
describe(`Features`, () => {
533
describe(`Catalogs`, () => {
634
test(
@@ -213,6 +241,34 @@ describe(`Features`, () => {
213241
),
214242
);
215243

244+
test(
245+
`it should handle custom descriptor supported by a plugin without a resolver`,
246+
makeTemporaryEnv(
247+
{
248+
dependencies: {
249+
[`no-deps`]: `catalog:`,
250+
},
251+
},
252+
async ({path, run, source}) => {
253+
await xfs.writeFilePromise(`${path}/plugin-custom-protocol.js` as PortablePath, CUSTOM_PROTOCOL_PLUGIN);
254+
255+
await yarn.writeConfiguration(path, {
256+
plugins: [`./plugin-custom-protocol.js`],
257+
catalog: {
258+
[`no-deps`]: `custom-protocol:2.0.0`,
259+
},
260+
});
261+
262+
await run(`install`);
263+
264+
await expect(source(`require('no-deps')`)).resolves.toMatchObject({
265+
name: `no-deps`,
266+
version: `2.0.0`,
267+
});
268+
},
269+
),
270+
);
271+
216272
test(
217273
`it should throw an error when catalog is not found`,
218274
makeTemporaryEnv(
@@ -271,6 +327,26 @@ describe(`Features`, () => {
271327
),
272328
);
273329

330+
test(
331+
`it should throw an error when protocol in catalog isn't supported by any resolver`,
332+
makeTemporaryEnv(
333+
{
334+
dependencies: {
335+
[`no-deps`]: `catalog:`,
336+
},
337+
},
338+
async ({path, run, source}) => {
339+
await yarn.writeConfiguration(path, {
340+
catalog: {
341+
[`no-deps`]: `unknown-protocol:2.0.0`,
342+
},
343+
});
344+
345+
await expect(run(`install`)).rejects.toThrow();
346+
},
347+
),
348+
);
349+
274350
test(
275351
`it should work with file: protocol ranges in catalogs`,
276352
makeTemporaryEnv(

packages/plugin-catalog/sources/utils.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ export const resolveDescriptorFromCatalog = (project: Project, dependency: Descr
5353
structUtils.makeDescriptor(dependency, resolvedRange),
5454
);
5555

56+
// If the descriptor isn't supported by any available resolver, return it as is
57+
if (!resolver.supportsDescriptor(normalizedDescriptor, resolveOptions))
58+
return normalizedDescriptor;
59+
5660
// Bind the descriptor to the project's top level workspace (which should match the project root),
5761
// addressing issues with relative file paths when using `file:` protocol
5862
const boundDescriptor = resolver.bindDescriptor(normalizedDescriptor, project.topLevelWorkspace.anchoredLocator, resolveOptions);

packages/plugin-catalog/tests/utils.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ describe(`utils`, () => {
9696
// Create mock resolver with bindDescriptor method
9797
mockResolver = {
9898
bindDescriptor: jest.fn(descriptor => descriptor),
99+
supportsDescriptor: jest.fn(() => true),
99100
} as any;
100101

101102
resolveOptions = {

0 commit comments

Comments
 (0)