feat(dx): warn when passing undefined/null locations

skirtles-code · skirtles-code · commit 0b483f9308a5 · 2024-03-03T02:18:16.000Z
diff --git a/packages/router/__tests__/router.spec.ts b/packages/router/__tests__/router.spec.ts
@@ -330,6 +330,22 @@ describe('Router', () => {
     expect(route1.params).toEqual({ p: 'a' })
   })
 
+  it('handles an undefined location', async () => {
+    const { router } = await newRouter()
+
+    const route1 = router.resolve(undefined as any)
+    expect('router.resolve() was passed an invalid location').toHaveBeenWarned()
+    expect(route1.path).toBe('/')
+  })
+
+  it('handles a null location', async () => {
+    const { router } = await newRouter()
+
+    const route1 = router.resolve(null as any)
+    expect('router.resolve() was passed an invalid location').toHaveBeenWarned()
+    expect(route1.path).toBe('/')
+  })
+
   it('removes null/undefined optional params when current location has it', async () => {
     const { router } = await newRouter()
 
diff --git a/packages/router/__tests__/useLink.spec.ts b/packages/router/__tests__/useLink.spec.ts
@@ -0,0 +1,148 @@
+/**
+ * @jest-environment jsdom
+ */
+import { nextTick, ref } from 'vue'
+import { mount } from '@vue/test-utils'
+import { mockWarn } from 'jest-mock-warn'
+import {
+  createMemoryHistory,
+  createRouter,
+  routeLocationKey,
+  RouteLocationRaw,
+  routerKey,
+  useLink,
+  UseLinkOptions,
+} from '../src'
+
+async function callUseLink(args: UseLinkOptions) {
+  const router = createRouter({
+    history: createMemoryHistory(),
+    routes: [
+      {
+        path: '/',
+        component: {},
+        name: 'root',
+      },
+      {
+        path: '/a',
+        component: {},
+        name: 'a',
+      },
+      {
+        path: '/b',
+        component: {},
+        name: 'b',
+      },
+    ],
+  })
+
+  await router.push('/')
+
+  let link: ReturnType<typeof useLink>
+
+  mount(
+    {
+      setup() {
+        link = useLink(args)
+
+        return () => ''
+      },
+    },
+    {
+      global: {
+        provide: {
+          [routerKey as any]: router,
+          [routeLocationKey as any]: router.currentRoute.value,
+        },
+      },
+    }
+  )
+
+  return link!
+}
+
+describe('useLink', () => {
+  describe('basic usage', () => {
+    it('supports a string for "to"', async () => {
+      const { href, route } = await callUseLink({
+        to: '/a',
+      })
+
+      expect(href.value).toBe('/a')
+      expect(route.value).toMatchObject({ name: 'a' })
+    })
+
+    it('supports an object for "to"', async () => {
+      const { href, route } = await callUseLink({
+        to: { path: '/a' },
+      })
+
+      expect(href.value).toBe('/a')
+      expect(route.value).toMatchObject({ name: 'a' })
+    })
+
+    it('supports a ref for "to"', async () => {
+      const to = ref<RouteLocationRaw>('/a')
+
+      const { href, route } = await callUseLink({
+        to,
+      })
+
+      expect(href.value).toBe('/a')
+      expect(route.value).toMatchObject({ name: 'a' })
+
+      to.value = { path: '/b' }
+
+      await nextTick()
+
+      expect(href.value).toBe('/b')
+      expect(route.value).toMatchObject({ name: 'b' })
+    })
+  })
+
+  describe('warnings', () => {
+    mockWarn()
+
+    it('should warn when "to" is undefined', async () => {
+      await callUseLink({
+        to: undefined as any,
+      })
+
+      expect('Invalid value for prop "to" in useLink()').toHaveBeenWarned()
+      expect(
+        'router.resolve() was passed an invalid location'
+      ).toHaveBeenWarned()
+    })
+
+    it('should warn when "to" is an undefined ref', async () => {
+      await callUseLink({
+        to: ref(undefined as any),
+      })
+
+      expect('Invalid value for prop "to" in useLink()').toHaveBeenWarned()
+      expect(
+        'router.resolve() was passed an invalid location'
+      ).toHaveBeenWarned()
+    })
+
+    it('should warn when "to" changes to a null ref', async () => {
+      const to = ref('/a')
+
+      const { href, route } = await callUseLink({
+        to,
+      })
+
+      expect(href.value).toBe('/a')
+      expect(route.value).toMatchObject({ name: 'a' })
+
+      to.value = null as any
+
+      await nextTick()
+
+      expect('Invalid value for prop "to" in useLink()').toHaveBeenWarned()
+      expect(
+        'router.resolve() was passed an invalid location'
+      ).toHaveBeenWarned()
+    })
+  })
+})
diff --git a/packages/router/src/RouterLink.ts b/packages/router/src/RouterLink.ts
@@ -36,7 +36,8 @@ import { isSameRouteLocationParams, isSameRouteRecord } from './location'
 import { routerKey, routeLocationKey } from './injectionSymbols'
 import { RouteRecord } from './matcher/types'
 import { NavigationFailure } from './errors'
-import { isArray, isBrowser, noop } from './utils'
+import { isArray, isBrowser, isObject, noop } from './utils'
+import { warn } from './warning'
 
 export interface RouterLinkOptions {
   /**
@@ -83,6 +84,7 @@ export interface UseLinkDevtoolsContext {
   route: RouteLocationNormalized & { href: string }
   isActive: boolean
   isExactActive: boolean
+  error: string
 }
 
 export type UseLinkOptions = VueUseOptions<RouterLinkOptions>
@@ -93,7 +95,40 @@ export function useLink(props: UseLinkOptions) {
   const router = inject(routerKey)!
   const currentRoute = inject(routeLocationKey)!
 
-  const route = computed(() => router.resolve(unref(props.to)))
+  const isValidTo = (to: unknown) => typeof to === 'string' || isObject(to)
+  let hasPrevious = false
+  let previousTo: unknown = null
+
+  const route = computed(() => {
+    const to = unref(props.to)
+
+    if (__DEV__ && (!hasPrevious || to !== previousTo)) {
+      if (!isValidTo(to)) {
+        if (hasPrevious) {
+          warn(
+            `Invalid value for prop "to" in useLink()\n- to:`,
+            to,
+            `\n- previous to:`,
+            previousTo,
+            `\n- props:`,
+            props
+          )
+        } else {
+          warn(
+            `Invalid value for prop "to" in useLink()\n- to:`,
+            to,
+            `\n- props:`,
+            props
+          )
+        }
+      }
+
+      previousTo = to
+      hasPrevious = true
+    }
+
+    return router.resolve(to)
+  })
 
   const activeRecordIndex = computed<number>(() => {
     const { matched } = route.value
@@ -157,6 +192,7 @@ export function useLink(props: UseLinkOptions) {
         route: route.value,
         isActive: isActive.value,
         isExactActive: isExactActive.value,
+        error: '',
       }
 
       // @ts-expect-error: this is internal
@@ -168,6 +204,9 @@ export function useLink(props: UseLinkOptions) {
           linkContextDevtools.route = route.value
           linkContextDevtools.isActive = isActive.value
           linkContextDevtools.isExactActive = isExactActive.value
+          linkContextDevtools.error = isValidTo(unref(props.to))
+            ? ''
+            : 'Invalid "to" value'
         },
         { flush: 'post' }
       )
diff --git a/packages/router/src/devtools.ts b/packages/router/src/devtools.ts
@@ -119,10 +119,16 @@ export function addDevtools(app: App, router: Router, matcher: RouterMatcher) {
           ;(
             componentInstance.__vrl_devtools as UseLinkDevtoolsContext[]
           ).forEach(devtoolsData => {
+            let label = devtoolsData.route.path
             let backgroundColor = ORANGE_400
             let tooltip: string = ''
+            let textColor = 0
 
-            if (devtoolsData.isExactActive) {
+            if (devtoolsData.error) {
+              label = devtoolsData.error
+              backgroundColor = RED_100
+              textColor = RED_700
+            } else if (devtoolsData.isExactActive) {
               backgroundColor = LIME_500
               tooltip = 'This is exactly active'
             } else if (devtoolsData.isActive) {
@@ -131,8 +137,8 @@ export function addDevtools(app: App, router: Router, matcher: RouterMatcher) {
             }
 
             node.tags.push({
-              label: devtoolsData.route.path,
-              textColor: 0,
+              label,
+              textColor,
               tooltip,
               backgroundColor,
             })
@@ -419,6 +425,8 @@ const CYAN_400 = 0x22d3ee
 const ORANGE_400 = 0xfb923c
 // const GRAY_100 = 0xf4f4f5
 const DARK = 0x666666
+const RED_100 = 0xfee2e2
+const RED_700 = 0xb91c1c
 
 function formatRouteRecordForInspector(
   route: RouteRecordMatcher
diff --git a/packages/router/src/router.ts b/packages/router/src/router.ts
@@ -32,7 +32,14 @@ import {
   NavigationRedirectError,
   isNavigationFailure,
 } from './errors'
-import { applyToParams, isBrowser, assign, noop, isArray } from './utils'
+import {
+  applyToParams,
+  isBrowser,
+  assign,
+  noop,
+  isArray,
+  isObject,
+} from './utils'
 import { useCallbacks } from './utils/callbacks'
 import { encodeParam, decode, encodeHash } from './encoding'
 import {
@@ -460,6 +467,14 @@ export function createRouter(options: RouterOptions): Router {
       })
     }
 
+    if (__DEV__ && !isObject(rawLocation)) {
+      warn(
+        `router.resolve() was passed an invalid location. This will fail in production.\n- Location:`,
+        rawLocation
+      )
+      rawLocation = {}
+    }
+
     let matcherLocation: MatcherLocationRaw
 
     // path could be relative in object as well
diff --git a/packages/router/src/utils/index.ts b/packages/router/src/utils/index.ts
@@ -37,3 +37,6 @@ export const noop = () => {}
  */
 export const isArray: (arg: ArrayLike<any> | any) => arg is ReadonlyArray<any> =
   Array.isArray
+
+export const isObject = (val: unknown): val is Record<any, any> =>
+  val !== null && typeof val === 'object'
