Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Ionic / enhance the capacitor plugin #604

Closed
ptmkenny opened this issue Apr 27, 2024 · 7 comments
Closed

Add support for Ionic / enhance the capacitor plugin #604

ptmkenny opened this issue Apr 27, 2024 · 7 comments
Labels
feature request Feature request

Comments

@ptmkenny
Copy link
Contributor

ptmkenny commented Apr 27, 2024

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!

@ptmkenny ptmkenny added the feature request Feature request label Apr 27, 2024
@webpro
Copy link
Collaborator

webpro commented Apr 29, 2024

Thanks @ptmkenny! Added to the central list.

@webpro webpro closed this as completed Apr 29, 2024
@ptmkenny
Copy link
Contributor Author

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
Copy link
Collaborator

webpro commented Apr 30, 2024

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

  • npx cap is what uses @capacitor/cli, but there are no references in the codebase to this (not sure how Knip could check this)
  • 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)

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?

@webpro webpro reopened this Apr 30, 2024
@ptmkenny
Copy link
Contributor Author

ptmkenny commented May 1, 2024

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
Copy link
Collaborator

webpro commented May 2, 2024

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
Copy link
Contributor Author

ptmkenny commented May 2, 2024

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 webpro closed this as completed in 4af5c86 May 2, 2024
@webpro
Copy link
Collaborator

webpro commented May 2, 2024

🚀 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request
Projects
None yet
Development

No branches or pull requests

2 participants