Skip to content

Commit

Permalink
refactor: properly use when
Browse files Browse the repository at this point in the history
  • Loading branch information
posva committed Feb 5, 2024
1 parent 8e6cbf6 commit 85e8602
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 57 deletions.
40 changes: 13 additions & 27 deletions src/data-fetching-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class UseQueryEntry<TResult = unknown, TError = any> {
constructor(
initialData?: TResult,
error: TError | null = null,
when: number = Date.now()
when: number = 0 // stale by default
) {
this.data = ref(initialData) as Ref<TResult | undefined>
this.error = shallowRef(error)
Expand All @@ -91,18 +91,10 @@ export class UseQueryEntry<TResult = unknown, TError = any> {
}
const { key, staleTime } = this.options!

if (
this.error.value ||
!this.previous ||
isExpired(this.previous.when, staleTime)
) {
if (this.previous) {
console.log(
`⬇️ refresh "${String(key)}". expired ${
this.previous?.when
} / ${staleTime}`
)
}
if (this.error.value || isExpired(this.when, staleTime)) {
console.log(
`⬇️ refresh "${String(key)}". expired ${this.when} / ${staleTime}`
)

if (this.pending?.refreshCall) console.log(' -> skipped!')

Expand All @@ -124,37 +116,33 @@ export class UseQueryEntry<TResult = unknown, TError = any> {

console.log('🔄 refetching', this.options!.key)
this.status.value = 'loading'
// will become this.previous once fetched
const nextPrevious = {
when: 0,
data: undefined as TResult | undefined,
error: null as TError | null,
} satisfies UseQueryEntry<TResult, TError>['previous']

// we create an object and verify we are the most recent pending request
// before doing anything
const pendingEntry = (this.pending = {
refreshCall: this.options!.query()
.then((data) => {
if (pendingEntry === this.pending) {
nextPrevious.data = data
this.error.value = null
this.data.value = data
this.status.value = 'success'
}
})
.catch((error) => {
if (pendingEntry === this.pending) {
nextPrevious.error = error
this.error.value = error
this.status.value = 'error'
}
})
.finally(() => {
if (pendingEntry === this.pending) {
this.pending = null
nextPrevious.when = Date.now()
this.previous = nextPrevious
this.when = Date.now()
this.previous = {
data: this.data.value,
error: this.error.value,
when: this.when,
}
}
}),
when: Date.now(),
Expand Down Expand Up @@ -230,10 +218,8 @@ export const useDataFetchingStore = defineStore('PiniaColada', () => {
}

for (const entry of entryNode) {
if (entry.previous) {
// will force a fetch next time
entry.previous.when = 0
}
// will force a fetch next time
entry.when = 0

// TODO: if active only
if (refetch) {
Expand Down
55 changes: 29 additions & 26 deletions src/use-query.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ describe('useQuery', () => {
expect(wrapper.vm.data).toBe(2)
})

it.todo('avoids fetching if initial data is fresh', async () => {
it('avoids fetching if initial data is fresh', async () => {
const pinia = createPinia()

const entryRegistry = shallowReactive(
Expand All @@ -172,7 +172,7 @@ describe('useQuery', () => {
})

describe('staleTime', () => {
it('when refreshed, does not fetch again if staleTime has not elapsed', async () => {
it('when refreshed, does not fetch again if staleTime has not been elapsed', async () => {
const { wrapper, query } = mountSimple({ staleTime: 1000 })

await runTimers()
Expand Down Expand Up @@ -342,31 +342,32 @@ describe('useQuery', () => {
})
}

it.todo(
'refreshes the data even with initial values after staleTime is elapsed',
async () => {
const pinia = createPinia()
pinia.state.value.PiniaColada = {
entryRegistry: shallowReactive(new TreeMapNode(['key'], 60)),
it('refreshes the data even with initial values after staleTime is elapsed', async () => {
const pinia = createPinia()
pinia.state.value.PiniaColada = {
entryRegistry: shallowReactive(
new TreeMapNode(['key'], new UseQueryEntry(60, null, Date.now()))
),
}
const { wrapper, query } = mountSimple(
{
staleTime: 100,
},
{
plugins: [pinia],
}
const { wrapper, query } = mountSimple(
{
staleTime: 100,
},
{}
)
)

await runTimers()
expect(wrapper.vm.data).toBe(60)
expect(query).toHaveBeenCalledTimes(0)
await runTimers()
expect(wrapper.vm.data).toBe(60)
expect(query).toHaveBeenCalledTimes(0)

await vi.advanceTimersByTime(101)
wrapper.vm.refresh()
await runTimers()
expect(wrapper.vm.data).toBe(42)
expect(query).toHaveBeenCalledTimes(1)
}
)
await vi.advanceTimersByTime(101)
wrapper.vm.refresh()
await runTimers()
expect(wrapper.vm.data).toBe(42)
expect(query).toHaveBeenCalledTimes(1)
})

it('refreshes the data if mounted and the key changes', async () => {
const { wrapper, query } = mountDynamicKey({
Expand Down Expand Up @@ -505,6 +506,8 @@ describe('useQuery', () => {
expect(wrapper.vm.data).toBe(42)
})

// NOTE: these tests are a bit different from the ones in `initial state`.
// They create the serialized state rather than the actual state
it('uses the initial data if present in the store', async () => {
const pinia = createPinia()
const tree = createTreeMap()
Expand All @@ -518,7 +521,7 @@ describe('useQuery', () => {
expect(wrapper.vm.data).toBe(2)
})

it.todo('avoids fetching if initial data is fresh', async () => {
it('avoids fetching if initial data is fresh', async () => {
const pinia = createPinia()
const tree = createTreeMap()
tree.set(['key'], new UseQueryEntry(2, null, Date.now()))
Expand All @@ -531,9 +534,9 @@ describe('useQuery', () => {
)

await runTimers()

// it should not fetch and use the initial data
expect(query).toHaveBeenCalledTimes(0)
expect(wrapper.vm.data).toBe(2)
})
})
})
10 changes: 6 additions & 4 deletions src/use-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,12 @@ export function useQuery<TResult, TError = Error>(
if (options.refetchOnMount && hasCurrentInstance) {
// TODO: optimize so it doesn't refresh if we are hydrating
onMounted(() => {
if (options.refetchOnMount === 'always') {
refetch()
} else {
refresh()
if (options.refetchOnMount) {
if (options.refetchOnMount === 'always') {
refetch()
} else {
refresh()
}
}
})
}
Expand Down

0 comments on commit 85e8602

Please sign in to comment.