Skip to content

Commit

Permalink
refactor(angular-query): use only computed signals for lazy initializ…
Browse files Browse the repository at this point in the history
…ation
  • Loading branch information
arnoud-dv committed Dec 6, 2024
1 parent b7bad3d commit 377cd13
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ describe('injectQuery', () => {
const withResultInfer = TestBed.runInInjectionContext(() =>
injectQuery(() => ({
queryKey: key,
queryFn: async () => true,
queryFn: () => true,
})),
)
expectTypeOf(withResultInfer.data()).toEqualTypeOf<boolean | undefined>()
Expand Down Expand Up @@ -263,8 +263,6 @@ describe('injectQuery', () => {
expect(query.isFetching()).toBe(true)
expect(query.isStale()).toBe(true)
expect(query.isFetched()).toBe(false)

flush()
}))

test('should resolve to success and update signal: injectQuery()', fakeAsync(() => {
Expand All @@ -275,7 +273,7 @@ describe('injectQuery', () => {
}))
})

flush()
tick()

expect(query.status()).toBe('success')
expect(query.data()).toBe('result2')
Expand All @@ -294,7 +292,7 @@ describe('injectQuery', () => {
}))
})

flush()
tick()

expect(query.status()).toBe('error')
expect(query.data()).toBe(undefined)
Expand All @@ -316,7 +314,7 @@ describe('injectQuery', () => {
queryFn: spy,
}))
})
flush()
tick()
expect(spy).toHaveBeenCalledTimes(1)

expect(query.status()).toBe('success')
Expand All @@ -331,7 +329,6 @@ describe('injectQuery', () => {
queryKey: ['key8'],
signal: expect.anything(),
})
flush()
}))

test('should only run query once enabled signal is set to true', fakeAsync(() => {
Expand All @@ -350,8 +347,7 @@ describe('injectQuery', () => {
expect(query.status()).toBe('pending')

enabled.set(true)
TestBed.flushEffects()
flush()
tick()
expect(spy).toHaveBeenCalledTimes(1)
expect(query.status()).toBe('success')
}))
Expand Down Expand Up @@ -381,7 +377,6 @@ describe('injectQuery', () => {
expect(dependentQueryFn).not.toHaveBeenCalled()

tick()
TestBed.flushEffects()

expect(query1.data()).toStrictEqual('Some data')
expect(query2.fetchStatus()).toStrictEqual('fetching')
Expand Down Expand Up @@ -419,7 +414,7 @@ describe('injectQuery', () => {
)
})

flush()
tick()

keySignal.set('key12')

Expand All @@ -433,8 +428,6 @@ describe('injectQuery', () => {
}),
)
})

flush()
}))

describe('throwOnError', () => {
Expand Down Expand Up @@ -471,7 +464,6 @@ describe('injectQuery', () => {
expect(() => {
flush()
}).toThrowError('Some error')
flush()
}))

test('should throw when throwOnError function returns true', fakeAsync(() => {
Expand All @@ -486,7 +478,6 @@ describe('injectQuery', () => {
expect(() => {
flush()
}).toThrowError('Some error')
flush()
}))
})

Expand All @@ -501,12 +492,12 @@ describe('injectQuery', () => {

expect(query.status()).toBe('pending')

flush()
tick()

expect(query.status()).toBe('error')
}))

test('should render with required signal inputs', fakeAsync(async () => {
test('should render with required signal inputs', fakeAsync(() => {
@Component({
selector: 'app-fake',
template: `{{ query.data() }}`,
Expand All @@ -517,7 +508,7 @@ describe('injectQuery', () => {

query = injectQuery(() => ({
queryKey: ['fake', this.name()],
queryFn: () => Promise.resolve(this.name()),
queryFn: () => this.name(),
}))
}

Expand All @@ -526,12 +517,10 @@ describe('injectQuery', () => {
name: 'signal-input-required-test',
})

flush()
fixture.detectChanges()
tick()

expect(fixture.debugElement.nativeElement.textContent).toEqual(
'signal-input-required-test',
)
expect(fixture.componentInstance.query.data()).toEqual('signal-input-required-test')
}))

test('should run optionsFn in injection context', fakeAsync(async () => {
Expand Down Expand Up @@ -565,13 +554,13 @@ describe('injectQuery', () => {

const fixture = TestBed.createComponent(FakeComponent)
fixture.detectChanges()
flush()
tick()

expect(fixture.componentInstance.query.data()).toEqual('test name')

fixture.componentInstance.name.set('test name 2')
fixture.detectChanges()
flush()
tick()

expect(fixture.componentInstance.query.data()).toEqual('test name 2')
}))
Expand Down Expand Up @@ -608,13 +597,13 @@ describe('injectQuery', () => {

const fixture = TestBed.createComponent(FakeComponent)
fixture.detectChanges()
flush()
tick()

expect(fixture.componentInstance.query.data()).toEqual('test name')

fixture.componentInstance.name.set('test name 2')
fixture.detectChanges()
flush()
tick()

expect(fixture.componentInstance.query.data()).toEqual('test name 2')
}))
Expand Down
110 changes: 66 additions & 44 deletions packages/angular-query-experimental/src/create-base-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
import { QueryClient, notifyManager } from '@tanstack/query-core'
import { signalProxy } from './signal-proxy'
import { shouldThrowError } from './util'
import { lazyInit } from './util/lazy-init/lazy-init'
import type {
QueryKey,
QueryObserver,
Expand Down Expand Up @@ -40,58 +39,75 @@ export function createBaseQuery<
Observer: typeof QueryObserver,
) {
const injector = inject(Injector)
return lazyInit(() => {
const ngZone = injector.get(NgZone)
const destroyRef = injector.get(DestroyRef)
const queryClient = injector.get(QueryClient)
const ngZone = injector.get(NgZone)
const destroyRef = injector.get(DestroyRef)
const queryClient = injector.get(QueryClient)

/**
* Signal that has the default options from query client applied
* computed() is used so signals can be inserted into the options
* making it reactive. Wrapping options in a function ensures embedded expressions
* are preserved and can keep being applied after signal changes
*/
const defaultedOptionsSignal = computed(() => {
const options = runInInjectionContext(injector, () => optionsFn())
const defaultedOptions = queryClient.defaultQueryOptions(options)
defaultedOptions._optimisticResults = 'optimistic'
return defaultedOptions
})
/**
* Signal that has the default options from query client applied
* computed() is used so signals can be inserted into the options
* making it reactive. Wrapping options in a function ensures embedded expressions
* are preserved and can keep being applied after signal changes
*/
const defaultedOptionsSignal = computed(() => {
const options = runInInjectionContext(injector, () => optionsFn())
const defaultedOptions = queryClient.defaultQueryOptions(options)
defaultedOptions._optimisticResults = 'optimistic'
return defaultedOptions
})

const observer = new Observer<
const observerSignal = (() => {
let instance: QueryObserver<
TQueryFnData,
TError,
TData,
TQueryData,
TQueryKey
>(queryClient, defaultedOptionsSignal())
> | null = null

return computed(() => {
return (instance ||= new Observer(queryClient, defaultedOptionsSignal()))
})
})()

const optimisticResultSignal = computed(() =>
observerSignal().getOptimisticResult(defaultedOptionsSignal()),
)

const subscriberResultSignal = signal<QueryObserverResult<
TData,
TError
> | null>(null)

const resultSignal = signal(
observer.getOptimisticResult(defaultedOptionsSignal()),
)
effect(
(onCleanup) => {
const observer = observerSignal()
const defaultedOptions = defaultedOptionsSignal()

effect(
() => {
const defaultedOptions = defaultedOptionsSignal()
untracked(() => {
observer.setOptions(defaultedOptions, {
// Do not notify on updates because of changes in the options because
// these changes should already be reflected in the optimistic result.
listeners: false,
})
untracked(() => {
resultSignal.set(observer.getOptimisticResult(defaultedOptions))
})
},
{
injector,
},
)
})
onCleanup(() => {
subscriberResultSignal.set(null)
})
},
{
injector,
},
)

effect(() => {
// observer.trackResult is not used as this optimization is not needed for Angular
const unsubscribe = ngZone.runOutsideAngular(() =>
observer.subscribe(
notifyManager.batchCalls(
(state: QueryObserverResult<TData, TError>) => {
const observer = observerSignal()

untracked(() => {
const unsubscribe = ngZone.runOutsideAngular(() =>
observer.subscribe(
notifyManager.batchCalls((state) => {
ngZone.run(() => {
if (
state.isError &&
Expand All @@ -104,14 +120,20 @@ export function createBaseQuery<
) {
throw state.error
}
resultSignal.set(state)
subscriberResultSignal.set(state)
})
},
}),
),
),
)
destroyRef.onDestroy(unsubscribe)

return signalProxy(resultSignal)
)
destroyRef.onDestroy(unsubscribe)
})
})

return signalProxy(
computed(() => {
const subscriberResult = subscriberResultSignal()
const optimisticResult = optimisticResultSignal()
return subscriberResult ?? optimisticResult
}),
)
}
Loading

0 comments on commit 377cd13

Please sign in to comment.