Skip to content
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

refactor(angular-query): change query functions to accept an options object instead of an injector #8776

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export default class LazyLoadDevtoolsPanelExampleComponent {
this.devtools = import(
'@tanstack/angular-query-devtools-experimental'
).then(({ injectDevtoolsPanel }) =>
injectDevtoolsPanel(this.devToolsOptions, this.injector),
injectDevtoolsPanel(this.devToolsOptions, { injector: this.injector }),
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
onlineManager,
} from '@tanstack/angular-query-experimental'
import { isPlatformBrowser } from '@angular/common'
import type { WithOptionalInjector } from '@tanstack/angular-query-experimental'
import type { ElementRef } from '@angular/core'
import type { DevtoolsErrorType } from '@tanstack/query-devtools'

Expand All @@ -26,19 +27,19 @@ import type { DevtoolsErrorType } from '@tanstack/query-devtools'
*
* Consider `withDevtools` instead if you don't need this.
* @param optionsFn - A function that returns devtools panel options.
* @param injector - The Angular injector to use.
* @param options - Additional configuration
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's think of a good name for this parameter - it's confusing if there's an optionsFn parameter and an options parameter.

Copy link
Author

Choose a reason for hiding this comment

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

I called it options because the parameter is also called options in Angular. I also didn't like having a param called optionsFn and options. Would you change it everywhere or just here? What do you think about staticOptions or something in that direction?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're right. Maybe we can follow effect from Angular's lead:

export function effect(
  effectFn: (onCleanup: EffectCleanupRegisterFn) => void,
  options?: CreateEffectOptions,

So first parameter e.g. named injectQueryFn, second options. What do you think?

Copy link
Author

Choose a reason for hiding this comment

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

Sounds good to me!

* @returns DevtoolsPanelRef
* @see https://tanstack.com/query/v5/docs/framework/angular/devtools
*/
export function injectDevtoolsPanel(
optionsFn: () => DevtoolsPanelOptions,
injector?: Injector,
options?: WithOptionalInjector,
): DevtoolsPanelRef {
!injector && assertInInjectionContext(injectDevtoolsPanel)
const currentInjector = injector ?? inject(Injector)
!options?.injector && assertInInjectionContext(injectDevtoolsPanel)
const currentInjector = options?.injector ?? inject(Injector)

return runInInjectionContext(currentInjector, () => {
const options = computed(optionsFn)
const queryOptions = computed(optionsFn)
let devtools: TanstackQueryDevtoolsPanel | null = null

const isBrowser = isPlatformBrowser(inject(PLATFORM_ID))
Expand All @@ -64,7 +65,7 @@ export function injectDevtoolsPanel(
shadowDOMTarget,
onClose,
hostElement,
} = options()
} = queryOptions()

untracked(() => {
if (!client) throw new Error('No QueryClient found')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ describe('injectInfiniteQuery', () => {
initialPageParam: 0,
getNextPageParam: () => 12,
}),
TestBed.inject(Injector),
{
injector: TestBed.inject(Injector),
},
)

expect(query.status()).toBe('pending')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ describe('injectIsFetching', () => {

test('can be used outside injection context when passing an injector', () => {
expect(
injectIsFetching(undefined, TestBed.inject(Injector)),
injectIsFetching(undefined, {
injector: TestBed.inject(Injector),
}),
).not.toThrow()
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ describe('injectIsMutating', () => {

test('can be used outside injection context when passing an injector', () => {
expect(
injectIsMutating(undefined, TestBed.inject(Injector)),
injectIsMutating(undefined, {
injector: TestBed.inject(Injector),
}),
).not.toThrow()
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,9 @@ describe('injectMutation', () => {
mutationKey: ['injectionContextError'],
mutationFn: () => Promise.resolve(),
}),
TestBed.inject(Injector),
{
injector: TestBed.inject(Injector),
},
)
}).not.toThrow()
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,9 @@ describe('injectQuery', () => {
queryKey: ['manualInjector'],
queryFn: simpleFetcher,
}),
TestBed.inject(Injector),
{
injector: TestBed.inject(Injector),
},
)

expect(query.status()).toBe('pending')
Expand Down
20 changes: 10 additions & 10 deletions packages/angular-query-experimental/src/inject-infinite-query.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { InfiniteQueryObserver } from '@tanstack/query-core'
import { createBaseQuery } from './create-base-query'
import { assertInjector } from './util/assert-injector/assert-injector'
import type { Injector } from '@angular/core'
import type {
DefaultError,
InfiniteData,
Expand All @@ -12,6 +11,7 @@ import type {
CreateInfiniteQueryOptions,
CreateInfiniteQueryResult,
DefinedCreateInfiniteQueryResult,
WithOptionalInjector,
} from './types'
import type {
DefinedInitialDataInfiniteOptions,
Expand All @@ -22,7 +22,7 @@ import type {
* Injects an infinite query: a declarative dependency on an asynchronous source of data that is tied to a unique key.
* Infinite queries can additively "load more" data onto an existing set of data or "infinite scroll"
* @param optionsFn - A function that returns infinite query options.
* @param injector - The Angular injector to use.
* @param options - Additional configuration.
* @returns The infinite query result.
* @public
*/
Expand All @@ -40,14 +40,14 @@ export function injectInfiniteQuery<
TQueryKey,
TPageParam
>,
injector?: Injector,
options?: WithOptionalInjector,
): DefinedCreateInfiniteQueryResult<TData, TError>

/**
* Injects an infinite query: a declarative dependency on an asynchronous source of data that is tied to a unique key.
* Infinite queries can additively "load more" data onto an existing set of data or "infinite scroll"
* @param optionsFn - A function that returns infinite query options.
* @param injector - The Angular injector to use.
* @param options - Additional configuration.
* @returns The infinite query result.
* @public
*/
Expand All @@ -65,14 +65,14 @@ export function injectInfiniteQuery<
TQueryKey,
TPageParam
>,
injector?: Injector,
options?: WithOptionalInjector,
): CreateInfiniteQueryResult<TData, TError>

/**
* Injects an infinite query: a declarative dependency on an asynchronous source of data that is tied to a unique key.
* Infinite queries can additively "load more" data onto an existing set of data or "infinite scroll"
* @param optionsFn - A function that returns infinite query options.
* @param injector - The Angular injector to use.
* @param options - Additional configuration.
* @returns The infinite query result.
* @public
*/
Expand All @@ -91,22 +91,22 @@ export function injectInfiniteQuery<
TQueryKey,
TPageParam
>,
injector?: Injector,
options?: WithOptionalInjector,
): CreateInfiniteQueryResult<TData, TError>

/**
* Injects an infinite query: a declarative dependency on an asynchronous source of data that is tied to a unique key.
* Infinite queries can additively "load more" data onto an existing set of data or "infinite scroll"
* @param optionsFn - A function that returns infinite query options.
* @param injector - The Angular injector to use.
* @param options - Additional configuration.
* @returns The infinite query result.
* @public
*/
export function injectInfiniteQuery(
optionsFn: () => CreateInfiniteQueryOptions,
injector?: Injector,
options?: WithOptionalInjector,
) {
return assertInjector(injectInfiniteQuery, injector, () =>
return assertInjector(injectInfiniteQuery, options?.injector, () =>
createBaseQuery(optionsFn, InfiniteQueryObserver as typeof QueryObserver),
)
}
9 changes: 5 additions & 4 deletions packages/angular-query-experimental/src/inject-is-fetching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,24 @@ import { DestroyRef, NgZone, inject, signal } from '@angular/core'
import { QueryClient, notifyManager } from '@tanstack/query-core'
import { assertInjector } from './util/assert-injector/assert-injector'
import type { QueryFilters } from '@tanstack/query-core'
import type { Injector, Signal } from '@angular/core'
import type { Signal } from '@angular/core'
import type { WithOptionalInjector } from './types'

/**
* Injects a signal that tracks the number of queries that your application is loading or
* fetching in the background.
*
* Can be used for app-wide loading indicators
* @param filters - The filters to apply to the query.
* @param injector - The Angular injector to use.
* @param options - Additional configuration
* @returns signal with number of loading or fetching queries.
* @public
*/
export function injectIsFetching(
filters?: QueryFilters,
injector?: Injector,
options?: WithOptionalInjector,
): Signal<number> {
return assertInjector(injectIsFetching, injector, () => {
return assertInjector(injectIsFetching, options?.injector, () => {
const destroyRef = inject(DestroyRef)
const ngZone = inject(NgZone)
const queryClient = inject(QueryClient)
Expand Down
9 changes: 5 additions & 4 deletions packages/angular-query-experimental/src/inject-is-mutating.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,23 @@ import { DestroyRef, NgZone, inject, signal } from '@angular/core'
import { QueryClient, notifyManager } from '@tanstack/query-core'
import { assertInjector } from './util/assert-injector/assert-injector'
import type { MutationFilters } from '@tanstack/query-core'
import type { Injector, Signal } from '@angular/core'
import type { Signal } from '@angular/core'
import type { WithOptionalInjector } from './types'

/**
* Injects a signal that tracks the number of mutations that your application is fetching.
*
* Can be used for app-wide loading indicators
* @param filters - The filters to apply to the query.
* @param injector - The Angular injector to use.
* @param options - Additional configuration
* @returns signal with number of fetching mutations.
* @public
*/
export function injectIsMutating(
filters?: MutationFilters,
injector?: Injector,
options?: WithOptionalInjector,
): Signal<number> {
return assertInjector(injectIsMutating, injector, () => {
return assertInjector(injectIsMutating, options?.injector, () => {
const destroyRef = inject(DestroyRef)
const ngZone = inject(NgZone)
const queryClient = inject(QueryClient)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@ import {
replaceEqualDeep,
} from '@tanstack/query-core'
import { assertInjector } from './util/assert-injector/assert-injector'
import type { Injector, Signal } from '@angular/core'
import type { Signal } from '@angular/core'
import type {
Mutation,
MutationCache,
MutationFilters,
MutationState,
} from '@tanstack/query-core'
import type { WithOptionalInjector } from './types'

type MutationStateOptions<TResult = MutationState> = {
filters?: MutationFilters
Expand All @@ -33,9 +34,7 @@ function getResult<TResult = MutationState>(
/**
* @public
*/
export interface InjectMutationStateOptions {
injector?: Injector
}
export type InjectMutationStateOptions = WithOptionalInjector

/**
* Injects a signal that tracks the state of all mutations.
Expand Down
16 changes: 10 additions & 6 deletions packages/angular-query-experimental/src/inject-mutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,19 @@ import { assertInjector } from './util/assert-injector/assert-injector'
import { signalProxy } from './signal-proxy'
import { noop, shouldThrowError } from './util'
import type { DefaultError, MutationObserverResult } from '@tanstack/query-core'
import type { CreateMutateFunction, CreateMutationResult } from './types'
import type {
CreateMutateFunction,
CreateMutationResult,
WithOptionalInjector,
} from './types'
import type { CreateMutationOptions } from './mutation-options'

/**
* Injects a mutation: an imperative function that can be invoked which typically performs server side effects.
*
* Unlike queries, mutations are not run automatically.
* @param optionsFn - A function that returns mutation options.
* @param injector - The Angular injector to use.
* @param options - Additional configuration
* @returns The mutation.
* @public
*/
Expand All @@ -37,9 +41,9 @@ export function injectMutation<
TContext = unknown,
>(
optionsFn: () => CreateMutationOptions<TData, TError, TVariables, TContext>,
injector?: Injector,
options?: WithOptionalInjector,
): CreateMutationResult<TData, TError, TVariables, TContext> {
return assertInjector(injectMutation, injector, () => {
return assertInjector(injectMutation, options?.injector, () => {
const currentInjector = inject(Injector)
const destroyRef = inject(DestroyRef)
const ngZone = inject(NgZone)
Expand Down Expand Up @@ -104,7 +108,7 @@ export function injectMutation<
})
},
{
injector,
injector: options?.injector,
},
)

Expand Down Expand Up @@ -136,7 +140,7 @@ export function injectMutation<
})
},
{
injector,
injector: options?.injector,
},
)

Expand Down
9 changes: 5 additions & 4 deletions packages/angular-query-experimental/src/inject-queries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
signal,
} from '@angular/core'
import { assertInjector } from './util/assert-injector/assert-injector'
import type { Injector, Signal } from '@angular/core'
import type { Signal } from '@angular/core'
import type {
DefaultError,
OmitKeyof,
Expand All @@ -24,6 +24,7 @@
QueryObserverResult,
ThrowOnError,
} from '@tanstack/query-core'
import type { WithOptionalInjector } from './types'

// This defines the `CreateQueryOptions` that are accepted in `QueriesOptions` & `GetOptions`.
// `placeholderData` function does not have a parameter
Expand Down Expand Up @@ -196,7 +197,7 @@
* @param root0
* @param root0.queries
* @param root0.combine
* @param injector
* @param props
* @public
*/
export function injectQueries<
Expand All @@ -210,9 +211,9 @@
queries: Signal<[...QueriesOptions<T>]>
combine?: (result: QueriesResults<T>) => TCombinedResult
},
injector?: Injector,
props?: WithOptionalInjector,
): Signal<TCombinedResult> {
return assertInjector(injectQueries, injector, () => {
return assertInjector(injectQueries, props?.injector, () => {

Check warning on line 216 in packages/angular-query-experimental/src/inject-queries.ts

View check run for this annotation

Codecov / codecov/patch

packages/angular-query-experimental/src/inject-queries.ts#L216

Added line #L216 was not covered by tests
const destroyRef = inject(DestroyRef)
const ngZone = inject(NgZone)
const queryClient = inject(QueryClient)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Injector, inject } from '@angular/core'
import { QueryClient } from '@tanstack/query-core'
import type { InjectOptions } from '@angular/core'
import type { WithOptionalInjector } from './types'

/**
* Injects a `QueryClient` instance and allows passing a custom injector.
Expand All @@ -17,7 +18,7 @@
* ```
*/
export function injectQueryClient(
injectOptions: InjectOptions & { injector?: Injector } = {},
injectOptions: InjectOptions & WithOptionalInjector = {},

Check warning on line 21 in packages/angular-query-experimental/src/inject-query-client.ts

View check run for this annotation

Codecov / codecov/patch

packages/angular-query-experimental/src/inject-query-client.ts#L21

Added line #L21 was not covered by tests
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need to change injectQueryClient as it's deprecated and will be removed.

) {
return (injectOptions.injector ?? inject(Injector)).get(QueryClient)
}
Loading
Loading