cordova-lib
cordova-lib copied to clipboard
[cordova-cli #578] plugin/remove.js: don't stomp opts.cli_variables in removePluginFromPlatform()
Platforms affected
CLI - adding and removing plugins with differing required variables per platform (e.g. cordova-plugin-googleplus )
Motivation and Context
resolves apache/cordova-cli#578 :
cordova plugin rm plugin-package --variable "IOS_ONLY_REQUIRED_VARIABLE=variable-value" will fail if the project includes e.g. both android and ios platforms.
Description
In removePluginFromPlatform(), at cordova/plugin/remove.js:105, opts.cli_variables is destructively reassigned to the result of mergeVariables(). As mergeVariables() selects only keys specified for a specific platform, this can result in CLI-specified values being lost before the platform that requires them is processed.
In the change, instead call uninstallPlatform() with a copy of opts that includes the platform-specific mergeVariables() result.
Testing
- Ran existing tests (
npm run test) - verified the change fixes my local use case by
npm linking
Checklist
- [x] I've run the tests to see all new and existing tests pass
- [ ] I added automated test coverage as appropriate for this change
I'm not familiar enough with how to create a reproduction case in the test library for this project.
- [x] Commit is prefixed with
(platform)if this change only applies to one platform (e.g.(android)) - [x] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
- [x] I've updated the documentation if necessary
No doc update should be required
Thank you for the PR.
We generally do not make back ports releases. If applicable, the PR should target the master branch so that when merged it will be included in our next release. I'm not sure if GitHub allows you to re-target a PR, it may need to be closed and recreated.
Will do! I've been working on the released version 11 and I wasn't sure whether to target the patch branch or master. Thanks.
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 88.36%. Comparing base (
7f8b2d0) to head (2a0993e). Report is 29 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #913 +/- ##
=======================================
Coverage 88.36% 88.36%
=======================================
Files 46 46
Lines 2123 2123
=======================================
Hits 1876 1876
Misses 247 247
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.