dpa99c / cordova-custom-config

Cordova/Phonegap plugin to update platform configuration files based on preferences and config-file data defined in config.xml
318 stars 84 forks source link

using custom-config-file removes empty string objects in .plist #136

Closed chrismowbraylit closed 6 years ago

chrismowbraylit commented 6 years ago

Running a standard config update:

<custom-config-file platform="ios" target="*-Info.plist" parent="MyCustomnode" mode="replace"> 
        <string>test</string> 
 </custom-config-file>

This removes any empty string objects from the .plist.

This...

<key>NSMainNibFile</key>
<string/>
<key>NSMainNibFile~ipad</key>
<string/>
<key>NSLocationWhenInUseUsageDescription</key>
<string/>

becomes this...

<key>NSMainNibFile</key>
<key>NSMainNibFile~ipad</key>
<key>NSLocationWhenInUseUsageDescription</key>

The .plist in now in an unreadable format and cant be used.

dpa99c commented 6 years ago

I'm not able to reproduce this behaviour (see console output below).

This plugin uses the plist module to read and write the plist contents, so any issue in formatting the plist file is likely to be caused by that.

console output ``` $ sw_vers ProductName: Mac OS X ProductVersion: 10.13.2 BuildVersion: 17C88 $ node -v v6.11.0 $ npm -v 3.10.10 $ cordova -v 7.1.0 $ git clone https://github.com/dpa99c/cordova-custom-config-example && cd cordova-custom-config-example Cloning into 'cordova-custom-config-example'... remote: Counting objects: 612, done. remote: Compressing objects: 100% (8/8), done. remote: Total 612 (delta 4), reused 5 (delta 2), pack-reused 602 Receiving objects: 100% (612/612), 3.86 MiB | 669.00 KiB/s, done. Resolving deltas: 100% (256/256), done. $ cordova plugin add cordova-custom-config && cordova plugin rm cordova-custom-config Adding cordova-custom-config to package.json Saved plugin info for "cordova-custom-config" to config.xml Removing "cordova-custom-config" Removing plugin cordova-custom-config from config.xml file... Removing cordova-custom-config from package.json $ cordova platform add ios@latest Using cordova-fetch for cordova-ios@latest Adding ios project... Creating Cordova project for the iOS platform: Path: platforms/ios Package: uk.co.workingedge.phonegap.plugins.example.customconfig Name: Custom config plugin example iOS project created with cordova-ios@4.5.4 Discovered plugin "cordova-plugin-whitelist" in config.xml. Adding it to the project Installing "cordova-plugin-whitelist" for ios Adding cordova-plugin-whitelist to package.json Saved plugin info for "cordova-plugin-whitelist" to config.xml --save flag or autosave detected Saving ios@latest into config.xml file ... $ more platforms/ios/Custom\ config\ plugin\ example/Custom\ config\ plugin\ example-Info.plist CFBundleDevelopmentRegion English CFBundleDisplayName Custom config plugin example CFBundleExecutable ${EXECUTABLE_NAME} CFBundleIcons CFBundleIcons~ipad CFBundleIdentifier uk.co.workingedge.phonegap.plugins.example.customconfig CFBundleInfoDictionaryVersion 6.0 CFBundleName ${PRODUCT_NAME} CFBundlePackageType APPL CFBundleShortVersionString 5.0.0 CFBundleSignature ???? CFBundleVersion 5.0.0 LSRequiresIPhoneOS NSMainNibFile NSMainNibFile~ipad UISupportedInterfaceOrientations UIInterfaceOrientationPortrait UIInterfaceOrientationLandscapeLeft UIInterfaceOrientationLandscapeRight UISupportedInterfaceOrientations~ipad UIInterfaceOrientationPortrait UIInterfaceOrientationLandscapeLeft UIInterfaceOrientationPortraitUpsideDown UIInterfaceOrientationLandscapeRight UIRequiresFullScreen NSAppTransportSecurity NSAllowsArbitraryLoads $ cordova plugin add cordova-custom-config@latest Installing "cordova-custom-config" for ios Adding cordova-custom-config to package.json Saved plugin info for "cordova-custom-config" to config.xml $ cordova prepare ios cordova-custom-config: Skipping auto-restore of config file backup(s) cordova-custom-config: Applied custom config from config.xml to /Users/dave/Documents/projects/@scratch/cordova-custom-config-example/platforms/ios/Custom config plugin example.xcodeproj/project.pbxproj cordova-custom-config: Applied custom config from config.xml to /Users/dave/Documents/projects/@scratch/cordova-custom-config-example/platforms/ios/cordova/build.xcconfig cordova-custom-config: Applied custom config from config.xml to /Users/dave/Documents/projects/@scratch/cordova-custom-config-example/platforms/ios/cordova/build-extras.xcconfig cordova-custom-config: Applied custom config from config.xml to /Users/dave/Documents/projects/@scratch/cordova-custom-config-example/platforms/ios/cordova/build-debug.xcconfig cordova-custom-config: Applied custom config from config.xml to /Users/dave/Documents/projects/@scratch/cordova-custom-config-example/platforms/ios/cordova/build-release.xcconfig cordova-custom-config: Applied custom config from config.xml to /Users/dave/Documents/projects/@scratch/cordova-custom-config-example/platforms/ios/Custom config plugin example/Custom config plugin example-Info.plist $ more platforms/ios/Custom\ config\ plugin\ example/Custom\ config\ plugin\ example-Info.plist CFBundleDevelopmentRegion English CFBundleDisplayName Custom config plugin example CFBundleExecutable ${EXECUTABLE_NAME} CFBundleIcons CFBundleIcons~ipad CFBundleIdentifier uk.co.workingedge.phonegap.plugins.example.customconfig CFBundleInfoDictionaryVersion 6.0 CFBundleName ${PRODUCT_NAME} CFBundlePackageType APPL CFBundleShortVersionString 5.0.0 CFBundleSignature ???? CFBundleVersion 5.0.0 LSRequiresIPhoneOS NSMainNibFile NSMainNibFile~ipad UISupportedInterfaceOrientations UIInterfaceOrientationPortrait UIInterfaceOrientationLandscapeLeft UIInterfaceOrientationLandscapeRight UIInterfaceOrientationPortraitUpsideDown UISupportedInterfaceOrientations~ipad UIInterfaceOrientationPortrait UIInterfaceOrientationPortraitUpsideDown UIRequiresFullScreen NSAppTransportSecurity NSAllowsArbitraryLoads LSApplicationQueriesSchemes myapp myapp2 myapp3 UIBackgroundModes location NSLocationAlwaysUsageDescription This app requires constant access to your location in order to track your position, even when the screen is off. NSLocationWhenInUseUsageDescription This app will now only track your location when the screen is on and the app is displayed. ```
chrismowbraylit commented 6 years ago

I cannot replicate the issue with the above setup. Although I cannot see any other plugin changing the plist in my repo.

dpa99c commented 6 years ago

Aha! I think I've found the cause of it.

This issue originally cropped up in #90. The cause was a bug in plist@2 (outlined in plist.js#79) and the solution was to keep the version pinned at plist@1.2.0. However, going through the commit history, it seems I inadvertently accepted PR #119 and forgot about #90. That PR bumps the versions to plist@2.1.0 and xcode@0.9.3 (which indirectly depends on plist@2.1.0 via simple-plist@0.2.1). The PR had good intentions, but has re-introduced this bug.

In the meantime, the ownership of xcode has moved from alunny to apache/cordova. The new cordova-node-xcode still depends on simple-plist@0.2.1 and hence on plist@2.1.0.

However, I don't think the problem encountered with this plugin is the indirect use of plist@2.1.0 via xcode, I think it's the now-explicit dependency thanks to PR #119.

plist.js#79 is still open and it seems no more recent versions have been released which fix that issue, however there's an open PR to fix it. I'm thinking that if I directly reference the fork which contains the fix in the package.json of this plugin, then not only will this plugin make direct use of that fixed version of plist@2.1.0, but the version is the same as that depended on by xcode hence it should hopefully also use the fixed version from the fork, rather than the buggy master version from npm.

chrismowbraylit commented 6 years ago

Hey @dpa99c thanks for the quick response. Are you going to push this into a release?

dpa99c commented 6 years ago

@chrismowbray I've just published v5.0.1 of this plugin, which references the forked plist dependency.

Give it a go in your project and let me know if it works OK. You may need to first remove node_modules to make sure it installs fresh dependencies.

chrismowbraylit commented 6 years ago

@dpa99c im not seeing the 5.0.1 in https://github.com/dpa99c/cordova-custom-config/releases or npm. Maybe it takes a while

dpa99c commented 6 years ago

https://www.npmjs.com/package/cordova-custom-config

image

chrismowbraylit commented 6 years ago

Yep and now I see it! ill post back when I test it out

chrismowbraylit commented 6 years ago

@dpa99c All looks good now. Thanks for the quick response. This issue can be closed