-
Notifications
You must be signed in to change notification settings - Fork 102
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: upgrade api layer #1910
base: main
Are you sure you want to change the base?
feat: upgrade api layer #1910
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
25e066e
to
7be35ba
Compare
7be35ba
to
6025c52
Compare
6025c52
to
8d9e372
Compare
8d9e372
to
8f5a43f
Compare
8f5a43f
to
9d76acd
Compare
9d76acd
to
d307216
Compare
d307216
to
7ae81b7
Compare
7ae81b7
to
7e16fa5
Compare
Codecov ReportAttention: Patch coverage is 📢 Thoughts on this report? Let us know! |
7e16fa5
to
58b12b5
Compare
58b12b5
to
d001bb7
Compare
4cee1df
to
0a8446c
Compare
0a8446c
to
c2659ae
Compare
c2659ae
to
6323121
Compare
6323121
to
1e9d13b
Compare
apiUrl: Endpoint, | ||
apiParams?: ApiParams<Endpoint> | ||
): Promise<OperationResponse[Endpoint]> { | ||
const { error, data } = await apiClient.GET(apiUrl, apiParams as any); |
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.
This function acts as a wrapper around the API client, it enforce type safety for the endpoint, parameters, and response, as well as providing centralized error handling. I had to use any when passing apiParams because the types provided by client library are not fully generic and rely on rigid definitions that cannot be dynamically inferred. Open to suggestions here.
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 think this works. We'll do with the missing types for now, unless it hinders us in some signifcant way. Would be interesting to see if we could have done this with middleware, but I think that is a separate investigation. Thanks for making these changes!
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.
One small thing about removing useBlockByHeight hook or w/e it was. Other than that LGTM
1e9d13b
to
a96fd05
Compare
What type of PR is this? (check all applicable)
Description
@stacks/blockchain-api-client no longer exports API call wrappers, we need to upgrade to latest stacks API JS client replace all API calls across Explorer.
https://github.com/hirosystems/stacks-blockchain-api/blob/HEAD/client/MIGRATION.md
Issue ticket number and link
closes #1896
Checklist:
Screenshots (if appropriate):
N/A