search

apache / cordova-ios

Apache Cordova iOS
https://cordova.apache.org/
Apache License 2.0
2.16k stars 986 forks source link

refactor!: Add CDVSettingsDictionary class #1458

Closed dpogue closed 3 months ago

dpogue commented 3 months ago

Platforms affected

iOS

Motivation and Context

This will replace the extension category on NSDictionary, which has always felt a little bit hacky and has caused issues in weird circumstances due to requiring special build flags to work properly.

Description

Add a new CDVSettingsDictionary class that is used for accessing preferences.

Yes, subclassing NSDictionary is generally considered a bad idea because the internal implementation details are weird and cause problems. However, in this case we're essentially wrapping a real NSMutableDictionary and proxying everything to it, so this should work the way we'd expect it to.

Testing

Added new unit test providing coverage for all the CDVSettingsDictionary methods.

Added a test file to check Swift behaviour as well, but adding it to the test suite project doesn't work (weird Xcode swift errors). I'd like to refactor the test project setup as well, but I'll deal with that after getting through the main CordovaLib project...

Checklist

codecov-commenter commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.31%. Comparing base (98cca7f) to head (a4b4e75).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1458 +/- ## ======================================= Coverage 78.31% 78.31% ======================================= Files 16 16 Lines 1826 1826 ======================================= Hits 1430 1430 Misses 396 396 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.