From 576a1471d45cb30fc8fe933759581c97a4527046 Mon Sep 17 00:00:00 2001 From: Lars Kappert Date: Wed, 27 Sep 2023 21:49:49 +0200 Subject: [PATCH] Report @types/pkg as unused if pkg already has types included (resolves #241) --- fixtures/types/index.ts | 2 ++ .../@types/micromatch/package.json | 5 ++++ .../types/node_modules/micromatch/index.js | 1 + .../node_modules/micromatch/package.json | 4 ++++ fixtures/types/node_modules/webpack/index.js | 0 .../types/node_modules/webpack/package.json | 7 ++++++ .../types/node_modules/webpack/types.d.ts | 1 + fixtures/types/package.json | 12 ++++++++++ fixtures/types/tsconfig.json | 1 + src/DependencyDeputy.ts | 14 +++++++++++ src/WorkspaceWorker.ts | 5 +++- src/manifest/index.ts | 6 +++++ tests/types.test.ts | 24 +++++++++++++++++++ 13 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 fixtures/types/index.ts create mode 100644 fixtures/types/node_modules/@types/micromatch/package.json create mode 100644 fixtures/types/node_modules/micromatch/index.js create mode 100644 fixtures/types/node_modules/micromatch/package.json create mode 100644 fixtures/types/node_modules/webpack/index.js create mode 100644 fixtures/types/node_modules/webpack/package.json create mode 100644 fixtures/types/node_modules/webpack/types.d.ts create mode 100644 fixtures/types/package.json create mode 100644 fixtures/types/tsconfig.json create mode 100644 tests/types.test.ts diff --git a/fixtures/types/index.ts b/fixtures/types/index.ts new file mode 100644 index 000000000..47cef8ee0 --- /dev/null +++ b/fixtures/types/index.ts @@ -0,0 +1,2 @@ +import m from 'micromatch'; +m; diff --git a/fixtures/types/node_modules/@types/micromatch/package.json b/fixtures/types/node_modules/@types/micromatch/package.json new file mode 100644 index 000000000..72a2a1e19 --- /dev/null +++ b/fixtures/types/node_modules/@types/micromatch/package.json @@ -0,0 +1,5 @@ +{ + "name": "@types/micromatch", + "main": "", + "types": "index.d.ts" +} diff --git a/fixtures/types/node_modules/micromatch/index.js b/fixtures/types/node_modules/micromatch/index.js new file mode 100644 index 000000000..2d1ec2382 --- /dev/null +++ b/fixtures/types/node_modules/micromatch/index.js @@ -0,0 +1 @@ +export default () => {}; diff --git a/fixtures/types/node_modules/micromatch/package.json b/fixtures/types/node_modules/micromatch/package.json new file mode 100644 index 000000000..19a46d868 --- /dev/null +++ b/fixtures/types/node_modules/micromatch/package.json @@ -0,0 +1,4 @@ +{ + "name": "micromatch", + "main": "index.js" +} diff --git a/fixtures/types/node_modules/webpack/index.js b/fixtures/types/node_modules/webpack/index.js new file mode 100644 index 000000000..e69de29bb diff --git a/fixtures/types/node_modules/webpack/package.json b/fixtures/types/node_modules/webpack/package.json new file mode 100644 index 000000000..82ff2783e --- /dev/null +++ b/fixtures/types/node_modules/webpack/package.json @@ -0,0 +1,7 @@ +{ + "name": "webpack", + "bin": { + "webpack": "index.js" + }, + "types": "types.d.ts" +} diff --git a/fixtures/types/node_modules/webpack/types.d.ts b/fixtures/types/node_modules/webpack/types.d.ts new file mode 100644 index 000000000..4708a1ee4 --- /dev/null +++ b/fixtures/types/node_modules/webpack/types.d.ts @@ -0,0 +1 @@ +declare interface Webpack {} diff --git a/fixtures/types/package.json b/fixtures/types/package.json new file mode 100644 index 000000000..bbe908899 --- /dev/null +++ b/fixtures/types/package.json @@ -0,0 +1,12 @@ +{ + "name": "@fixtures/types", + "scripts": { + "pack": "webpack" + }, + "devDependencies": { + "micromatch": "*", + "webpack": "*", + "@types/micromatch": "*", + "@types/webpack": "*" + } +} diff --git a/fixtures/types/tsconfig.json b/fixtures/types/tsconfig.json new file mode 100644 index 000000000..0967ef424 --- /dev/null +++ b/fixtures/types/tsconfig.json @@ -0,0 +1 @@ +{} diff --git a/src/DependencyDeputy.ts b/src/DependencyDeputy.ts index cae4c994f..015c8aeb8 100644 --- a/src/DependencyDeputy.ts +++ b/src/DependencyDeputy.ts @@ -28,6 +28,7 @@ export class DependencyDeputy { referencedBinaries: Map>; hostDependencies: Map; installedBinaries: Map; + hasTypesIncluded: Map>; ignoreBinaries: string[] = []; ignoreDependencies: string[] = []; @@ -37,6 +38,7 @@ export class DependencyDeputy { this.referencedBinaries = new Map(); this.hostDependencies = new Map(); this.installedBinaries = new Map(); + this.hasTypesIncluded = new Map(); } public addWorkspace({ @@ -112,6 +114,14 @@ export class DependencyDeputy { return this.installedBinaries.get(workspaceName); } + setHasTypesIncluded(workspaceName: string, hasTypesIncluded: Set) { + this.hasTypesIncluded.set(workspaceName, hasTypesIncluded); + } + + getHasTypesIncluded(workspaceName: string) { + return this.installedBinaries.get(workspaceName); + } + addReferencedDependency(workspaceName: string, packageName: string) { if (!this.referencedDependencies.has(workspaceName)) { this.referencedDependencies.set(workspaceName, new Set()); @@ -217,6 +227,7 @@ export class DependencyDeputy { for (const [workspaceName, { manifestPath, ignoreDependencies, ignoreBinaries }] of this._manifests.entries()) { const referencedDependencies = this.referencedDependencies.get(workspaceName); const installedBinaries = this.getInstalledBinaries(workspaceName); + const hasTypesIncluded = this.getHasTypesIncluded(workspaceName); const ignoreBins = [...IGNORED_GLOBAL_BINARIES, ...this.ignoreBinaries, ...ignoreBinaries]; const ignoreDeps = [...IGNORED_DEPENDENCIES, ...this.ignoreDependencies, ...ignoreDependencies]; @@ -244,6 +255,9 @@ export class DependencyDeputy { const [scope, typedDependency] = dependency.split('/'); if (scope === '@types') { + // The `pkg` dependency already has types included, i.e. this `@types/pkg` is obsolete + if (hasTypesIncluded?.has(typedDependency)) return false; + const typedPackageName = getPackageFromDefinitelyTyped(typedDependency); // Ignore `@types/*` packages that don't have a related dependency (e.g. `@types/node`) if (IGNORE_DEFINITELY_TYPED.includes(typedPackageName)) return true; diff --git a/src/WorkspaceWorker.ts b/src/WorkspaceWorker.ts index ff0e68a43..869026ce5 100644 --- a/src/WorkspaceWorker.ts +++ b/src/WorkspaceWorker.ts @@ -53,6 +53,7 @@ export class WorkspaceWorker { referencedDependencies: ReferencedDependencies = new Set(); hostDependencies: HostDependencies = new Map(); installedBinaries: InstalledBinaries = new Map(); + hasTypesIncluded: Set = new Set(); constructor({ name, @@ -115,7 +116,7 @@ export class WorkspaceWorker { } private async initReferencedDependencies() { - const { dependencies, hostDependencies, installedBinaries } = await npm.findDependencies({ + const { dependencies, hostDependencies, installedBinaries, hasTypesIncluded } = await npm.findDependencies({ manifest: this.manifest, isProduction: this.isProduction, isStrict: this.isStrict, @@ -127,6 +128,7 @@ export class WorkspaceWorker { dependencies.forEach(dependency => this.referencedDependencies.add([filePath, dependency])); this.hostDependencies = hostDependencies; this.installedBinaries = installedBinaries; + this.hasTypesIncluded = hasTypesIncluded; } private getConfigForPlugin(pluginName: PluginName): PluginConfiguration { @@ -316,6 +318,7 @@ export class WorkspaceWorker { hostDependencies: this.hostDependencies, installedBinaries: this.installedBinaries, referencedDependencies: this.referencedDependencies, + hasTypesIncluded: this.hasTypesIncluded, enabledPlugins: this.enabledPlugins, }; } diff --git a/src/manifest/index.ts b/src/manifest/index.ts index 5f6f91100..62b1001b5 100644 --- a/src/manifest/index.ts +++ b/src/manifest/index.ts @@ -1,4 +1,5 @@ import { _getDependenciesFromScripts } from '../binaries/index.js'; +import { isDefinitelyTyped } from '../util/modules.js'; import { timerify } from '../util/Performance.js'; import { getPackageManifest } from './helpers.js'; import type { InstalledBinaries, HostDependencies } from '../types/workspace.js'; @@ -28,6 +29,8 @@ const findManifestDependencies = async ({ manifest, isProduction, isStrict, dir, // Find all binaries for each dependency const installedBinaries: InstalledBinaries = new Map(); + const hasTypesIncluded = new Set(); + const packageNames = [ ...Object.keys(manifest.dependencies ?? {}), ...(isStrict ? Object.keys(manifest.peerDependencies ?? {}) : []), @@ -62,6 +65,8 @@ const findManifestDependencies = async ({ manifest, isProduction, isStrict, dir, hostDependencies.set(packagePeerDependency, new Set([packageName])); } }); + + if (!isDefinitelyTyped(packageName) && (manifest.types || manifest.typings)) hasTypesIncluded.add(packageName); } } @@ -69,6 +74,7 @@ const findManifestDependencies = async ({ manifest, isProduction, isStrict, dir, dependencies, hostDependencies, installedBinaries, + hasTypesIncluded, }; }; diff --git a/tests/types.test.ts b/tests/types.test.ts new file mode 100644 index 000000000..2d74ebd70 --- /dev/null +++ b/tests/types.test.ts @@ -0,0 +1,24 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { main } from '../src/index.js'; +import { resolve } from '../src/util/path.js'; +import baseArguments from './helpers/baseArguments.js'; +import baseCounters from './helpers/baseCounters.js'; + +const cwd = resolve('fixtures/types'); + +test('Find @types/pkg that are obsolete, since pkg has types included', async () => { + const { issues, counters } = await main({ + ...baseArguments, + cwd, + }); + + assert(issues.devDependencies['package.json']['@types/webpack']); + + assert.deepEqual(counters, { + ...baseCounters, + devDependencies: 1, + processed: 1, + total: 1, + }); +});