From 2d9a673ec8655f91f005ffc1e76ae31327d50f81 Mon Sep 17 00:00:00 2001 From: Ryan Bas <18267769+ryanbas21@users.noreply.github.com> Date: Tue, 11 Feb 2025 13:20:51 -0700 Subject: [PATCH] fix(linter): dependency-check-support-catalogs (#29923) ## Current Behavior Currently there doesn't seem to be support for pnpm catalogs in the dependency check which throws an error. ## Expected Behavior Catalog should be supported and at least not throw an error since its a default rule in some generators. I noticed that also that only `workspace:*` was implemented so I added the other possible options. https://pnpm.io/workspaces#publishing-workspace-packages I copy and pasted a test twice to add tests for this. I mean ideally I guess we would check the `pnpm-workspace` file and say is this catalog defined, I don't think that's up my ally for this purpose and I'm not sure if how its implemented does that for the workspace protocols as well. ## Related Issue(s) Fixes # closed #29903, #29959 Gave it a shot, not sure if this is gonna be comprehensive enough. But let me know! happy to try and get this right. --------- Co-authored-by: Jack Hsu --- .../src/rules/dependency-checks.spec.ts | 131 +++++++++++++++++- .../src/rules/dependency-checks.ts | 8 +- 2 files changed, 136 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/dependency-checks.spec.ts b/packages/eslint-plugin/src/rules/dependency-checks.spec.ts index c30dbfe8a7..4c1a10c22b 100644 --- a/packages/eslint-plugin/src/rules/dependency-checks.spec.ts +++ b/packages/eslint-plugin/src/rules/dependency-checks.spec.ts @@ -30,6 +30,8 @@ const rootPackageJson = { dependencies: { external1: '~16.1.2', external2: '^5.2.0', + external3: '1.0.0', + external4: '1.0.0', }, devDependencies: { tslib: '^2.1.0', @@ -53,6 +55,22 @@ const externalNodes: Record = { version: '5.5.6', }, }, + 'npm:external3': { + name: 'npm:external3', + type: 'npm', + data: { + packageName: 'external3', + version: '1.0.0', + }, + }, + 'npm:external4': { + name: 'npm:external3', + type: 'npm', + data: { + packageName: 'external4', + version: '1.0.0', + }, + }, 'npm:random-external': { name: 'npm:random-external', type: 'npm', @@ -1581,12 +1599,121 @@ describe('Dependency checks (eslint)', () => { `); }); - it('should not report * and workspace:*', () => { + it('should not report *', () => { const packageJson = { name: '@mycompany/liba', dependencies: { external1: '*', - external2: 'workspace:*', + }, + }; + + const fileSys = { + './libs/liba/package.json': JSON.stringify(packageJson, null, 2), + './libs/liba/src/index.ts': '', + './package.json': JSON.stringify(rootPackageJson, null, 2), + }; + vol.fromJSON(fileSys, '/root'); + + const failures = runRule( + {}, + `/root/libs/liba/package.json`, + JSON.stringify(packageJson, null, 2), + { + nodes: { + liba: { + name: 'liba', + type: 'lib', + data: { + root: 'libs/liba', + targets: { + build: {}, + }, + }, + }, + }, + externalNodes, + dependencies: { + liba: [{ source: 'liba', target: 'npm:external1', type: 'static' }], + }, + }, + { + liba: [createFile(`libs/liba/src/main.ts`, ['npm:external1'])], + } + ); + expect(failures.length).toEqual(0); + }); + + it('should not report workspace: protocol', () => { + const packageJson = { + name: '@mycompany/liba', + dependencies: { + external1: 'workspace:~', + external2: 'workspace:^', + external3: 'workspace:', + external4: 'workspace:../external4', + }, + }; + + const fileSys = { + './libs/liba/package.json': JSON.stringify(packageJson, null, 2), + './libs/liba/src/index.ts': '', + './package.json': JSON.stringify(rootPackageJson, null, 2), + }; + vol.fromJSON(fileSys, '/root'); + + const failures = runRule( + {}, + `/root/libs/liba/package.json`, + JSON.stringify(packageJson, null, 2), + { + nodes: { + liba: { + name: 'liba', + type: 'lib', + data: { + root: 'libs/liba', + targets: { + build: {}, + }, + }, + }, + }, + externalNodes, + dependencies: { + liba: [ + { source: 'liba', target: 'npm:external1', type: 'static' }, + { source: 'liba', target: 'npm:external2', type: 'static' }, + { source: 'liba', target: 'npm:external3', type: 'static' }, + { source: 'liba', target: 'npm:external4', type: 'static' }, + ], + }, + }, + { + liba: [ + createFile(`libs/liba/src/main.ts`, [ + 'npm:external1', + 'npm:external2', + 'npm:external3', + 'npm:external4', + ]), + createFile(`libs/liba/package.json`, [ + 'npm:external1', + 'npm:external2', + 'npm:external3', + 'npm:external4', + ]), + ], + } + ); + expect(failures.length).toEqual(0); + }); + + it('should not report catalog: protocol', () => { + const packageJson = { + name: '@mycompany/liba', + dependencies: { + external1: 'catalog:', + external2: 'catalog:nx', }, }; diff --git a/packages/eslint-plugin/src/rules/dependency-checks.ts b/packages/eslint-plugin/src/rules/dependency-checks.ts index 4b90c148e4..0a22469e79 100644 --- a/packages/eslint-plugin/src/rules/dependency-checks.ts +++ b/packages/eslint-plugin/src/rules/dependency-checks.ts @@ -218,7 +218,13 @@ export default ESLintUtils.RuleCreator( packageRange.startsWith('file:') || npmDependencies[packageName] === '*' || packageRange === '*' || - packageRange === 'workspace:*' || + packageRange.startsWith('workspace:') || + /** + * Catalogs can be named, or left unnamed + * So just checking up until the : will catch both cases + * e.g. catalog:some-catalog or catalog: + */ + packageRange.startsWith('catalog:') || satisfies(npmDependencies[packageName], packageRange, { includePrerelease: true, })