Skip to content

Commit

Permalink
feat(dx): warn when passing undefined/null locations
Browse files Browse the repository at this point in the history
  • Loading branch information
skirtles-code committed Mar 3, 2024
1 parent 13303bd commit 0b483f9
Show file tree
Hide file tree
Showing 6 changed files with 235 additions and 6 deletions.
16 changes: 16 additions & 0 deletions packages/router/__tests__/router.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
148 changes: 148 additions & 0 deletions packages/router/__tests__/useLink.spec.ts
Original file line number Diff line number Diff line change
@@ -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()
})
})
})
43 changes: 41 additions & 2 deletions packages/router/src/RouterLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -83,6 +84,7 @@ export interface UseLinkDevtoolsContext {
route: RouteLocationNormalized & { href: string }
isActive: boolean
isExactActive: boolean
error: string
}

export type UseLinkOptions = VueUseOptions<RouterLinkOptions>
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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' }
)
Expand Down
14 changes: 11 additions & 3 deletions packages/router/src/devtools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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,
})
Expand Down Expand Up @@ -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
Expand Down
17 changes: 16 additions & 1 deletion packages/router/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions packages/router/src/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

0 comments on commit 0b483f9

Please sign in to comment.