Skip to content

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

Merged
merged 1 commit into from
Jul 8, 2025

Conversation

Dji75
Copy link
Contributor

@Dji75 Dji75 commented Jun 30, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #212

What is the new behavior?

A dedicated InjectionToken has been created in order to provide Query 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?

  • Yes
  • No

Other information

N/A

Copy link

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@luii
Copy link
Contributor

luii commented Jul 1, 2025

LGTM

@NetanelBasal at that point wouldn't it be wise to also include a provider for Mutation, IsMutating, IsFetching and InfiniteQuery? Maybe we could also bundle them under a umbrella provide function with the mentioned fields marked as optional.

@luii
Copy link
Contributor

luii commented Jul 1, 2025

@Dji75 Would you please add some documentation for that in the README.md

@NetanelBasal
Copy link
Member

@luii makes sense

@Dji75
Copy link
Contributor Author

Dji75 commented Jul 1, 2025

@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 ?

@luii
Copy link
Contributor

luii commented Jul 2, 2025

What is the purpose of this class @Dji75 ?

If you have the time could you also implement, what you initially committed here, also for Mutation, IsMutating, IsFetching and InfiniteQuery ? Maybe we can implement that under just one provider function, something like that

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
@NetanelBasal any suggestions about these implementation details?

@NetanelBasal
Copy link
Member

LGTM

@Dji75
Copy link
Contributor Author

Dji75 commented Jul 3, 2025

I would purpose to merge all provideXXX functions into single one provideQueryConfig.

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.

@Dji75
Copy link
Contributor Author

Dji75 commented Jul 4, 2025

@luii after your question on this, I think you're right, there is no real interest, so I rolled back that point ;)

Now, regarding query config, I'll proceed for all properties except probably query options: it looks too tricky for me to refactor the whole library :)

@Dji75 Dji75 force-pushed the add-query-token-provider branch 2 times, most recently from f0a9663 to 3a45ee5 Compare July 4, 2025 10:35
@Dji75
Copy link
Contributor Author

Dji75 commented Jul 4, 2025

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

@Dji75
Copy link
Contributor Author

Dji75 commented Jul 7, 2025

I don't think I'll go any further on this.

@luii any feedback ? :)

TQueryFnData,
TQueryKey
>,
) => any);
Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why any?

Copy link
Contributor Author

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

Copy link
Member

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?

Copy link
Contributor Author

@Dji75 Dji75 Jul 8, 2025

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

@Dji75 Dji75 force-pushed the add-query-token-provider branch from 3a45ee5 to 39c1c85 Compare July 8, 2025 09:01
@Dji75
Copy link
Contributor Author

Dji75 commented Jul 8, 2025

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 :)

@NetanelBasal NetanelBasal merged commit fabfdf8 into ngneat:main Jul 8, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants