-
Notifications
You must be signed in to change notification settings - Fork 167
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
chore: refactor frontend API client methods & add doc comments #1864
base: main
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1864 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 5
Lines 1242 1242
=========================================
Hits 1242 1242 Continue to review full report at Codecov.
|
Deploying with Cloudflare Pages
|
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.
LGTM just one console.log leftover
This refactors a few things related to the frontend login flow that I found awkward while reading through the code. I'm definitely open to suggestions / change requests if anyone thinks I've made things worse :)
The major changes:
getToken
fromapi.js
has been renamed togetMagicUserToken
, since there's a billion things called "token" in this codebase. Since this method only ever returns magic.link user id tokens, I gave it a more specific name, and also moved it tomagic.js
, since it's magic.link specific. Also renamed some of the global state vars related to saving the token to be more specific (e.g.LIFESPAN
=>MAGIC_USER_TOKEN_LIFESPAN_SEC
).isLoggedIn
got renamed togetMagicUserMetadata
, since that's what it actually does, and a function calledisSomething
that doesn't return a boolean is surprising.logoutMagicSession
function, so the logout component doesn't need direct access to the magic SDK instancegetMagic
accessor internal tomagic.js
getStorageClient
accessor toapi.js
that returns an authenticatedNFTStorage
instance (using the key fromgetMagicUserToken
).API
constant and thegetToken
functions, which were only used to new up client instances in UI codeI also refactored almost all of the functions in
api.js
to remove copy/paste boilerplatefetchRoute
helper that appends the route path to the API base url, sets theContent-Type
to json, and unpacks the JSON response bodyfetchAuthenticated
also sets the auth header to the value ofgetMagicUserToken
@redaphid would you mind checking this out and let me know if there's anything surprising?