-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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(plugins): typed invoke arguments for mobile plugins #8076
Conversation
This reverts commit 49383c2.
seems like the jackson dependency costs 1MB on the AAB size, maybe we can optimize that or find a better package for this (though AFAIK there's no Java expert on the team 😂 ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Android sounds OK to me
@@ -167,7 +185,8 @@ abstract class Plugin(private val activity: Activity) { | |||
|
|||
// If call was made with a list of specific permission aliases to request, save them | |||
// to be requested | |||
val providedPerms: JSArray = invoke.getArray("permissions", JSArray()) | |||
// TODO val providedPerms: JSArray = invoke.getArray("permissions", JSArray()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this changed to TODO? should it be solved in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops missed this one, thanks for catching it @amrbashir
Java yikes, do these developers exist any more 😂? |
What kind of change does this PR introduce?
Does this PR introduce a breaking change?
Checklist
fix: remove a typo, closes #___, #___
)Other information
This PR changes the plugin command API to enforce typed arguments instead of relying on on-demand argument processing via
invoke.getString
,invoke.getInt
,invoke.getObject
etc.New API for Android:
New API for iOS:
Note that you can also resolve with an object instead of the generic
JSObject
class / dictionary.