Skip to content

Commit

Permalink
Report @types/pkg as unused if pkg already has types included (resolves
Browse files Browse the repository at this point in the history
  • Loading branch information
webpro committed Sep 27, 2023
1 parent fedd56d commit 576a147
Show file tree
Hide file tree
Showing 13 changed files with 81 additions and 1 deletion.
2 changes: 2 additions & 0 deletions fixtures/types/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import m from 'micromatch';
m;
5 changes: 5 additions & 0 deletions fixtures/types/node_modules/@types/micromatch/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions fixtures/types/node_modules/micromatch/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions fixtures/types/node_modules/micromatch/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Empty file.
7 changes: 7 additions & 0 deletions fixtures/types/node_modules/webpack/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions fixtures/types/node_modules/webpack/types.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions fixtures/types/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "@fixtures/types",
"scripts": {
"pack": "webpack"
},
"devDependencies": {
"micromatch": "*",
"webpack": "*",
"@types/micromatch": "*",
"@types/webpack": "*"
}
}
1 change: 1 addition & 0 deletions fixtures/types/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
14 changes: 14 additions & 0 deletions src/DependencyDeputy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export class DependencyDeputy {
referencedBinaries: Map<string, Set<string>>;
hostDependencies: Map<string, HostDependencies>;
installedBinaries: Map<string, InstalledBinaries>;
hasTypesIncluded: Map<string, Set<string>>;
ignoreBinaries: string[] = [];
ignoreDependencies: string[] = [];

Expand All @@ -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({
Expand Down Expand Up @@ -112,6 +114,14 @@ export class DependencyDeputy {
return this.installedBinaries.get(workspaceName);
}

setHasTypesIncluded(workspaceName: string, hasTypesIncluded: Set<string>) {
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());
Expand Down Expand Up @@ -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];

Expand Down Expand Up @@ -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;
Expand Down
5 changes: 4 additions & 1 deletion src/WorkspaceWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ export class WorkspaceWorker {
referencedDependencies: ReferencedDependencies = new Set();
hostDependencies: HostDependencies = new Map();
installedBinaries: InstalledBinaries = new Map();
hasTypesIncluded: Set<string> = new Set();

constructor({
name,
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -316,6 +318,7 @@ export class WorkspaceWorker {
hostDependencies: this.hostDependencies,
installedBinaries: this.installedBinaries,
referencedDependencies: this.referencedDependencies,
hasTypesIncluded: this.hasTypesIncluded,
enabledPlugins: this.enabledPlugins,
};
}
Expand Down
6 changes: 6 additions & 0 deletions src/manifest/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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<string>();

const packageNames = [
...Object.keys(manifest.dependencies ?? {}),
...(isStrict ? Object.keys(manifest.peerDependencies ?? {}) : []),
Expand Down Expand Up @@ -62,13 +65,16 @@ const findManifestDependencies = async ({ manifest, isProduction, isStrict, dir,
hostDependencies.set(packagePeerDependency, new Set([packageName]));
}
});

if (!isDefinitelyTyped(packageName) && (manifest.types || manifest.typings)) hasTypesIncluded.add(packageName);
}
}

return {
dependencies,
hostDependencies,
installedBinaries,
hasTypesIncluded,
};
};

Expand Down
24 changes: 24 additions & 0 deletions tests/types.test.ts
Original file line number Diff line number Diff line change
@@ -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,
});
});

0 comments on commit 576a147

Please sign in to comment.