webpro-nl / knip

✂️ Find unused files, dependencies and exports in your JavaScript and TypeScript projects. Knip it before you ship it!
https://knip.dev
ISC License
7.06k stars 177 forks source link

Allow other angular builders instead of default #717

Closed muuvmuuv closed 4 months ago

muuvmuuv commented 4 months ago

Hey, I saw that Angular is supported built-in, but we use a different builder (to customize esbuild), would love to see an option (maybe dot-path) to customize Angular main file. We use "@angular-builders/custom-esbuild:application". Also since Angular latest release "main" is deprecated in favor of "browser" with application builder. So any configuration would make this more future proof.

webpro commented 4 months ago

Can you give more details, like an actual example of relevant configurationm, and what you'd expect from Knip here? And what's the output you're seeing now?

muuvmuuv commented 4 months ago

Here it says https://knip.dev/explanations/plugins#angular:

This will result in src/main.ts being added as an entry file (and @angular-devkit/build-angular as a referenced dependency).

Which wont work with other builders. Setting it manually works partly (angular deps are marked as missing peers and others that are used in ts files):

{
    "$schema": "https://unpkg.com/knip@5/schema.json",
    "entry": ["src/main.ts"],
    "project": ["src/*.{js,ts}"]
}

A Angular built-in plugin should maybe look for "defaultProject" in angular.json (or pick the first app that appears) and pick the first builder option as "build" then looks for "main" or "browser" to get the entry file. And uses "sourceRoot" as "project" instead of "*/.{js,ts}".

Here a part of our angular.json:

{
    "version": 1,
    "$schema": "./node_modules/@angular/cli/lib/config/schema.json",
    "projects": {
        "app": {
            "root": "./",
            "sourceRoot": "src",
            "projectType": "application",
            "prefix": "app",
            "architect": {
                "build": {
                    "builder": "@angular-builders/custom-esbuild:application",
                    "options": {
                        "browser": "./src/main.ts",
                        "index": "./src/index.html",
                        "outputPath": "./www",
                        "tsConfig": "./tsconfig.app.json",
                        "inlineStyleLanguage": "scss",
                        "crossOrigin": "anonymous",

Also CLI devDeps are marked as "unused" but I guess I have not yet found the relevant option to exclude them.

webpro commented 4 months ago

Which wont work with other builders.

The Angular plugin is quite generic and I think Knip detects @angular-builders/custom-esbuild as a dependency.

A Angular built-in plugin should maybe look for "defaultProject" in angular.json (or pick the first app that appears) and pick the first builder option as "build"

This should already happen, it iterates over all projects → architect targets.

The Angular plugin implementation is not that much code even: https://github.com/webpro-nl/knip/blob/main/packages/knip/src/plugins/angular/index.ts#L26-L38

then looks for "main" or "browser" to get the entry file.

That src/main.ts is a default entry point. Additional to main, Knip could add the browser field.

And uses "sourceRoot" as "project" instead of "*/.{js,ts}".

That's a good RFC. For now you can use a manual project: "src/**/*.{js,ts}"

Would still be good to better understand what's not working well for you. A reproduction or incorrect output would be helpful.

muuvmuuv commented 4 months ago

Oh yes, I see. Great so far.

It works now, it was just hard to get it working with Angular. Without specifying project it also tried my ios- and android-build dirs to scan which produced a lot of errors. After adjusting the config it works. Just wanted to point out that it does not out of the box with current Angular plugin.

webpro commented 4 months ago

So your point is mostly that the sourceRoot could be respected, right? The challenge here is that Knip plugins do not have the ability to override the global project config.

Using project: ["src/**/*.ts"] over the default probably should be promoted better I guess :)

muuvmuuv commented 4 months ago

Yes mostly, and that it would require zero config to use with Angular.

webpro commented 4 months ago

Thanks!