From 423d9f763ed033dd6687ce7687c859c92554a60d Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Mon, 1 Jul 2024 10:42:00 +0200 Subject: [PATCH] fix(types): stricter meta with required fields --- packages/router/src/index.ts | 1 + packages/router/src/types/index.ts | 40 ++++++- .../router/test-dts/components.test-d.tsx | 47 ++++---- packages/router/test-dts/legacy.test-d.ts | 38 ++++-- packages/router/test-dts/meta.test-d.ts | 28 +++-- .../test-dts/navigationGuards.test-d.ts | 87 +++++++------- .../router/test-dts/routeRecords.test-d.ts | 109 ++++++++++-------- .../router/test-dts/typed-routes.test-d.ts | 2 +- packages/router/tsconfig.json | 5 +- packages/router/vitest.config.ts | 2 +- 10 files changed, 225 insertions(+), 134 deletions(-) diff --git a/packages/router/src/index.ts b/packages/router/src/index.ts index 2a62ad156..089cfe0a7 100644 --- a/packages/router/src/index.ts +++ b/packages/router/src/index.ts @@ -59,6 +59,7 @@ export type { RouteRecordMultipleViewsWithChildren, RouteRecordRedirect, RouteMeta, + _RouteMetaBase, RouteComponent, // RawRouteComponent, RouteParamsGeneric, diff --git a/packages/router/src/types/index.ts b/packages/router/src/types/index.ts index e3069740b..3508f3a6c 100644 --- a/packages/router/src/types/index.ts +++ b/packages/router/src/types/index.ts @@ -189,7 +189,9 @@ export type RawRouteComponent = RouteComponent | Lazy /** * Internal type for common properties among all kind of {@link RouteRecordRaw}. */ -export interface _RouteRecordBase extends PathParserOptions { +export interface _RouteRecordBase + extends PathParserOptions, + _RouteRecordBaseMeta { /** * Path of the record. Should start with `/` unless the record is the child of * another record. @@ -228,7 +230,7 @@ export interface _RouteRecordBase extends PathParserOptions { /** * Arbitrary data attached to the record. */ - meta?: RouteMeta + // meta?: RouteMeta /** * Array of nested routes. @@ -241,6 +243,12 @@ export interface _RouteRecordBase extends PathParserOptions { props?: _RouteRecordProps | Record } +/** + * Default type for RouteMeta when not augmented. + * @internal + */ +export type _RouteMetaBase = Record + /** * Interface to type `meta` fields in route records. * @@ -257,7 +265,33 @@ export interface _RouteRecordBase extends PathParserOptions { * } * ``` */ -export interface RouteMeta extends Record {} +export interface RouteMeta extends _RouteMetaBase {} + +/** + * Returns `true` if the passed `RouteMeta` type hasn't been augmented. Return `false` otherwise. + * @internal + */ +export type IsRouteMetaBase = _RouteMetaBase extends RM ? true : false +/** + * Returns `true` if the passed `RouteMeta` type has been augmented with required fields. Return `false` otherwise. + * @internal + */ +export type IsRouteMetaRequired = Partial extends RM ? false : true + +export type _RouteRecordBaseMeta = IsRouteMetaRequired extends true + ? { + /** + * Arbitrary data attached to the record. Required because the `RouteMeta` type has been augmented with required + * fields. + */ + meta: RouteMeta + } + : { + /** + * Arbitrary data attached to the record. + */ + meta?: RouteMeta + } /** * Route Record defining one single component with the `component` option. diff --git a/packages/router/test-dts/components.test-d.tsx b/packages/router/test-dts/components.test-d.tsx index 532f66bac..07c270520 100644 --- a/packages/router/test-dts/components.test-d.tsx +++ b/packages/router/test-dts/components.test-d.tsx @@ -5,27 +5,32 @@ import { createRouter, createMemoryHistory, } from './index' -import { expectTypeOf } from 'vitest' +import { it, describe, expectTypeOf } from 'vitest' -let router = createRouter({ - history: createMemoryHistory(), - routes: [], -}) +describe('Components', () => { + let router = createRouter({ + history: createMemoryHistory(), + routes: [], + }) -// RouterLink -// @ts-expect-error missing to -expectError() -// @ts-expect-error: invalid prop -expectError() -// @ts-expect-error: invalid prop -expectError() -expectTypeOf() -expectTypeOf() -expectTypeOf() -expectTypeOf() -expectTypeOf() + // TODO: split into multiple tests + it('works', () => { + // RouterLink + // @ts-expect-error missing to + expectError() + // @ts-expect-error: invalid prop + expectError() + // @ts-expect-error: invalid prop + expectError() + expectTypeOf() + expectTypeOf() + expectTypeOf() + expectTypeOf() + expectTypeOf() -// RouterView -expectTypeOf() -expectTypeOf() -expectTypeOf() + // RouterView + expectTypeOf() + expectTypeOf() + expectTypeOf() + }) +}) diff --git a/packages/router/test-dts/legacy.test-d.ts b/packages/router/test-dts/legacy.test-d.ts index 8f129a7e6..3b5f70b57 100644 --- a/packages/router/test-dts/legacy.test-d.ts +++ b/packages/router/test-dts/legacy.test-d.ts @@ -1,12 +1,32 @@ -import { expectTypeOf } from 'vitest' -import { Router, RouteLocationNormalizedLoaded } from './index' +import { describe, expectTypeOf, it } from 'vitest' +import { + useRouter, + useRoute, + // rename types for better error messages, otherwise they have the same name + // RouteLocationNormalizedLoadedTyped as I_RLNLT +} from './index' import { defineComponent } from 'vue' -defineComponent({ - methods: { - doStuff() { - expectTypeOf(this.$router) - expectTypeOf(this.$route) - }, - }, +describe('Instance types', () => { + it('creates a $route instance property', () => { + defineComponent({ + methods: { + doStuff() { + // TODO: can't do a proper check because of typed routes + expectTypeOf(this.$route.params).toMatchTypeOf(useRoute().params) + }, + }, + }) + }) + + it('creates $router instance properties', () => { + defineComponent({ + methods: { + doStuff() { + // TODO: can't do a proper check because of typed routes + expectTypeOf(this.$router.back).toEqualTypeOf(useRouter().back) + }, + }, + }) + }) }) diff --git a/packages/router/test-dts/meta.test-d.ts b/packages/router/test-dts/meta.test-d.ts index 32e2010fa..95aa46101 100644 --- a/packages/router/test-dts/meta.test-d.ts +++ b/packages/router/test-dts/meta.test-d.ts @@ -4,10 +4,11 @@ import { describe, it, expectTypeOf } from 'vitest' const component = defineComponent({}) -declare module './index' { +declare module '.' { interface RouteMeta { requiresAuth?: boolean - nested: { foo: string } + // TODO: it would be nice to be able to test required meta without polluting all tests + nested?: { foo: string } } } @@ -27,14 +28,18 @@ describe('RouteMeta', () => { }, }, }, - { - path: '/foo', - component, - // @ts-expect-error - meta: {}, - }, ], }) + + router.addRoute({ + path: '/foo', + component, + meta: { + nested: { + foo: 'foo', + }, + }, + }) }) it('route location in guards', () => { @@ -43,9 +48,12 @@ describe('RouteMeta', () => { routes: [], }) router.beforeEach(to => { - expectTypeOf<{ requiresAuth?: Boolean; nested: { foo: string } }>(to.meta) + expectTypeOf<{ requiresAuth?: Boolean; nested?: { foo: string } }>( + to.meta + ) expectTypeOf(to.meta.lol) - if (to.meta.nested.foo == 'foo' || to.meta.lol) return false + if (to.meta.nested?.foo == 'foo' || to.meta.lol) return false + return }) }) }) diff --git a/packages/router/test-dts/navigationGuards.test-d.ts b/packages/router/test-dts/navigationGuards.test-d.ts index 6bf24ed8a..86f4a0c4a 100644 --- a/packages/router/test-dts/navigationGuards.test-d.ts +++ b/packages/router/test-dts/navigationGuards.test-d.ts @@ -1,4 +1,4 @@ -import { expectTypeOf } from 'vitest' +import { expectTypeOf, describe, it } from 'vitest' import { createRouter, createWebHistory, @@ -14,44 +14,49 @@ const router = createRouter({ routes: [], }) -router.beforeEach((to, from) => { - return { path: '/' } -}) - -router.beforeEach((to, from) => { - return '/' -}) - -router.beforeEach((to, from) => { - return false -}) - -router.beforeEach((to, from, next) => { - next(undefined) -}) - -// @ts-expect-error -router.beforeEach((to, from, next) => { - return Symbol('not supported') -}) -// @ts-expect-error -router.beforeEach(() => { - return Symbol('not supported') -}) - -router.beforeEach((to, from, next) => { - // @ts-expect-error - next(Symbol('not supported')) -}) - -router.afterEach((to, from, failure) => { - expectTypeOf(failure) - if (isNavigationFailure(failure)) { - expectTypeOf(failure.from) - expectTypeOf(failure.to) - } - if (isNavigationFailure(failure, NavigationFailureType.cancelled)) { - expectTypeOf(failure.from) - expectTypeOf(failure.to) - } +describe('Navigation guards', () => { + // TODO: split into multiple tests + it('works', () => { + router.beforeEach((to, from) => { + return { path: '/' } + }) + + router.beforeEach((to, from) => { + return '/' + }) + + router.beforeEach((to, from) => { + return false + }) + + router.beforeEach((to, from, next) => { + next(undefined) + }) + + // @ts-expect-error + router.beforeEach((to, from, next) => { + return Symbol('not supported') + }) + // @ts-expect-error + router.beforeEach(() => { + return Symbol('not supported') + }) + + router.beforeEach((to, from, next) => { + // @ts-expect-error + next(Symbol('not supported')) + }) + + router.afterEach((to, from, failure) => { + expectTypeOf(failure) + if (isNavigationFailure(failure)) { + expectTypeOf(failure.from) + expectTypeOf(failure.to) + } + if (isNavigationFailure(failure, NavigationFailureType.cancelled)) { + expectTypeOf(failure.from) + expectTypeOf(failure.to) + } + }) + }) }) diff --git a/packages/router/test-dts/routeRecords.test-d.ts b/packages/router/test-dts/routeRecords.test-d.ts index e806dad1c..86cac2e68 100644 --- a/packages/router/test-dts/routeRecords.test-d.ts +++ b/packages/router/test-dts/routeRecords.test-d.ts @@ -1,4 +1,5 @@ -import { RouteRecordRaw } from './index' +import { describe, it } from 'vitest' +import type { RouteLocationNormalized, RouteRecordRaw } from './index' import { defineComponent } from 'vue' const component = defineComponent({}) @@ -6,56 +7,72 @@ const components = { default: component } const routes: RouteRecordRaw[] = [] -routes.push({ path: '/', redirect: '/foo' }) +describe('RouteRecords', () => { + // TODO: split into multiple tests + it('works', () => { + routes.push({ path: '/', redirect: '/foo' }) -// @ts-expect-error cannot have components and component at the same time -routes.push({ path: '/', components, component }) + // @ts-expect-error cannot have components and component at the same time + routes.push({ path: '/', components, component }) -// a redirect record with children to point to a child -routes.push({ - path: '/', - redirect: '/foo', - children: [ - { - path: 'foo', + // a redirect record with children to point to a child + routes.push({ + path: '/', + redirect: '/foo', + children: [ + { + path: 'foo', + component, + }, + ], + }) + + // same but with a nested route + routes.push({ + path: '/', component, - }, - ], -}) + redirect: '/foo', + children: [ + { + path: 'foo', + component, + }, + ], + }) -// same but with a nested route -routes.push({ - path: '/', - component, - redirect: '/foo', - children: [ - { - path: 'foo', + routes.push({ path: '/a/b', component, props: true }) + routes.push({ + path: '/a/b', component, - }, - ], -}) + props: (to: RouteLocationNormalized<'/[id]+'>) => to.params.id, + }) + // @ts-expect-error: props should be an object + routes.push({ path: '/a/b', components, props: to => to.params.id }) + routes.push({ + path: '/a/b', + components, + props: { + default: (to: RouteLocationNormalized<'/[id]+'>) => to.params.id, + }, + }) + routes.push({ path: '/', components, props: true }) -routes.push({ path: '/', component, props: true }) -routes.push({ path: '/', component, props: to => to.params.id }) -// @ts-expect-error: props should be an object -routes.push({ path: '/', components, props: to => to.params.id }) -routes.push({ path: '/', components, props: { default: to => to.params.id } }) -routes.push({ path: '/', components, props: true }) - -// let r: RouteRecordRaw = { -// path: '/', -// component, -// components, -// } - -export function filterNestedChildren(children: RouteRecordRaw[]) { - return children.filter(r => { - if (r.redirect) { - r.children?.map(() => {}) - } - if (r.children) { - r.children = filterNestedChildren(r.children) + // let r: RouteRecordRaw = { + // path: '/', + // component, + // components, + // } + + function filterNestedChildren(children: RouteRecordRaw[]) { + return children.filter(r => { + if (r.redirect) { + r.children?.map(() => {}) + } + if (r.children) { + r.children = filterNestedChildren(r.children) + } + }) } + filterNestedChildren(routes) }) -} +}) diff --git a/packages/router/test-dts/typed-routes.test-d.ts b/packages/router/test-dts/typed-routes.test-d.ts index e12bdab2a..7f6849818 100644 --- a/packages/router/test-dts/typed-routes.test-d.ts +++ b/packages/router/test-dts/typed-routes.test-d.ts @@ -10,7 +10,7 @@ import { // type is needed instead of an interface // https://github.com/microsoft/TypeScript/issues/15300 -type RouteMap = { +export type RouteMap = { '/[...path]': RouteRecordInfo< '/[...path]', '/:path(.*)', diff --git a/packages/router/tsconfig.json b/packages/router/tsconfig.json index 22219c6f0..41fc6c378 100644 --- a/packages/router/tsconfig.json +++ b/packages/router/tsconfig.json @@ -2,7 +2,8 @@ "include": [ "src/global.d.ts", "src/**/*.ts", - "__tests__/**/*.ts" + "__tests__/**/*.ts", + "test-dts/**/*.ts" ], "compilerOptions": { "baseUrl": ".", @@ -12,7 +13,7 @@ "noEmit": true, "target": "esnext", "module": "esnext", - "moduleResolution": "node", + "moduleResolution": "Bundler", "allowJs": false, "noUnusedLocals": true, "strictNullChecks": true, diff --git a/packages/router/vitest.config.ts b/packages/router/vitest.config.ts index bb42313f7..1e60c7351 100644 --- a/packages/router/vitest.config.ts +++ b/packages/router/vitest.config.ts @@ -32,7 +32,7 @@ export default defineConfig({ checker: 'vue-tsc', // only: true, // by default it includes all specs too - include: ['**/*.test-d.ts'], + // include: ['**/*.test-d.ts'], // tsconfig: './tsconfig.typecheck.json', },