webpro / knip

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

Add support for Ionic / enhance the capacitor plugin #604

Closed ptmkenny closed 2 weeks ago

ptmkenny commented 3 weeks ago

knip already has a plugin for capacitor, but it doesn't seem to have anything for Ionic. Ionic is a framework built on top of capacitor (by the same developers) that provides the developer's choice of PWA, Android, iOS, and/or Windows apps.

I ran knip in my Ionic project with no configuration and, because of the lack of Ionic support, some parts of the report are incorrect.

Specifically:

Unused dependencies                                                                                                                                                   
@capacitor/android       package.json                                                                                                                          
@capacitor/ios           package.json

@capacitor/android and @capacitor/ios are packages for building mobile apps with the capacitor cli. (getting started instructions)

So they are used by the project, but knip seems unable to detect that.

Also:

Unused files
ios/App/App/public/chunk-web.231b8fdf.js                                                                                                                                   16:15:02
ios/App/App/public/chunk-web.38bdad3e.js                                                                                                                                   16:15:02
ios/App/App/public/chunk-web.51dd9266.js                                                                                                                                   16:15:02
ios/App/App/public/chunk-web.599da473.js                                                                                                                                   16:15:02
ios/App/App/public/chunk-web.63c4dae5.js                                                                                                                                   16:15:02
ios/App/App/public/chunk-web.665e8798.js                                                                                                                                   16:15:02
ios/App/App/public/chunk-web.d233a663.js                                                                                                                                   16:15:02
ios/App/App/public/chunk-web.e7a3fd8c.js                                                                                                                                   16:15:02
ios/App/App/public/chunk-web.e9907dfd.js       
android/app/build/intermediates/assets/debug/public/chunk-web.4FW2AnQR.js                                                                                                  16:15:02
android/app/build/intermediates/assets/debug/public/chunk-web.Bn4HHHKd.js                                                                                                  16:15:02
android/app/build/intermediates/assets/debug/public/chunk-web.CblH4nsc.js                                                                                                  16:15:02
android/app/build/intermediates/assets/debug/public/chunk-web.DDxlPr_5.js                                                                                                  16:15:02
android/app/build/intermediates/assets/debug/public/chunk-web.DFzIyWd8.js                                                                                                  16:15:02
android/app/build/intermediates/assets/debug/public/chunk-web.Dvdwtucr.js       

These are the Ionic build outputs from capacitor in the default locations; these are build artifacts, so I assume normally knip does not need to parse them at all (they should be ignored, since they are generated from the build scripts and code).

So this is a feature request to support Ionic (specifically the capacitor plugins for building mobile apps, @capacitor/android, @capacitor/ios) and ignoring their build artifacts (when @capacitor/android is present, the android directory, and when @capacitor/ios is present, the ios directory).

Repro

https://github.com/ptmkenny/ionic-react-conference-app/tree/knip

git clone git@github.com:ptmkenny/ionic-react-conference-app.git
cd ionic-react-conference-app
git checkout knip
npm install
npm run build
npx cap sync android; npx cap sync ios
npm run knip

Thanks

Thank you for this awesome project; it has helped me remove a lot of cruft that has built up over the past couple years!

webpro commented 2 weeks ago

Thanks @ptmkenny! Added to the central list.

ptmkenny commented 2 weeks ago

New ionic fixture/repro in which I attempted to minimize the dependencies: https://github.com/ptmkenny/ionic-knip

git clone git@github.com:ptmkenny/ionic-knip.git
cd ionic-knip
npm install
npm run build
npx cap sync android; npx cap sync ios
npm run knip

Output will be:

Unused dependencies (3)
@capacitor/android  package.json
@capacitor/core     package.json
@capacitor/ios      package.json

However, all three of these packages are actually used.

@capacitor/core is a dependency of @capacitor/android and of @capacitor/ios.

@capacitor/android is used to generate the build artifacts at android/.

@capacitor/ios is used to generate the build artifacts at ios/.

This repo includes a knip.json that sets the endpoint to the Ionic app (src/main.tsx). If you delete this knip.json and run knip with the defaults, the ios/ and android/ directories are incorrectly includes as "Unused files":

Unused files (42)
ios/App/App/public/cordova.js
ios/App/App/public/cordova_plugins.js
ios/App/App/public/assets/focus-visible-legacy-CdO5cX4I.js
ios/App/App/public/assets/focus-visible-supuXXMI.js
ios/App/App/public/assets/index-CfT0cAct.js
ios/App/App/public/assets/index-legacy-DTAAmUea.js
ios/App/App/public/assets/index9-ByFxGPRu.js
ios/App/App/public/assets/index9-legacy-CF3nKXXW.js
ios/App/App/public/assets/input-shims-82cFO9EE.js
ios/App/App/public/assets/input-shims-legacy-D9JDzCDq.js
ios/App/App/public/assets/ios.transition-C5dvlF3n.js
ios/App/App/public/assets/ios.transition-legacy-DG_clsmT.js
ios/App/App/public/assets/keyboard2-Yl815b2O.js
ios/App/App/public/assets/keyboard2-legacy-CgL1guoI.js
ios/App/App/public/assets/md.transition-B4vKbx_8.js
ios/App/App/public/assets/md.transition-legacy-Caj7FDWH.js
ios/App/App/public/assets/polyfills-legacy-B0HgiJVV.js
ios/App/App/public/assets/status-tap-ClKt3noB.js
ios/App/App/public/assets/status-tap-legacy-CNkiqfKF.js
ios/App/App/public/assets/swipe-back-Aw0EkCcf.js
ios/App/App/public/assets/swipe-back-legacy-BSHhuRhW.js
android/app/src/main/assets/public/cordova.js
android/app/src/main/assets/public/cordova_plugins.js
android/app/src/main/assets/public/assets/focus-visible-legacy-CdO5cX4I.js
android/app/src/main/assets/public/assets/focus-visible-supuXXMI.js
android/app/src/main/assets/public/assets/index-CfT0cAct.js
android/app/src/main/assets/public/assets/index-legacy-DTAAmUea.js
android/app/src/main/assets/public/assets/index9-ByFxGPRu.js
android/app/src/main/assets/public/assets/index9-legacy-CF3nKXXW.js
android/app/src/main/assets/public/assets/input-shims-82cFO9EE.js
android/app/src/main/assets/public/assets/input-shims-legacy-D9JDzCDq.js
android/app/src/main/assets/public/assets/ios.transition-C5dvlF3n.js
android/app/src/main/assets/public/assets/ios.transition-legacy-DG_clsmT.js
android/app/src/main/assets/public/assets/keyboard2-Yl815b2O.js
android/app/src/main/assets/public/assets/keyboard2-legacy-CgL1guoI.js
android/app/src/main/assets/public/assets/md.transition-B4vKbx_8.js
android/app/src/main/assets/public/assets/md.transition-legacy-Caj7FDWH.js
android/app/src/main/assets/public/assets/polyfills-legacy-B0HgiJVV.js
android/app/src/main/assets/public/assets/status-tap-ClKt3noB.js
android/app/src/main/assets/public/assets/status-tap-legacy-CNkiqfKF.js
android/app/src/main/assets/public/assets/swipe-back-Aw0EkCcf.js
android/app/src/main/assets/public/assets/swipe-back-legacy-BSHhuRhW.js
webpro commented 2 weeks ago

Looking at it, there's a little bit of "magic" going on here:

This all seems Capacitor area to me. Before we're going to try and automate this in Knip, I would like to understand whether the following configuration helps to get rid of false positives?

{
  "entry": ["src/main.tsx"]          // can be omitted since it's included by default
  "project": ["src/**/*.{ts,tsx}"]   // alternative: ["**/*.{ts,tsx}", "!ios/**", "!android/**"],
  "ignoreDependencies": ["@capacitor/android", "@capacitor/ios", "@capacitor/cli"]
}

Do you think there are further issues? And additional issues specific to Ionic?

ptmkenny commented 2 weeks ago

Thank you for looking into this.

I tried the config you suggested locally and it successfully eliminates the false positives.

That said, this pattern ("src/**/*.{ts,tsx}") might be problematic for every Ionic project because some old ones are still built in js/jsx and don't use TypeScript.

As for @capacitor/cli, it is referenced in capacitor.config.ts, which I thought knip already detects. (The first line of that file is import type { CapacitorConfig } from '@capacitor/cli';)

In fact, on my main project (not the repro I shared here), @capacitor/cli is not reported as unused, which is why I didn't mention it in my issue. So it's strange that it is reported as unused in the repro.

npx cap android is probably what uses @capacitor/android, but no references other than in android/capacitor.settings.gradle (Knip could check this) npx cap ios is probably what uses @capacitor/ios, but no references other than in ios/App/Podfile (Knip could check this)

That would be great!

webpro commented 2 weeks ago

That said, this pattern ("src/**/*.{ts,tsx}") might be problematic for every Ionic project because some old ones are still built in js/jsx and don't use TypeScript.

For now, this is something to control in entry and project yourself, so you could adjust to whatever extension(s) you'd need.

As for @capacitor/cli, it is referenced in capacitor.config.ts, which I thought knip already detects. (The first line of that file is import type { CapacitorConfig } from '@capacitor/cli';)

In fact, on my main project (not the repro I shared here), @capacitor/cli is not reported as unused, which is why I didn't mention it in my issue. So it's strange that it is reported as unused in the repro.

Knip would detect @capacitor/cli either in capacitor.config.ts indeed, let's skip over that for now. Also having "scripts": { "cap": "cap" } in package.json would probably do.

npx cap android is probably what uses @capacitor/android, but no references other than in android/capacitor.settings.gradle (Knip could check this) npx cap ios is probably what uses @capacitor/ios, but no references other than in ios/App/Podfile (Knip could check this)

That would be great!

Yes this I have now locally, just checking of those files exist. But just to be sure: looking at https://github.com/ptmkenny/ionic-knip/blob/main/android/capacitor.settings.gradle and https://github.com/ptmkenny/ionic-knip/blob/main/ios/App/Podfile, they reference more packages, such as @capacitor/haptics and @capacitor/status-bar - is this someone Knip should extract too, or are those packages also referenced in source code?

ptmkenny commented 2 weeks ago

they reference more packages, such as @capacitor/haptics and @capacitor/status-bar - is this someone Knip should extract too, or are those packages also referenced in source code?

The plugins such as haptics and status-bar will be referenced in the app's JS/TS source code if used; knip already detects all my plugins in my main app correctly.

@capacitor/ionic and @capacitor/android are special because they are used to build apps.

The other capacitor plugins are used to access native device features from app source code, so they can be handled the same as other npm packages.

webpro commented 2 weeks ago

:rocket: This issue has been resolved in v5.12.0. See Release 5.12.0 for release notes.

Using Knip in a commercial project? Please consider sponsoring me.