-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix!: Wrap Axios type and export new type #210
base: main
Are you sure you want to change the base?
Conversation
http/index.ts
Outdated
@@ -16,6 +16,8 @@ import { HubSpotHttpError } from '../models/HubSpotHttpError'; | |||
|
|||
const i18nKey = 'http.index'; | |||
|
|||
export type HubSpotResponse<T = unknown> = AxiosPromise<T>; |
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 is the primary change, all of the others are reactionary.
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.
Looked over the changes, and the type change makes sense to me.
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.
Just one minor thing, LGTM otherwise
@@ -16,6 +16,8 @@ import { HubSpotHttpError } from '../models/HubSpotHttpError'; | |||
|
|||
const i18nKey = 'http.index'; | |||
|
|||
export type HubSpotPromise<T = unknown> = AxiosPromise<T>; |
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.
LDL currently defines all types within the types
dir, so this should live in types/http
Description and Context
Wraps the
AxiosPromise
in aHubSpotPromise
type and exports it so that integrators don't need to useReturnType
annotations to have access to the type.