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

feat(iOS): Obj-C convenience accessors on CAPPluginCall #4309

Merged
merged 5 commits into from
Mar 9, 2021

Conversation

ikeith
Copy link
Contributor

@ikeith ikeith commented Mar 9, 2021

This branch revisits the Obj-C versions of the CAPPluginCall convenience accessors, deprecates the redundant methods on CAPPlugin, adds test coverage, and fixes a bug with dates.

  • Adds a getNumber method for NSNumber values.
  • Switches the redundant CAPPlugin accessors to use the call's implementation.
  • Marks the redundant CAPPlugin accessors as deprecated.
  • Fixes the incorrect handling of NSDate objects in getDate.
  • Adds unit tests to verify behavior.

@ikeith ikeith requested a review from jcesarmobile March 9, 2021 00:21
Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the bridged types have a getBool like the swift plugin call has?
Having to use NSNumber every time you want a boolean might make users use the deprecated getBool from CAPPlugin class instead as it's easier to get despite it's deprecated.

@ikeith
Copy link
Contributor Author

ikeith commented Mar 9, 2021

@jcesarmobile
Swift cares about types more which is why it has more accessors. But we can add a getBool, although I'm not sure how helpful it is since you can use boolValue and NSNumber:
[call getBool:@"foo" defaultValue:TRUE];
vs
[[call getNumber:@"foo" defaultValue:@TRUE] boolValue];

@jcesarmobile
Copy link
Member

If they are using a getBool and they are told that that getBool is deprecated and should use the ones from pluginCall, but there is no getBool there it's not trivial to know that they can use getNumber and cast to bool

@ikeith
Copy link
Contributor Author

ikeith commented Mar 9, 2021

That's a fair point. It probably makes sense to ease the migration from the deprecated methods.

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

Successfully merging this pull request may close these issues.

2 participants