Skip to content

Commit

Permalink
refactor: rename pending to isLoading
Browse files Browse the repository at this point in the history
BREAKING CHANGE: Data Loaders now return an `isLoading` property instead
of `pending`. This aligns better with the wording of Data Loaders being
in a loading state rather than pending, which can have more meanings.
  • Loading branch information
posva committed Jan 16, 2024
1 parent 092e8bc commit 9502751
Show file tree
Hide file tree
Showing 13 changed files with 79 additions and 75 deletions.
4 changes: 2 additions & 2 deletions playground/src/pages/[name].vue
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ const thing = 'THING'
// const $route = useRoute()
const { data: user, pending, refresh } = useUserData()
const { data: user, isLoading, refresh } = useUserData()
const { data: one } = useOne()
const { data: two } = useTwo()
Expand Down Expand Up @@ -120,7 +120,7 @@ definePage({
<h1>Param: {{ $route.name === '/[name]' && $route.params.name }}</h1>
<h2>Param: {{ route.params.name }}</h2>
<p v-show="false">{{ thing }}</p>
<p v-if="pending">Loading user...</p>
<p v-if="isLoading">Loading user...</p>
<pre v-else>{{ user }}</pre>

<p>one:</p>
Expand Down
4 changes: 2 additions & 2 deletions playground/src/pages/users/[id].vue
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const useUserData = defineBasicLoader(
<script lang="ts" setup>
const route = useRoute('/users/[id]')
const { data: user, pending, error } = useUserData()
const { data: user, isLoading, error } = useUserData()
const { data: user2 } = useOldData()
definePage({
Expand Down Expand Up @@ -69,7 +69,7 @@ const MY_VAL = 'INSIDE SETUP TEST'
>Next</RouterLink
>

<pre v-if="pending">Loading...</pre>
<pre v-if="isLoading">Loading...</pre>
<pre v-else-if="error">Error: {{ error }}</pre>
<pre v-else>{{ user }}</pre>
</main>
Expand Down
4 changes: 2 additions & 2 deletions playground/src/pages/users/query.[id].vue
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ const useUserData = defineQueryLoader(
import { useQuery } from '@tanstack/vue-query'
const route = useRoute('/users/[id]')
const { data: user, pending, error } = useUserData()
const { data: user, isLoading, error } = useUserData()
const {
data: tqUser,
error: tqError,
Expand Down Expand Up @@ -120,7 +120,7 @@ const {
>

<h2>Data Loaders</h2>
<pre v-if="pending">Loading...</pre>
<pre v-if="isLoading">Loading...</pre>
<pre v-else-if="error">Error: {{ error }}</pre>
<pre v-else>{{ user }}</pre>

Expand Down
18 changes: 9 additions & 9 deletions src/data-fetching_new/createDataLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ export interface DataLoaderEntryBase<
*/
error: ShallowRef<any> // any is simply more convenient for errors

// TODO: allow delaying pending? maybe allow passing a custom ref that can use refDebounced https://vueuse.org/shared/refDebounced/#refdebounced
// TODO: allow delaying isLoading? maybe allow passing a custom ref that can use refDebounced https://vueuse.org/shared/refDebounced/#refdebounced
/**
* Whether there is an ongoing request.
*/
pending: Ref<boolean>
isLoading: Ref<boolean>

options: DefineDataLoaderOptionsBase<isLazy>

Expand Down Expand Up @@ -73,8 +73,8 @@ export function createDataLoader<Context extends DataLoaderContextBase>({
after,
}: CreateDataLoaderOptions<Context>): DefineDataLoader<Context> {
const defineLoader: DefineDataLoader<Context> = (dataLoader, options) => {
const { pending, error, data } = effectScope(true).run(() => ({
pending: ref(false),
const { isLoading, error, data } = effectScope(true).run(() => ({
isLoading: ref(false),
// TODO: allow generic for error type
error: ref<unknown | null>(null),
data: ref<unknown | undefined>(),
Expand All @@ -99,14 +99,14 @@ export function createDataLoader<Context extends DataLoaderContextBase>({
error.value = err
})
.finally(() => {
pending.value = false
isLoading.value = false
})

// TODO: merge promise

return {
data: data,
pending,
isLoading,
error,
}
// we need this cast because we add extra properties to the function object itself
Expand Down Expand Up @@ -196,10 +196,10 @@ export interface UseDataLoader<
* Data Loader composable returned by `defineLoader()`.
*
* @example
* Returns the Data loader data, pending, error etc. Meant to be used in `setup()` or `<script setup>` **without `await`**:
* Returns the Data loader data, isLoading, error etc. Meant to be used in `setup()` or `<script setup>` **without `await`**:
* ```vue
* <script setup>
* const { data, pending, error } = useUserData()
* const { data, isLoading, error } = useUserData()
* </script>
* ```
*
Expand Down Expand Up @@ -282,7 +282,7 @@ export interface UseDataLoaderResult<
/**
* Whether there is an ongoing request.
*/
pending: Ref<boolean>
isLoading: Ref<boolean>

/**
* Whether the loader is running or not. TODO: change pending to this and make pending wait until commit is called
Expand Down
8 changes: 4 additions & 4 deletions src/data-fetching_new/defineColadaLoader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { enableAutoUnmount, mount } from '@vue/test-utils'
import RouterViewMock from '../../tests/data-loaders/RouterViewMock.vue'
import { setActivePinia, createPinia, Pinia } from 'pinia'

describe(
describe.skip(
'defineColadaLoader',
() => {
enableAutoUnmount(afterEach)
Expand Down Expand Up @@ -67,15 +67,15 @@ describe(
// @ts-expect-error: wat?
useDataResult = useData()

const { data, error, pending } = useDataResult
return { data, error, pending }
const { data, error, isLoading: isLoading } = useDataResult
return { data, error, isLoading }
},
template: `\
<div>
<p id="route">{{ $route.path }}</p>
<p id="data">{{ data }}</p>
<p id="error">{{ error }}</p>
<p id="pending">{{ pending }}</p>
<p id="isLoading">{{ isLoading }}</p>
</div>`,
})
const router = getRouter()
Expand Down
27 changes: 7 additions & 20 deletions src/data-fetching_new/defineColadaLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export function defineColadaLoader<Data, isLazy extends boolean>(
pendingData: data,
pendingError: error,
// TODO: rename at some point
pending: isFetching,
isLoading: isFetching,
refresh,
refetch,
})
Expand All @@ -117,7 +117,7 @@ export function defineColadaLoader<Data, isLazy extends boolean>(
return entry.pendingLoad
}

const { error, pending, data } = entry
const { error, isLoading: isLoading, data } = entry

// FIXME: the key should be moved here and the strategy adapted to not depend on the navigation guard. This depends on how other loaders can be implemented.
const initialRootData = router[INITIAL_DATA_KEY]
Expand All @@ -144,7 +144,7 @@ export function defineColadaLoader<Data, isLazy extends boolean>(
entry.pendingTo = to

error.value = null
pending.value = true
isLoading.value = true
// save the current context to restore it later
const currentContext = getCurrentContext()

Expand Down Expand Up @@ -192,7 +192,7 @@ export function defineColadaLoader<Data, isLazy extends boolean>(
// currentContext?.[2]?.fullPath
// )
if (entry.pendingLoad === currentLoad) {
pending.value = false
isLoading.value = false
// we must run commit here so nested loaders are ready before used by their parents
if (options.lazy || options.commit === 'immediate') {
entry.commit(to)
Expand Down Expand Up @@ -298,12 +298,12 @@ export function defineColadaLoader<Data, isLazy extends boolean>(
parentEntry.children.add(entry!)
}

const { data, error, pending } = entry
const { data, error, isLoading } = entry

const useDataLoaderResult = {
data,
error,
pending,
isLoading,
refresh: (
// @ts-expect-error: FIXME: should be fixable
to: _RouteLocationNormalizedLoaded = router.currentRoute.value
Expand Down Expand Up @@ -364,7 +364,7 @@ function createDefineLoaderEntry<
return {
// force the type to match
data: ref() as Ref<_DataMaybeLazy<Data, isLazy>>,
pending: ref(false),
isLoading: ref(false),
error: shallowRef<any>(),

options,
Expand Down Expand Up @@ -393,16 +393,3 @@ export const INITIAL_DATA_KEY = Symbol()
* - `refreshData()` -> refresh one or all data loaders
* - `invalidateData()` / `clearData()` -> clear one or all data loaders (only useful if there is a cache strategy)
*/

// TODO: is it better to move this to an ambient declaration file so it's not included in the final bundle?

declare module 'vue-router' {
interface Router {
/**
* Gives access to the initial state during rendering. Should be set to `false` once it's consumed.
* @internal
*/
[SERVER_INITIAL_DATA_KEY]?: Record<string, unknown> | false
[INITIAL_DATA_KEY]?: Record<string, unknown> | false
}
}
10 changes: 5 additions & 5 deletions src/data-fetching_new/defineLoader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,15 @@ describe(
// @ts-expect-error: wat?
useDataResult = useData()

const { data, error, pending } = useDataResult
return { data, error, pending }
const { data, error, isLoading } = useDataResult
return { data, error, isLoading }
},
template: `\
<div>
<p id="route">{{ $route.path }}</p>
<p id="data">{{ data }}</p>
<p id="error">{{ error }}</p>
<p id="pending">{{ pending }}</p>
<p id="isLoading">{{ isLoading }}</p>
</div>`,
})
const router = getRouter()
Expand Down Expand Up @@ -229,10 +229,10 @@ describe(
router[INITIAL_DATA_KEY] = { root: 'initial', nested: 'nested-initial' }

await router.push('/fetch?p=one')
const { data: root, pending: rootPending } = app.runWithContext(() =>
const { data: root, isLoading: rootPending } = app.runWithContext(() =>
l1.loader()
)
const { data: nested, pending: nestedPending } = app.runWithContext(
const { data: nested, isLoading: nestedPending } = app.runWithContext(
() => l2.loader()
)
expect(l1.spy).toHaveBeenCalledTimes(0)
Expand Down
12 changes: 6 additions & 6 deletions src/data-fetching_new/defineLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export function defineBasicLoader<Data, isLazy extends boolean>(
return entry.pendingLoad
}

const { error, pending, data } = entry
const { error, isLoading, data } = entry

// FIXME: the key should be moved here and the strategy adapted to not depend on the navigation guard. This depends on how other loaders can be implemented.
const initialRootData = router[INITIAL_DATA_KEY]
Expand Down Expand Up @@ -125,7 +125,7 @@ export function defineBasicLoader<Data, isLazy extends boolean>(

// TODO: move to commit
error.value = null
pending.value = true
isLoading.value = true
// save the current context to restore it later
const currentContext = getCurrentContext()

Expand Down Expand Up @@ -177,7 +177,7 @@ export function defineBasicLoader<Data, isLazy extends boolean>(
// currentContext?.[2]?.fullPath
// )
if (entry.pendingLoad === currentLoad) {
pending.value = false
isLoading.value = false
// we must run commit here so nested loaders are ready before used by their parents
if (options.lazy || options.commit === 'immediate') {
entry.commit(to)
Expand Down Expand Up @@ -283,12 +283,12 @@ export function defineBasicLoader<Data, isLazy extends boolean>(
parentEntry.children.add(entry!)
}

const { data, error, pending } = entry
const { data, error, isLoading } = entry

const useDataLoaderResult = {
data,
error,
pending,
isLoading,
refresh: (
// @ts-expect-error: FIXME: should be fixable
to: _RouteLocationNormalizedLoaded = router.currentRoute.value
Expand Down Expand Up @@ -354,7 +354,7 @@ function createDefineLoaderEntry<
return {
// force the type to match
data: ref() as Ref<_DataMaybeLazy<Data, isLazy>>,
pending: ref(false),
isLoading: ref(false),
error: shallowRef<any>(),

options,
Expand Down
25 changes: 21 additions & 4 deletions src/data-fetching_new/defineQueryLoader.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,29 @@
import { Ref, shallowRef } from 'vue'
import { defineQueryLoader } from './defineQueryLoader'
import { expectType } from 'ts-expect'
import { describe } from 'vitest'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import { NavigationResult } from './navigation-guard'
import { testDefineLoader } from '../../tests/data-loaders'
import { enableAutoUnmount } from '@vue/test-utils'
import { setCurrentContext } from './utils'

describe.skip('defineQueryLoader', () => {
testDefineLoader('Basic', defineQueryLoader)
enableAutoUnmount(afterEach)
testDefineLoader(
({ fn, key, ...options }) =>
defineQueryLoader(fn, {
...options,
queryKey: key ? [key] : ['id'],
}),
{
beforeEach() {
// invalidate current context
setCurrentContext(undefined)
},
// TODO: query plugin
// plugins: ({ pinia }) => [pinia],
}
)
})

// dts testing
Expand All @@ -33,7 +50,7 @@ dts(async () => {
expectType<{
data: Ref<UserData>
error: Ref<unknown>
pending: Ref<boolean>
isLoading: Ref<boolean>
refresh: () => Promise<void>
}>(useDataLoader())

Expand All @@ -50,7 +67,7 @@ dts(async () => {
expectType<{
data: Ref<UserData>
error: Ref<unknown>
pending: Ref<boolean>
isLoading: Ref<boolean>
refresh: () => Promise<void>
}>(useWithRef())

Expand Down
Loading

0 comments on commit 9502751

Please sign in to comment.