-
Notifications
You must be signed in to change notification settings - Fork 2
feat(DataSource): add the transformError data-source function #24
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
Conversation
src/core/types/DataSource.ts
Outdated
@@ -6,11 +6,14 @@ export type DataSourceTag = string; | |||
declare const errorHintSymbol: unique symbol; | |||
declare const stateHintSymbol: unique symbol; | |||
|
|||
export type NonNullableUnknown = NonNullable<unknown>; |
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.
suggestion(blocking): Let's move it to utils.ts
src/core/types/DataSource.ts
Outdated
* When set, the `fetch` errors will be transformed into data without changing the state to error. | ||
* @returns NonNullable | ||
*/ | ||
transformError?: (reason: unknown) => TResponseErrorTransform; |
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.
suggestion(non-blocking): reason -> error
src/core/types/DataSource.ts
Outdated
export interface DataSource< | ||
TContext, | ||
TParams, | ||
TRequest, | ||
TResponse, | ||
TResponseErrorTransform extends NonNullableUnknown, |
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.
issue(blocking): Move it after TError
and add default like TResponseErrorTransform extends NonNullableUnknown = NonNullableUnknown
to avoid breaking 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.
Required
type parameters may not follow optional
type parameters, so I will move it to the end.
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 resolved an undefined
and null
return values of the transformError
, so the TErrorResponse
does not extend NonNullableUnknown
and can be anything at this point.
src/core/types/DataSource.ts
Outdated
export interface DataSource< | ||
TContext, | ||
TParams, | ||
TRequest, | ||
TResponse, | ||
TResponseErrorTransform extends NonNullableUnknown, |
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.
suggestion(non-blocking): TResponseErrorTransform -> TErrorResponse
src/core/types/DataSource.ts
Outdated
* When set, the `fetch` errors will be transformed into data without changing the state to error. | ||
* @returns NonNullable | ||
*/ | ||
transformError?: (reason: unknown) => TResponseErrorTransform; |
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.
question(blocking): Can we use TError
here instead of unknown
?
infer TData, | ||
infer _TError, | ||
infer _TOptions, | ||
infer _TState, | ||
infer _TFetchContext | ||
> | ||
? ActualData<TData, TResponse> | ||
? ActualData<TResponse, TResponseErrorTransform, TData> | ||
: never; | ||
|
||
export type DataSourceError<TDataSource> = |
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.
suggestion(blocking): Add DataSourceErrorResponse
for inferring like DataSourceError
src/core/types/DataSource.ts
Outdated
unknown extends TResponseErrorTransform ? TResponse : TResponse | TResponseErrorTransform; | ||
|
||
export type ActualData<TResponse, TResponseErrorTransform, TData> = unknown extends TData | ||
? unknown extends TResponseErrorTransform |
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.
suggestion(blocking): Looks like you can use ActualResponse
here
QueryDataSourceContext, | ||
TParams, | ||
TRequest, | ||
TResponse, | ||
TResponseErrorTransform, | ||
TData, | ||
TError, | ||
InfiniteQueryObserverExtendedOptions< | ||
TResponse, |
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.
issue(blocking): You have to wrap TResponse
using ActualResponse
in the TOptions
and TState
types
src/react-query/impl/plain/types.ts
Outdated
TError, | ||
TErrorResponse, | ||
QueryObserverExtendedOptions< | ||
TResponse, |
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.
issue(blocking): Wrap it using ActualResponse
TError, | ||
TErrorResponse, | ||
InfiniteQueryObserverExtendedOptions< | ||
TResponse, |
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.
issue(blocking): Wrap it using ActualResponse
} catch (error) { | ||
if (!transformError) throw error; | ||
|
||
return formatNullableValue(transformError(error)); |
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.
suggestion(blocking): Use formatNullableValue
for fetch result too
? (data) => ({...data, pages: data.pages.map(transformResponse)}) | ||
: undefined, | ||
select: | ||
transformResponse || transformError |
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.
suggestion(blocking): Use innertTransform
without condition
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.
Oh, missed it =\ Thanks!
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.
❤️
refactor: review issues feat: resolve an undefined and null error response values docs(skipContext): add the JSDoc fix(react-query/impl): review issues
a286b3f
to
834c3d7
Compare
When the
transformError
is set, thefetch
errors will be transformed into data without changing the state to error.