-
Notifications
You must be signed in to change notification settings - Fork 247
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 isDeviceSupported method #1359
Add isDeviceSupported method #1359
Conversation
🦋 Changeset detectedLatest commit: 731d95c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to 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.
Looks good overall, some questions inline
// To check if current device is supported | ||
export const isDeviceSupported = async (): Promise<boolean> => { | ||
try { | ||
await createKey('test-device'); |
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.
Is there any way to check for support without side effects?
I see that the current implementation uses the response from createKey
, so it's probably not a straightforward thing. But if you are aware of a less intrusive check it would be helpful to document in a comment or follow-up issue.
If this is the necessary approach however, would it be better to use a different key each time? I.e. is the outcome the same if the test-device
key doesn't exist vs was previously created?
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.
After spending some time researching how to do this we found that the only way to do it was by using the response from createKey
. The outcome should be the same after each call.
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.
Thank you for the clarification
@gtsonevv Thank you for your contribution! Your pull request is now a part of the Race of Sloths! Current status: executed
The average score is 3 @gtsonevv check out your results on the Race of Sloths Leaderboard! and in the profile What is the Race of SlothsRace of Sloths is a friendly competition where you can participate in challenges and compete with other open-source contributors within your normal workflow For contributors:
For maintainers:
Feel free to check our website for additional details! Bot commands
|
@race-of-sloths score 3 |
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.
🚢
🌟 Score recorded!@andy-haynes, thank you for scoring this pull request in the Race of Sloths! |
✅ PR is finalized!Your contribution is much appreciated with a final score of 3! Another weekly streak completed, well done @gtsonevv! To keep your weekly streak and get another bonus make pull request next week! Looking forward to see you in race-of-sloths |
Pre-flight checklist
pnpm changeset
to create achangeset
JSON document appropriate for this change.Motivation
Test Plan
Related issues/PRs