-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add getNativeSdkVersion
to get native sdk version
#790
Conversation
getNativeSdkVersion
to get native sdk versiongetNativeSdkVersion
to get native sdk version
d9a0af0
to
afd88ea
Compare
afd88ea
to
8310922
Compare
getNativeSdkVersion
to get native sdk versiongetNativeSdkVersion
to get native sdk version
src/functions.ts
Outdated
|
||
export async function getNativeSdkVersion(): Promise<string> { | ||
return Logger.traceSdkMethod(async () => { | ||
return await StripeTerminalSdk.getNativeSdkVersion(); |
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.
try / catch block?
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.
Receive the const will not cause exception but we can align the usage and prevent future break, will add in following commit.
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.
I found few different approaches, would like to hear from your opinion:
- Wrap the result with error like
confirmRefund()
does below to consistently return same type to caller:
export type ConfirmRefundResultType = {
refund?: Refund.Props;
error?: StripeError;
};
- Return string when success, return error object when fail like
getOfflineStatus()
does
export type OfflineStatus = {
sdk: OfflineStatusDetails;
reader?: OfflineStatusDetails;
};
- Return empty string or
NA
when error happen to keep using string type
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.
i'm peaceful with returning an empty string. i don't expect errors to happen here but juuuuuust in case we don't break
@@ -1069,6 +1070,10 @@ export function useStripeTerminal(props?: Props) { | |||
[_isInitialized, setLoading] | |||
); | |||
|
|||
const _getNativeSdkVersion = useCallback(async () => { | |||
return await getNativeSdkVersion(); |
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.
should we include the isInitialized
check here or is that redundant due to how the constants are retrieved from the SDK?
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.
Because it's just a const inside each sdk, we don't even need to call the terminal instance. So no matter whatever state it should return the same value.
@bric-stripe @ugochukwu-stripe could you take a look at the native changes? |
iOS bits lgtm |
Summary
Add
getNativeSdkVersion
to get native sdk versionMotivation
Add
getNativeSdkVersion
function to reveal the embedded Android/iOS sdk version.Reference: https://bugs.bbpos.com/browse/SDK-227
Testing
Documentation
Select one: