From e80086b771e7b86733abd5925afcb33cea661ff5 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Sat, 30 Dec 2023 13:33:17 +0100 Subject: [PATCH 1/2] wip --- playground/src/App.vue | 5 + playground/src/pages/users/query.[id].vue | 55 ++-- playground/src/pages/users/tq-query.[id].vue | 50 ++++ playground/typed-router.d.ts | 1 + src/data-fetching_new/createDataLoader.ts | 10 +- src/data-fetching_new/defineLoader.spec.ts | 241 +++++++++++++++++- src/data-fetching_new/defineLoader.ts | 39 ++- src/data-fetching_new/defineQueryLoader.ts | 140 +++++----- src/data-fetching_new/meta-extensions.ts | 9 +- .../navigation-guard.spec.ts | 4 +- src/data-fetching_new/navigation-guard.ts | 12 - src/data-fetching_new/symbols.ts | 6 - src/runtime.ts | 2 +- tests/data-loaders/tester.ts | 126 +-------- tests/router-mock.ts | 4 +- 15 files changed, 435 insertions(+), 269 deletions(-) create mode 100644 playground/src/pages/users/tq-query.[id].vue diff --git a/playground/src/App.vue b/playground/src/App.vue index 58ce30dc3..e5ba09388 100644 --- a/playground/src/App.vue +++ b/playground/src/App.vue @@ -50,6 +50,11 @@ const customRoute = useRoute('/deep/nesting/works/custom-path') href }} +
  • + {{ + href + }} +
  • [name]
  • diff --git a/playground/src/pages/users/query.[id].vue b/playground/src/pages/users/query.[id].vue index 170ab27a6..32b0942a2 100644 --- a/playground/src/pages/users/query.[id].vue +++ b/playground/src/pages/users/query.[id].vue @@ -1,13 +1,13 @@ - - -const a = 20 as 20 | 30 -console.log('WITHIN ROUTE_BLOCK', a) +
    -export default { - alias: '/u/:id', - meta: { - a, - other: 'other', - }, -} -
    +

    TQ

    +
    Loading...
    +
    Error: {{ tqError }}
    +
    {{ tqUser }}
    + + diff --git a/playground/src/pages/users/tq-query.[id].vue b/playground/src/pages/users/tq-query.[id].vue new file mode 100644 index 000000000..4b561dfb1 --- /dev/null +++ b/playground/src/pages/users/tq-query.[id].vue @@ -0,0 +1,50 @@ + + + + + diff --git a/playground/typed-router.d.ts b/playground/typed-router.d.ts index 5aed013bd..70e2248d7 100644 --- a/playground/typed-router.d.ts +++ b/playground/typed-router.d.ts @@ -82,6 +82,7 @@ declare module 'vue-router/auto/routes' { '/users/[id].edit': RouteRecordInfo<'/users/[id].edit', '/users/:id/edit', { id: ParamValue }, { id: ParamValue }>, '/users/nested.route.deep': RouteRecordInfo<'/users/nested.route.deep', '/users/nested/route/deep', Record, Record>, '/users/query.[id]': RouteRecordInfo<'/users/query.[id]', '/users/query/:id', { id: ParamValue }, { id: ParamValue }>, + '/users/tq-query.[id]': RouteRecordInfo<'/users/tq-query.[id]', '/users/tq-query/:id', { id: ParamValue }, { id: ParamValue }>, '/vuefire-tests/get-doc': RouteRecordInfo<'/vuefire-tests/get-doc', '/vuefire-tests/get-doc', Record, Record>, '/with-extension': RouteRecordInfo<'/with-extension', '/with-extension', Record, Record>, } diff --git a/src/data-fetching_new/createDataLoader.ts b/src/data-fetching_new/createDataLoader.ts index b24a9751d..fa211e541 100644 --- a/src/data-fetching_new/createDataLoader.ts +++ b/src/data-fetching_new/createDataLoader.ts @@ -23,7 +23,7 @@ export interface DataLoaderEntryBase< /** * Data stored in the entry. */ - data: Ref<_DataMaybeLazy, isLazy>> + data: Ref<_DataMaybeLazy> /** * Error if there was an error. @@ -36,6 +36,8 @@ export interface DataLoaderEntryBase< */ pending: Ref + options: DefineDataLoaderOptionsBase + /** * The latest pending load. Used to verify if the load is still valid when it resolves. */ @@ -178,7 +180,7 @@ export interface DataLoaderContextBase { export interface DefineDataLoader { ( - fn: DefineLoaderFn, Context>, + fn: DefineLoaderFn, options?: DefineDataLoaderOptionsBase // TODO: or a generic that allows a more complex UseDataLoader ): UseDataLoader @@ -333,9 +335,9 @@ function _testing() { * Loader function that can be passed to `defineLoader()`. */ export interface DefineLoaderFn< - P extends Promise, + Data, Context extends DataLoaderContextBase = DataLoaderContextBase, Route = RouteLocationNormalizedLoaded > { - (route: Route, context: Context): P + (route: Route, context: Context): Promise } diff --git a/src/data-fetching_new/defineLoader.spec.ts b/src/data-fetching_new/defineLoader.spec.ts index b351b1fdd..4ebd6177a 100644 --- a/src/data-fetching_new/defineLoader.spec.ts +++ b/src/data-fetching_new/defineLoader.spec.ts @@ -1,15 +1,246 @@ /** * @vitest-environment happy-dom */ -import { Ref, shallowRef } from 'vue' -import { defineBasicLoader } from './defineLoader' +import { App, Ref, defineComponent, shallowRef } from 'vue' +import { + INITIAL_DATA_KEY, + SERVER_INITIAL_DATA_KEY, + defineBasicLoader, +} from './defineLoader' import { expectType } from 'ts-expect' -import { describe } from 'vitest' -import { NavigationResult } from './navigation-guard' +import { + describe, + it, + expect, + vi, + afterEach, + beforeEach, + beforeAll, + afterAll, +} from 'vitest' +import { + DataLoaderPlugin, + DataLoaderPluginOptions, + NavigationResult, +} from './navigation-guard' import { testDefineLoader } from '~/tests/data-loaders' +import { setCurrentContext } from './utils' +import { UseDataLoader } from './createDataLoader' +import { getRouter } from 'vue-router-mock' +import { enableAutoUnmount, mount } from '@vue/test-utils' +import RouterViewMock from '~/tests/data-loaders/RouterViewMock.vue' +import { mockedLoader } from '~/tests/utils' +import { RouteLocationNormalizedLoaded } from 'vue-router' describe('defineLoader', () => { - testDefineLoader('Basic', defineBasicLoader, { ssrKeyName: 'key' }) + enableAutoUnmount(afterEach) + + beforeEach(() => { + // invalidate current context + setCurrentContext(undefined) + }) + + // we use fake timers to ensure debugging tests do not rely on timers + const now = new Date(2000, 0, 1).getTime() // 1 Jan 2000 in local time as number of milliseconds + beforeAll(() => { + vi.useFakeTimers() + vi.setSystemTime(now) + }) + + afterAll(() => { + vi.useRealTimers() + }) + + testDefineLoader('Basic', defineBasicLoader) + + function singleLoaderOneRoute( + useData: Loader, + pluginOptions?: Omit + ) { + let useDataResult: ReturnType + const component = defineComponent({ + setup() { + // @ts-expect-error: wat? + useDataResult = useData() + + const { data, error, pending } = useDataResult + return { data, error, pending } + }, + template: `\ +
    +

    {{ $route.path }}

    +

    {{ data }}

    +

    {{ error }}

    +

    {{ pending }}

    +
    `, + }) + const router = getRouter() + router.addRoute({ + name: '_test', + path: '/fetch', + meta: { + loaders: [useData], + }, + component, + }) + + const wrapper = mount(RouterViewMock, { + global: { + plugins: [[DataLoaderPlugin, { router, ...pluginOptions }]], + }, + }) + + const app: App = wrapper.vm.$.appContext.app + + return { + wrapper, + router, + useData: () => { + if (useDataResult) { + return useDataResult + } + // forced to ensure similar running context to within a component + // this is for tests that call useData() before the navigation is finished + setCurrentContext(undefined) + return app.runWithContext(() => useData()) as ReturnType + }, + app, + } + } + + describe('initialData', () => { + it.skip('stores initialData', async () => { + const router = getRouter() + const spy1 = vi.fn().mockResolvedValue('f1') + const spy2 = vi.fn().mockResolvedValue('f2') + const l1 = defineBasicLoader(spy1, { key: 'd1' }) + const l2 = defineBasicLoader(spy2, { key: 'd2' }) + router.addRoute({ + name: '_test', + path: '/fetch', + component: defineComponent({ + setup() { + const { data: d1 } = l1() + const { data: d2 } = l2() + return { d1, d2 } + }, + template: `

    {{ d1 }} {{ d2 }}

    `, + }), + meta: { + loaders: [l1, l2], + }, + }) + const wrapper = mount(RouterViewMock, { + global: { + plugins: [ + [ + DataLoaderPlugin, + { + router, + }, + ], + ], + }, + }) + + await router.push('/fetch?p=one') + expect(spy1).toHaveBeenCalledTimes(1) + expect(spy2).toHaveBeenCalledTimes(1) + expect(router[SERVER_INITIAL_DATA_KEY]).toEqual({ + d1: 'f1', + d2: 'f2', + }) + }) + + it('uses initialData if present', async () => { + const spy = vi + .fn<[to: RouteLocationNormalizedLoaded], Promise>() + .mockResolvedValue('initial') + const { wrapper, app, router, useData } = singleLoaderOneRoute( + defineBasicLoader(spy, { + // TODO: figure out a way of passing these options that are specific to some + key: 'root', + }) + ) + router[INITIAL_DATA_KEY] = { root: 'initial' } + + await router.push('/fetch?p=one') + const { data } = useData() + expect(data.value).toEqual('initial') + expect(spy).toHaveBeenCalledTimes(0) + }) + + it('ignores initialData on subsequent navigations', async () => { + const spy = vi + .fn<[to: RouteLocationNormalizedLoaded], Promise>() + .mockImplementation(async (to) => to.query.p as string) + const { wrapper, app, router, useData } = singleLoaderOneRoute( + defineBasicLoader(spy, { key: 'root' }) + ) + + router[INITIAL_DATA_KEY] = { root: 'initial' } + + await router.push('/fetch?p=one') + await router.push('/fetch?p=two') + const { data } = useData() + expect(spy).toHaveBeenCalledTimes(1) + expect(data.value).toEqual('two') + }) + + it('can use initialData on nested loaders that are not exported', async () => { + const router = getRouter() + const l1 = mockedLoader({ key: 'root' }) + const l2 = mockedLoader({ key: 'nested' }) + router.addRoute({ + name: '_test', + path: '/fetch', + component: defineComponent({ + setup() { + const { data } = l1.loader() + const { data: nested } = l2.loader() + return { data, nested } + }, + template: `

    {{ data }} {{ nested }}

    `, + }), + meta: { + loaders: [ + // we purposely only expose the 1st loader + l1.loader, + // l2.loader, + ], + }, + }) + const wrapper = mount(RouterViewMock, { + global: { + plugins: [ + [ + DataLoaderPlugin, + { + router, + }, + ], + ], + }, + }) + const app: App = wrapper.vm.$.appContext.app + router[INITIAL_DATA_KEY] = { root: 'initial', nested: 'nested-initial' } + + await router.push('/fetch?p=one') + const { data: root, pending: rootPending } = app.runWithContext(() => + l1.loader() + ) + const { data: nested, pending: nestedPending } = app.runWithContext(() => + l2.loader() + ) + expect(l1.spy).toHaveBeenCalledTimes(0) + expect(l2.spy).toHaveBeenCalledTimes(0) + expect(wrapper.text()).toBe('initial nested-initial') + expect(root.value).toEqual('initial') + expect(nested.value).toEqual('nested-initial') + expect(rootPending.value).toEqual(false) + expect(nestedPending.value).toEqual(false) + }) + }) }) // dts testing diff --git a/src/data-fetching_new/defineLoader.ts b/src/data-fetching_new/defineLoader.ts index 39bf9f01d..c9a149e8f 100644 --- a/src/data-fetching_new/defineLoader.ts +++ b/src/data-fetching_new/defineLoader.ts @@ -16,7 +16,6 @@ import { import { ABORT_CONTROLLER_KEY, APP_KEY, - INITIAL_DATA_KEY, IS_USE_DATA_LOADER_KEY, LOADER_ENTRIES_KEY, NAVIGATION_RESULTS_KEY, @@ -106,12 +105,14 @@ export function defineBasicLoader< const { error, pending, data } = entry - const initialRootData = to.meta[INITIAL_DATA_KEY] + // 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] const key = options.key || '' - const initialData = - initialRootData && key in initialRootData - ? initialRootData[key] - : STAGED_NO_VALUE + let initialData: unknown = STAGED_NO_VALUE + if (initialRootData && key in initialRootData) { + initialData = initialRootData[key] + delete initialRootData[key] + } // we are rendering for the first time and we have initial data // we need to synchronously set the value so it's available in components @@ -354,10 +355,11 @@ function createDefineLoaderEntry< ): DataLoaderEntryBase { return { // force the type to match - data: ref() as Ref<_DataMaybeLazy, isLazy>>, + data: ref() as Ref<_DataMaybeLazy>, pending: ref(false), error: shallowRef(), + options, children: new Set(), pendingLoad: null, pendingTo: null, @@ -366,8 +368,31 @@ function createDefineLoaderEntry< } satisfies DataLoaderEntryBase } +/** + * Symbol used to store the data in the router so it can be retrieved after the initial navigation. + * @internal + */ +export const SERVER_INITIAL_DATA_KEY = Symbol() + +/** + * Initial data generated on server and consumed on client. + * @internal + */ +export const INITIAL_DATA_KEY = Symbol() + /** * TODO: * - `refreshData()` -> refresh one or all data loaders * - `invalidateData()` / `clearData()` -> clear one or all data loaders (only useful if there is a cache strategy) */ + +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 | false + [INITIAL_DATA_KEY]?: Record | false + } +} diff --git a/src/data-fetching_new/defineQueryLoader.ts b/src/data-fetching_new/defineQueryLoader.ts index 935faf314..eaba35428 100644 --- a/src/data-fetching_new/defineQueryLoader.ts +++ b/src/data-fetching_new/defineQueryLoader.ts @@ -16,7 +16,6 @@ import { import { ABORT_CONTROLLER_KEY, APP_KEY, - INITIAL_DATA_KEY, IS_USE_DATA_LOADER_KEY, LOADER_ENTRIES_KEY, NAVIGATION_RESULTS_KEY, @@ -36,51 +35,43 @@ import { UseQueryReturnType, useQuery, } from '@tanstack/vue-query' +import { SERVER_INITIAL_DATA_KEY } from './defineLoader' -export function defineQueryLoader< - P extends Promise, - isLazy extends boolean ->( +export function defineQueryLoader( name: RouteRecordName, loader: ( route: RouteLocationNormalizedLoaded, context: QueryLoaderContext - ) => P, - options?: DefineQueryLoaderOptions> -): UseDataLoader> -export function defineQueryLoader< - P extends Promise, - isLazy extends boolean ->( + ) => Promise, + options?: DefineQueryLoaderOptions +): UseDataLoader +export function defineQueryLoader( loader: ( route: RouteLocationNormalizedLoaded, context: QueryLoaderContext - ) => P, - options?: DefineQueryLoaderOptions> -): UseDataLoader> - -export function defineQueryLoader< - P extends Promise, - isLazy extends boolean ->( - nameOrLoader: RouteRecordName | DefineLoaderFn, + ) => Promise, + options?: DefineQueryLoaderOptions +): UseDataLoader + +export function defineQueryLoader( + nameOrLoader: RouteRecordName | DefineLoaderFn, _loaderOrOptions?: - | DefineQueryLoaderOptions> - | DefineLoaderFn, - opts?: DefineQueryLoaderOptions> -): UseDataLoader> { + | DefineQueryLoaderOptions + | DefineLoaderFn, + opts?: DefineQueryLoaderOptions +): UseDataLoader { // TODO: make it DEV only and remove the first argument in production mode // resolve option overrides const loader = typeof nameOrLoader === 'function' ? nameOrLoader - : (_loaderOrOptions! as DefineLoaderFn) + : (_loaderOrOptions! as DefineLoaderFn) opts = typeof _loaderOrOptions === 'object' ? _loaderOrOptions : opts const options = assign( - {} as DefineQueryLoaderOptions>, - DEFAULT_DEFINE_LOADER_OPTIONS, + {} as DefineQueryLoaderOptions, + DEFAULT_DEFINE_QUERY_LOADER_OPTIONS, opts - ) satisfies DefineQueryLoaderOptions> + ) satisfies DefineQueryLoaderOptions function load( to: RouteLocationNormalizedLoaded, @@ -89,16 +80,16 @@ export function defineQueryLoader< ): Promise { const entries = router[LOADER_ENTRIES_KEY]! if (!entries.has(loader)) { - const queryResult = useQuery, Error, Awaited

    >({ + // TODO: ensure there is an effectScope + const queryResult = useQuery({ ...options, queryFn: () => loader(to, { signal: to.meta[ABORT_CONTROLLER_KEY]!.signal }), - // queryKey: ['hey'] }) queryResult.data.value entries.set(loader, { // force the type to match - data: ref(), + data: ref() as Ref, // data: queryResult.data as any, pending: ref(false), error: shallowRef(), @@ -111,20 +102,21 @@ export function defineQueryLoader< // TODO: could we find a way to make this type safe through the type of loader? // @ts-expect-error vq: queryResult, - } satisfies QueryLoaderEntry) + } satisfies QueryLoaderEntry) } const entry = entries.get(loader)! + const key: string = options.queryKey?.join('/') || '' // Nested loaders might get called before the navigation guard calls them, so we need to manually skip these calls if (entry.pendingTo === to && entry.pendingLoad) { - // console.log(`πŸ” already loading "${options.key}"`) + console.log(`πŸ” already loading "${key}"`) return entry.pendingLoad } const { error, pending, data } = entry - const initialRootData = to.meta[INITIAL_DATA_KEY] - const key = options.key || '' + // FIXME: not needed because vue query has its own state + const initialRootData: Record = {} const initialData = (initialRootData && key in initialRootData && initialRootData[key]) ?? STAGED_NO_VALUE @@ -138,9 +130,9 @@ export function defineQueryLoader< return (entry.pendingLoad = Promise.resolve()) } - // console.log( - // `😎 Loading context to "${to.fullPath}" with current "${currentContext[2]?.fullPath}"` - // ) + console.log( + `😎 Loading context to "${to.fullPath}" with current "${entry.pendingTo?.fullPath}"` + ) // Currently load for this loader entry.pendingTo = to @@ -152,7 +144,7 @@ export function defineQueryLoader< if (process.env.NODE_ENV === 'development') { if (parent !== currentContext[0]) { console.warn( - `βŒπŸ‘Ά "${options.key}" has a different parent than the current context. This shouldn't be happening. Please report a bug with a reproduction to https://github.com/posva/unplugin-vue-router/` + `βŒπŸ‘Ά "${key}" has a different parent than the current context. This shouldn't be happening. Please report a bug with a reproduction to https://github.com/posva/unplugin-vue-router/` ) } } @@ -164,22 +156,22 @@ export function defineQueryLoader< loader(to, { signal: to.meta[ABORT_CONTROLLER_KEY]!.signal }) ) .then((d) => { - // console.log( - // `βœ… resolved ${options.key}`, - // to.fullPath, - // `accepted: ${entry.pendingLoad === currentLoad}; data: ${d}` - // ) + console.log( + `βœ… resolved ${key}`, + to.fullPath, + `accepted: ${entry.pendingLoad === currentLoad}; data: ${d}` + ) if (entry.pendingLoad === currentLoad) { entry.staged = d } }) .catch((e) => { - // console.log( - // '‼️ rejected', - // to.fullPath, - // `accepted: ${entry.pendingLoad === currentLoad} =`, - // e - // ) + console.log( + '‼️ rejected', + to.fullPath, + `accepted: ${entry.pendingLoad === currentLoad} =`, + e + ) if (entry.pendingLoad === currentLoad) { error.value = e // propagate error if non lazy or during SSR @@ -190,10 +182,7 @@ export function defineQueryLoader< }) .finally(() => { setCurrentContext(currentContext) - // console.log( - // `😩 restored context ${options.key}`, - // currentContext?.[2]?.fullPath - // ) + console.log(`😩 restored context ${key}`, currentContext?.[2]?.fullPath) if (entry.pendingLoad === currentLoad) { pending.value = false // we must run commit here so nested loaders are ready before used by their parents @@ -208,18 +197,23 @@ export function defineQueryLoader< // this still runs before the promise resolves even if loader is sync entry.pendingLoad = currentLoad - // console.log(`πŸ”Ά Promise set to pendingLoad "${options.key}"`) + console.log(`πŸ”Ά Promise set to pendingLoad "${key}"`) return currentLoad } - function commit(this: QueryLoaderEntry, to: RouteLocationNormalizedLoaded) { + function commit( + this: QueryLoaderEntry, + to: RouteLocationNormalizedLoaded + ) { if (this.pendingTo === to && !this.error.value) { - // console.log('πŸ‘‰ commit', this.staged) + console.log('πŸ‘‰ commit', this.staged) if (process.env.NODE_ENV === 'development') { if (this.staged === STAGED_NO_VALUE) { console.warn( - `Loader "${options.key}"'s "commit()" was called but there is no staged data.` + `Loader "${options.queryKey?.join( + '/' + )}"'s "commit()" was called but there is no staged data.` ) } } @@ -244,7 +238,7 @@ export function defineQueryLoader< // @ts-expect-error: requires the internals and symbol that are added later const useDataLoader: // for ts - UseDataLoader> = () => { + UseDataLoader = () => { // work with nested data loaders const [parentEntry, _router, _route] = getCurrentContext() // fallback to the global router and routes for useDataLoaders used within components @@ -281,9 +275,11 @@ export function defineQueryLoader< // the existing pending location isn't good, we need to load again (parentEntry && entry.pendingTo !== route) ) { - // console.log( - // `πŸ” loading from useData for "${options.key}": "${route.fullPath}"` - // ) + console.log( + `πŸ” loading from useData for "${options.queryKey?.join('/')}": "${ + route.fullPath + }"` + ) router[APP_KEY].runWithContext(() => load(route, router, parentEntry)) } @@ -292,7 +288,9 @@ export function defineQueryLoader< // add ourselves to the parent entry children if (parentEntry) { if (parentEntry === entry) { - console.warn(`πŸ‘ΆβŒ "${options.key}" has itself as parent`) + console.warn( + `πŸ‘ΆβŒ "${options.queryKey?.join('/')}" has itself as parent` + ) } // console.log(`πŸ‘Ά "${options.key}" has parent ${parentEntry}`) parentEntry.children.add(entry!) @@ -340,13 +338,8 @@ export function defineQueryLoader< export interface DefineQueryLoaderOptions extends DefineDataLoaderOptionsBase, - Omit, 'queryFn'> { - /** - * Key to use for SSR state. - * @deprecated: use `queryKey` instead - */ - key?: never -} + // NOTE: queryFn is always needed and passed as the first argument + Omit, 'queryFn'> {} export interface QueryLoaderEntry< isLazy extends boolean = boolean, @@ -357,12 +350,13 @@ export interface QueryLoaderEntry< export interface QueryLoaderContext extends DataLoaderContextBase {} -const DEFAULT_DEFINE_LOADER_OPTIONS: DefineQueryLoaderOptions< +const DEFAULT_DEFINE_QUERY_LOADER_OPTIONS: DefineQueryLoaderOptions< boolean, unknown > = { lazy: false, server: true, commit: 'immediate', - keepPreviousData: true, + + // keepPreviousData: true, } diff --git a/src/data-fetching_new/meta-extensions.ts b/src/data-fetching_new/meta-extensions.ts index 42bdf98ad..cfae319d0 100644 --- a/src/data-fetching_new/meta-extensions.ts +++ b/src/data-fetching_new/meta-extensions.ts @@ -7,7 +7,6 @@ import type { PENDING_LOCATION_KEY, ABORT_CONTROLLER_KEY, NAVIGATION_RESULTS_KEY, - INITIAL_DATA_KEY, } from './symbols' import { type NavigationResult } from './navigation-guard' @@ -19,7 +18,7 @@ export type _DefineLoaderEntryMap = WeakMap< // Depending on the `defineLoader()` they might use a different thing as key // e.g. an function for basic defineLoader, a doc instance for VueFire object, - DataLoaderEntryBase + DataLoaderEntryBase > // we want to import from this meta extensions to include the changes to route @@ -68,11 +67,5 @@ declare module 'vue-router' { * @internal */ [NAVIGATION_RESULTS_KEY]?: NavigationResult[] - - /** - * The initial data of all loaders ran on the server. This is only used once. - * @internal - */ - [INITIAL_DATA_KEY]?: Record } } diff --git a/src/data-fetching_new/navigation-guard.spec.ts b/src/data-fetching_new/navigation-guard.spec.ts index 9253a7f9a..9e4b7a309 100644 --- a/src/data-fetching_new/navigation-guard.spec.ts +++ b/src/data-fetching_new/navigation-guard.spec.ts @@ -237,7 +237,7 @@ describe('navigation-guard', () => { expect(router.currentRoute.value.path).toBe('/fetch') }) - it(' does not run loaders on server side if server: false', async () => { + it('does not run loaders on server side if server: false', async () => { // @ts-expect-error: normally not allowed _utils.IS_CLIENT = false const router = getRouter() @@ -259,7 +259,7 @@ describe('navigation-guard', () => { expect(l2.spy).not.toHaveBeenCalled() }) - it.each([true, false])( + it.each([true, false] as const)( 'throws if a non lazy loader rejects, IS_CLIENT: %s', async (isClient) => { // @ts-expect-error: normally not allowed diff --git a/src/data-fetching_new/navigation-guard.ts b/src/data-fetching_new/navigation-guard.ts index 6951b40c9..aef49581b 100644 --- a/src/data-fetching_new/navigation-guard.ts +++ b/src/data-fetching_new/navigation-guard.ts @@ -8,7 +8,6 @@ import { effectScope, type App, type EffectScope } from 'vue' import { ABORT_CONTROLLER_KEY, APP_KEY, - INITIAL_DATA_KEY, LOADER_ENTRIES_KEY, LOADER_SET_KEY, NAVIGATION_RESULTS_KEY, @@ -34,7 +33,6 @@ export function setupLoaderGuard({ app, effect, selectNavigationResult = (results) => results[0].value, - initialData, }: SetupLoaderGuardOptions) { // avoid creating the guards multiple times if (router[LOADER_ENTRIES_KEY] != null) { @@ -77,11 +75,6 @@ export function setupLoaderGuard({ to.meta[ABORT_CONTROLLER_KEY] = new AbortController() // allow loaders to add navigation results to.meta[NAVIGATION_RESULTS_KEY] = [] - // set the initial data on the route so it can be used by the loaders, including - // nested ones - to.meta[INITIAL_DATA_KEY] = initialData - // clean it up so it can't be used again - initialData = undefined // Collect all the lazy loaded components to await them in parallel const lazyLoadingPromises: Promise[] = [] @@ -282,11 +275,6 @@ export interface SetupLoaderGuardOptions { */ router: Router - /** - * Initial data to skip the initial data loaders. This is useful for SSR and should be set only on client side. - */ - initialData?: Record - /** * Called if any data loader returns a `NavigationResult` with an array of them. Should decide what is the outcome of * the data fetching guard. Note this isn't called if no data loaders return a `NavigationResult` or if an error is thrown. diff --git a/src/data-fetching_new/symbols.ts b/src/data-fetching_new/symbols.ts index 2bbd550e9..a5d65ef31 100644 --- a/src/data-fetching_new/symbols.ts +++ b/src/data-fetching_new/symbols.ts @@ -46,9 +46,3 @@ export const ABORT_CONTROLLER_KEY = Symbol() * @internal */ export const NAVIGATION_RESULTS_KEY = Symbol() - -/** - * Initial data generated on server and consumed on client. - * @internal - */ -export const INITIAL_DATA_KEY = Symbol() diff --git a/src/runtime.ts b/src/runtime.ts index ddefb7c21..17e5b2d28 100644 --- a/src/runtime.ts +++ b/src/runtime.ts @@ -22,7 +22,7 @@ export { export type { DataLoaderPluginOptions } from './data-fetching_new/navigation-guard' // NOTE: for tests only -// export * from './data-fetching_new/defineQueryLoader' +export * from './data-fetching_new/defineQueryLoader' /** * Defines properties of the route for the current page component. diff --git a/tests/data-loaders/tester.ts b/tests/data-loaders/tester.ts index bc1d830e0..acb3cd7d6 100644 --- a/tests/data-loaders/tester.ts +++ b/tests/data-loaders/tester.ts @@ -1,8 +1,7 @@ /** * @vitest-environment happy-dom */ -import { type App, type Ref, defineComponent, inject, shallowRef } from 'vue' -import { expectType } from 'ts-expect' +import { type App, defineComponent, inject, shallowRef } from 'vue' import { afterAll, afterEach, @@ -13,13 +12,12 @@ import { it, vi, } from 'vitest' -import { enableAutoUnmount, mount } from '@vue/test-utils' +import { disableAutoUnmount, enableAutoUnmount, mount } from '@vue/test-utils' import { getRouter } from 'vue-router-mock' import { setCurrentContext } from '~/src/data-fetching_new/utils' import { DataLoaderPlugin, type DataLoaderPluginOptions, - NavigationResult, } from '~/src/data-fetching_new/navigation-guard' import type { DataLoaderContextBase, @@ -34,32 +32,13 @@ import type { RouteLocationNormalizedLoaded } from 'vue-router' export function testDefineLoader< DefineLoaderT extends DefineDataLoader ->( - name: string, - defineLoader: DefineLoaderT, - { ssrKeyName }: { ssrKeyName: string } -) { +>(name: string, defineLoader: DefineLoaderT) { describe(name, () => { beforeEach(() => { - // invalidate current context - setCurrentContext(undefined) dataOneSpy.mockClear() dataTwoSpy.mockClear() }) - enableAutoUnmount(afterEach) - - // we use fake timers to ensure debugging tests do not rely on timers - const now = new Date(2000, 0, 1).getTime() // 1 Jan 2000 in local time as number of milliseconds - beforeAll(() => { - vi.useFakeTimers() - vi.setSystemTime(now) - }) - - afterAll(() => { - vi.useRealTimers() - }) - function singleLoaderOneRoute( useData: Loader, pluginOptions?: Omit @@ -812,104 +791,5 @@ export function testDefineLoader< await vi.runOnlyPendingTimersAsync() expect(data.value).toEqual('ok,one') }) - - it('uses initialData if present', async () => { - const spy = vi - .fn<[to: RouteLocationNormalizedLoaded], Promise>() - .mockResolvedValue('initial') - const { wrapper, app, router, useData } = singleLoaderOneRoute( - defineLoader(spy, { - // TODO: figure out a way of passing these options that are specific to some - [ssrKeyName]: 'root', - }), - { - initialData: { - root: 'initial', - }, - } - ) - - await router.push('/fetch?p=one') - const { data } = useData() - expect(data.value).toEqual('initial') - expect(spy).toHaveBeenCalledTimes(0) - }) - - it('ignores initialData on subsequent navigations', async () => { - const spy = vi - .fn<[to: RouteLocationNormalizedLoaded], Promise>() - .mockImplementation(async (to) => to.query.p as string) - const { wrapper, app, router, useData } = singleLoaderOneRoute( - defineLoader(spy, { [ssrKeyName]: 'root' }), - { - initialData: { - root: 'initial', - }, - } - ) - - await router.push('/fetch?p=one') - await router.push('/fetch?p=two') - const { data } = useData() - expect(spy).toHaveBeenCalledTimes(1) - expect(data.value).toEqual('two') - }) - - it('can use initialData on nested loaders that are not exported', async () => { - const router = getRouter() - const l1 = mockedLoader({ key: 'root' }) - const l2 = mockedLoader({ key: 'nested' }) - router.addRoute({ - name: '_test', - path: '/fetch', - component: defineComponent({ - setup() { - const { data } = l1.loader() - const { data: nested } = l2.loader() - return { data, nested } - }, - template: `

    {{ data }} {{ nested }}

    `, - }), - meta: { - loaders: [ - // we purposely only expose the 1st loader - l1.loader, - // l2.loader, - ], - }, - }) - const wrapper = mount(RouterViewMock, { - global: { - plugins: [ - [ - DataLoaderPlugin, - { - router, - initialData: { - root: 'initial', - nested: 'nested-initial', - }, - }, - ], - ], - }, - }) - const app: App = wrapper.vm.$.appContext.app - - await router.push('/fetch?p=one') - const { data: root, pending: rootPending } = app.runWithContext(() => - l1.loader() - ) - const { data: nested, pending: nestedPending } = app.runWithContext(() => - l2.loader() - ) - expect(l1.spy).toHaveBeenCalledTimes(0) - expect(l2.spy).toHaveBeenCalledTimes(0) - expect(wrapper.text()).toBe('initial nested-initial') - expect(root.value).toEqual('initial') - expect(nested.value).toEqual('nested-initial') - expect(rootPending.value).toEqual(false) - expect(nestedPending.value).toEqual(false) - }) }) } diff --git a/tests/router-mock.ts b/tests/router-mock.ts index a7c7f4a98..cb26e286e 100644 --- a/tests/router-mock.ts +++ b/tests/router-mock.ts @@ -5,14 +5,14 @@ import { RouterMockOptions, } from 'vue-router-mock' import { config } from '@vue/test-utils' -import { beforeEach, vi, SpyInstance } from 'vitest' +import { beforeEach, vi, MockInstance } from 'vitest' export function createRouterMock(options?: RouterMockOptions) { return _createRouterMock({ ...options, spy: { create: (fn) => vi.fn(fn), - reset: (spy: SpyInstance) => spy.mockClear(), + reset: (spy: MockInstance) => spy.mockClear(), ...options?.spy, }, }) From fd768548d754f6016a26c7480ec8172455ffeb1a Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Tue, 2 Jan 2024 22:12:47 +0100 Subject: [PATCH 2/2] types: use RouterTyped in createRouter --- client.d.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/client.d.ts b/client.d.ts index b7cfa0892..46356d86e 100644 --- a/client.d.ts +++ b/client.d.ts @@ -1,5 +1,8 @@ declare module 'vue-router/auto/routes' { import type { RouteRecordRaw } from 'vue-router' + /** + * Array of routes generated by unplugin-vue-router + */ export const routes: RouteRecordRaw[] } @@ -12,12 +15,13 @@ declare module 'vue-router/auto' { */ export interface _RouterOptions extends Omit { /** - * Allows modifying the routes before they are passed to the router. You can modify the existing array or return a + * Modify the routes before they are passed to the router. You can modify the existing array or return a * new one. * * @param routes - The routes generated by this plugin and exposed by `vue-router/auto/routes` */ extendRoutes?: (routes: RouteRecordRaw[]) => RouteRecordRaw[] | void } - export function createRouter(options: _RouterOptions): Router + + export function createRouter(options: _RouterOptions): RouterTyped }