-
-
Notifications
You must be signed in to change notification settings - Fork 41
feat(query): use provideQuery to provide custom Query #213
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
|
LGTM @NetanelBasal at that point wouldn't it be wise to also include a provider for |
@Dji75 Would you please add some documentation for that in the |
@luii makes sense |
@NetanelBasal @luii Query and some interfaces are not exported yet but I think it make sense in order to extend it: import { DefinedInitialDataOptions, provideQuery, Query, Result } from '@ngneat/query';
import { DefaultError, DefinedQueryObserverResult, QueryKey, QueryObserverResult } from '@tanstack/query-core';
export class CustomQuery extends Query {
override use<TQueryFnData = unknown, TError = DefaultError, TData = TQueryFnData, TQueryKey extends QueryKey = QueryKey>(
options: DefinedInitialDataOptions<TQueryFnData, TError, TData, TQueryKey>
): Result<DefinedQueryObserverResult<TData, TError>> {
// do your own specific stuff
return super.use(options);
}
}
provideQuery(() => new CustomQuery()) // or provideQuery(new CustomQuery()) What do you think ? |
What is the purpose of this class @Dji75 ? If you have the time could you also implement, what you initially committed here, also for provideQueryConfig({
query: queryMock,
mutation: mutationMock,
isMutating: isMutatingMock,
isFetching: isFetchingMock,
infiniteQuery: infiniteQueryMock
}); @Dji75 All of these properties would be optional of course and use a default factory, like you already did in you commit |
LGTM |
I would purpose to merge all If you don't want to a a breaking change, I can keep the other ones and mark them as deprecated (or do not merge them at all) provideQueryConfig({
client: clientMock,
clientOptions: clientOptionsMock,
query: queryMock,
queryOptions: queryOptionsMock,
mutation: mutationMock,
isMutating: isMutatingMock,
isFetching: isFetchingMock,
infiniteQuery: infiniteQueryMock
}); I guess I have to create dedicated interfaces for Query class (containing use function) and Query Options (alias for DefinedInitialDataOptions | UndefinedInitialDataOptions) I didn't checked if it will be easy at all, but let me know if you think it is a good idea :) P.S : I will probably do it next week. |
f0a9663
to
3a45ee5
Compare
I did it for Query, Mutation, IsMutating, IsFetching and InfiniteQuery objects for the moment. I updated documentation accordingly, let me know if you find it clear |
I don't think I'll go any further on this. @luii any feedback ? :) |
TQueryFnData, | ||
TQueryKey | ||
>, | ||
) => 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.
Why do you use any
here? Is there a reason for it?
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.
Otherwise it looks good, if the test are working and if @netbasal has no objections we can merge it
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.
Why 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.
any
because of createBaseQuery
return type
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.
the types are still works as expected?
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.
types are stricly identical as previously, I've just exported them in dedicated interfaces
3a45ee5
to
39c1c85
Compare
I introduced an incorrect import in query.spec.ts which introduced a lint issus (from @ngneat/query) instead of local file), I had to push force the fix :) |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #212
What is the new behavior?
A dedicated
InjectionToken
has been created in order to provideQuery
class instead using@Injectable({ providedIn: 'root' })
at class level.Then, the function
provideQuery
have been also created in order to be able to override it.Does this PR introduce a breaking change?
Other information
N/A