Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13,399 changes: 13,399 additions & 0 deletions package-lock.json

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions packages/knip/fixtures/plugins/aws-cdk/bin/infra.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// @ts-ignore
import * as cdk from 'aws-cdk-lib';
import { MyStack } from '../lib/myStack';

const app = new cdk.App();
new MyStack(app, 'MyStack');

app.synth();
19 changes: 19 additions & 0 deletions packages/knip/fixtures/plugins/aws-cdk/cdk.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"app": "ts-node --project=tsconfig.json --prefer-ts-exts cdk/bin/infra.ts",
"watch": {
"include": ["cdk/lib/**", "cdk/bin/**"]
},
"context": {
"aws-cdk-lib/core:enableDiffNoFail": true,
"aws-cdk-lib/core:bootstrapQualifier": "custom-v2",
"aws-cdk-lib/core:checkSecretUsage": true,
"aws-cdk-lib/core:target-partitions": [
"aws",
"aws-cn"
],
"aws-cdk-lib/aws-apigateway:disableCloudWatchRole": true,
"aws-cdk-lib/aws-apigateway:usagePlanKeyOrderInsensitiveId": true,
"aws-cdk-lib/aws-cloudfront:defaultSecurityPolicyTLSv1.2_2021": false,
"aws-cdk-lib/aws-lambda:recognizeLayerVersion": true
}
}
10 changes: 10 additions & 0 deletions packages/knip/fixtures/plugins/aws-cdk/lib/myConstruct.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// @ts-ignore
import { Construct } from 'constructs';

export class MyConstruct extends Construct {
constructor(scope: Construct, id: string) {
super(scope, id);

// The code that defines your stack goes here
}
}
11 changes: 11 additions & 0 deletions packages/knip/fixtures/plugins/aws-cdk/lib/myStack.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// @ts-ignore
import * as cdk from 'aws-cdk-lib';

export class MyStack extends cdk.Stack {
constructor(scope: cdk.App, id: string) {
super(scope, id);

// The code that defines your stack goes here
}
}

Empty file.

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

14 changes: 14 additions & 0 deletions packages/knip/fixtures/plugins/aws-cdk/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"name": "@fixtures/aws-cdk",
"dependencies": {
"aws-cdk-lib": "^2.0.0",
"constructs": "^10.0.0"
},
"devDependencies": {
"aws-cdk": "^2.0.0",
"@types/aws-cdk-lib": "^2.0.0",
"@types/constructs": "^10.0.0",
"ts-node": "^10.0.0",
"typescript": "^5.0.0"
}
}
4 changes: 4 additions & 0 deletions packages/knip/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,10 @@
"title": "ava plugin configuration (https://knip.dev/reference/plugins/ava)",
"$ref": "#/definitions/plugin"
},
"aws-cdk": {
"title": "aws-cdk plugin configuration (https://knip.dev/reference/plugins/aws-cdk)",
"$ref": "#/definitions/plugin"
},
"babel": {
"title": "Babel plugin configuration (https://knip.dev/reference/plugins/babel)",
"$ref": "#/definitions/plugin"
Expand Down
38 changes: 38 additions & 0 deletions packages/knip/src/plugins/aws-cdk/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { toDependency } from 'src/util/input.js';
import type { IsPluginEnabled, Plugin, ResolveConfig } from '../../types/config.js';
import { compact } from '../../util/array.js';
import { hasDependency } from '../../util/plugin.js';
import type { AwsCdkConfig } from './types.ts';

// https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib-readme.html

const title = 'aws-cdk';

const enablers = ['aws-cdk'];

const isEnabled: IsPluginEnabled = ({ dependencies }) => hasDependency(dependencies, enablers);

const config: string[] = ["cdk.json"];

const resolveConfig: ResolveConfig<AwsCdkConfig> = async config => {
if (!config) return [];

const app = config.app.split(" ")[0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The app option looks like a command, we should use getInputsFromScripts for this.

This function is available on the second options argument that ResolveConfig is invoked with.

Look for getInputsFromScripts in the plugins folder for other usage examples.

Copy link
Author

@thoroc thoroc Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 7c45bbe. Just a question though: what is the knownBinsOnly option for?

PS: I initially forgot to run the test again (somehow) before pushing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By default, binaries are expected to be shipped alongside the dependencies in node_modules/.bin - it's a setting that allows to be less strict and accept "any" binary name in CI environments—which are usually provisioned with unknown binaries—to prevent false positives for "unlisted binaries". However, the variable should def be improved.

const watch = config.watch?.include ?? [];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My guess would be that the watch values aren't necessarily dependencies, nor entry files. Maybe we should ignore those. Maybe there are other options that represent entry points?

Copy link
Author

@thoroc thoroc Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To the best of my knowledge, the watch.include entries are file that should form the codebase for the IaC when using AWS CDK. From the name, it would indicate those are source files being watched for changes when running cdk watch which would allow an immediate changes of the generated CloudFormation template.

Those are not mandatory properties on the json Object, but should they be specified, they should be identical to the expected source files (bin/**/*.ts, lib/**/*.ts, cdk/**/*.ts. It is possible that some project are configured differently and those source files are living somewhere else. I would probably need help determining how to leave it to knip's users to specify this through configuration.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we would add the watch.include patterns, then they should be returned as project files (not entry files), i.e. toInput('bin/**/*.ts'). Adding "all" files as entry files would have two disadvantages: by default, unused exports of entry files are not reported, and Knip wouldn't be able to find unused files (as entry files are inherently used files).

However, the default project pattern is **/*.{js,ts}, so adding more specific patterns from a plugin wouldn't be effective.

My suggestion would be to ignore the watch.include patterns and see whether the defaults Knip finds are sufficient, then we could add default entry patterns in the plugin if there are any, and perhaps there are specific entry patterns to be found in aws-cdk configuration files.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok that makes sense. I'll remove that 'watch' from the returned list.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 7efb03d

const context = Object.keys(config.context ?? {}).map(key => key.split("/")[0]) ?? [];
return compact([app, ...watch, ...context]).map(id => toDependency(id));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that we're using compact here and already convert string to so-called "inputs" using toDependency. However, even though the context items are such dependencies indeed, returned items could also be entry points. You'll see what I mean when using getInputsFromScripts - its return value could contain various types of inputs.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you meaning that I should be checking for the type of id in the map function? I saw typescript complaining about mismatched type and used something I saw somewhere else in the codebase:

compact([...app, ...watch, ...context]).map(id => (typeof id === 'string' ? toDependency(id) : id));

Copy link
Member

@webpro webpro Apr 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, something like that, e.g. the return value of getInputsFromScripts is inputs, not strings.

};

const production: string[] = [
'{src/,cdk/,}bin/**/*.{js,ts}',
'{src/,cdk/,}lib/**/*.{js,ts}',
];

export default {
title,
enablers,
isEnabled,
config,
resolveConfig,
production,
} satisfies Plugin;
7 changes: 7 additions & 0 deletions packages/knip/src/plugins/aws-cdk/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export type AwsCdkConfig = {
app: string;
watch?: {
include: string[];
};
context?: Record<string, unknown>;
};
2 changes: 2 additions & 0 deletions packages/knip/src/plugins/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { default as angular } from './angular/index.js';
import { default as astro } from './astro/index.js';
import { default as ava } from './ava/index.js';
import { default as awsCdk } from './aws-cdk/index.js';
import { default as babel } from './babel/index.js';
import { default as c8 } from './c8/index.js';
import { default as capacitor } from './capacitor/index.js';
Expand Down Expand Up @@ -101,6 +102,7 @@ export const Plugins = {
angular,
astro,
ava,
'aws-cdk': awsCdk,
babel,
c8,
capacitor,
Expand Down
1 change: 1 addition & 0 deletions packages/knip/src/schema/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const pluginsSchema = z.object({
angular: pluginSchema,
astro: pluginSchema,
ava: pluginSchema,
'aws-cdk': pluginSchema,
babel: pluginSchema,
c8: pluginSchema,
capacitor: pluginSchema,
Expand Down
2 changes: 2 additions & 0 deletions packages/knip/src/types/PluginNames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ export type PluginName =
| 'angular'
| 'astro'
| 'ava'
| 'aws-cdk'
| 'babel'
| 'c8'
| 'capacitor'
Expand Down Expand Up @@ -102,6 +103,7 @@ export const pluginNames = [
'angular',
'astro',
'ava',
'aws-cdk',
'babel',
'c8',
'capacitor',
Expand Down
28 changes: 28 additions & 0 deletions packages/knip/test/plugins/aws-cdk.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { test } from 'bun:test';
import assert from 'node:assert/strict';
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/plugins/aws-cdk');

test('Find dependencies with the aws-cdk plugin', async () => {
const { issues, counters } = await main({
...baseArguments,
cwd,
});

console.log(issues);

assert(issues.devDependencies['package.json']['aws-cdk']);

assert.deepEqual(counters, {
...baseCounters,
dependencies: 0,
devDependencies: 1,
unlisted: 1,
processed: 3,
total: 3,
});
});