refactor: replace simple-plist with a typescript safe version#84
refactor: replace simple-plist with a typescript safe version#84
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR reduces the public API surface by removing many Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
There was a problem hiding this comment.
Pull request overview
This PR refactors the codebase to replace the simple-plist library with @webnativellc/simple-plist, improve encapsulation by making several interfaces and functions internal, and clean up unused code. The primary motivation appears to be improving type safety (evidenced by the skipLibCheck change) while reducing the public API surface.
Changes:
- Replaces
simple-plistwith@webnativellc/simple-plistfor iOS property list file handling - Converts many exported interfaces and functions to internal scope across multiple modules
- Removes unused/dead code including functions like
excludeIgnoredTips,getAndroidManifestIntent,checkAndroidManifest, andwarnIfNotUsing - Updates TypeScript configuration to enable stricter library type checking
- Reorders VS Code launch configurations for better developer experience
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds @webnativellc/simple-plist and @types/semver dependencies, removes simple-plist |
| package-lock.json | Updates lockfile with new plist library and updated dependency versions |
| src/native-project-ios.ts | Switches to new plist library but incompletely - missing writeFileSync import |
| src/xcode.ts | Updates plist import and makes iosFolder function internal |
| tsconfig.json | Disables skipLibCheck to enable stricter type checking |
| src/utilities.ts | Makes passesRemoteFilter function internal |
| src/splash-icon.ts | Makes AssetType enum internal |
| src/rules-capacitor-plugins.ts | Makes interface and migration function internal |
| src/npm-info.ts | Makes getNpmToken function internal |
| src/monorepo.ts | Makes MonoFolder interface internal |
| src/monorepo-nx.ts | Removes unused NXWorkspace interface |
| src/live-reload.ts | Makes setupServerCertificate function internal |
| src/ignore.ts | Removes unused excludeIgnoredTips function |
| src/error-handler.ts | Makes extractErrors function internal |
| src/capacitor-migrate.ts | Makes MinPlugin and AndroidStudioInfo interfaces internal |
| src/capacitor-configure.ts | Removes duplicate getCapacitorConfigWebDir function |
| src/capacitor-build.ts | Makes KeyStoreSettings interface internal |
| src/audit.ts | Makes SecurityVulnerability interface internal |
| src/angular-generate.ts | Makes angularGenerate function internal |
| src/android-debug-models.ts | Makes WebViewPage interface internal |
| src/analyzer.ts | Removes unused functions, adds type annotations to exported function |
| src/analyze-size.ts | Makes SizeResults and SizeResult interfaces internal |
| .vscode/launch.json | Reorders debug configurations |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/analyzer.ts (1)
246-257: Return type is too narrow (can return undefined).This function still returns
undefinedwhencordovaConfigis missing or when the preference meets the minimum, but the signature now promisesTip. This is misleading and can break builds understrictNullChecks.🔧 Proposed fix
-export function checkCordovaAndroidPreferenceMinimum(preference: string, minVersion: string): Tip { +export function checkCordovaAndroidPreferenceMinimum( + preference: string, + minVersion: string, +): Tip | undefined {
🤖 Fix all issues with AI agents
In `@src/native-project-ios.ts`:
- Around line 227-231: The current setDisplayName method reads the Info.plist
via readFileSync but writes the modified object back with fs.writeFileSync,
which will serialize to "[object Object]"; change the write to use the plist
serializer from `@webnativellc/simple-plist` (the library you used to parse it) so
the plist is correctly serialized—update setDisplayName to call the plist
library's write method (or its writeFileSync) instead of fs.writeFileSync and
ensure the import references match (keep references to setDisplayName,
this._infoPlistPath, and the readFileSync call so you modify the correct
function).
…ion to Tip | undefined
This pull request primarily refactors the codebase to reduce the use of exported interfaces and functions, making many previously exported items internal to their modules. It also updates the iOS project’s use of property list (plist) file handling by switching to a different plist library, and cleans up some unused or redundant code. There are also minor dependency updates and configuration tweaks.
Refactoring: Restricting Scope of Interfaces and Functions
src/analyze-size.ts,src/android-debug-models.ts,src/audit.ts,src/capacitor-migrate.ts,src/monorepo-nx.ts,src/monorepo.ts,src/rules-capacitor-plugins.ts,src/splash-icon.ts, and others. [1] [2] [3] [4] [5] [6] [7] [8] [9]Dependency and Library Updates
simple-plistlibrary with@webnativellc/simple-plistfor reading and writing plist files insrc/native-project-ios.ts, and updates the relevant usage accordingly. Also updatespackage.jsondependencies to reflect this change and adds@types/semver. [1] [2] [3] [4] [5] [6]Code and Configuration Cleanup
getAndroidManifestIntent,checkAndroidManifest,warnIfNotUsing, andexcludeIgnoredTips. [1] [2] [3] [4] [5]angularGenerate,extractErrors,setupServerCertificate,getNpmToken,passesRemoteFilter, andmigrateCapacitorPlugin. [1] [2] [3] [4] [5] [6]Development and Build Configuration
.vscode/launch.jsonto reorder and re-add the "Run vitest" configuration, making it easier to run tests from VS Code. [1] [2]These changes collectively improve code maintainability, reduce the risk of unintended usage of internal APIs, and modernize dependencies.
Summary by CodeRabbit
Chores
Refactor
Other
✏️ Tip: You can customize this high-level summary in your review settings.