Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor error with enum codes #123

Merged
merged 16 commits into from
Mar 18, 2020
15 changes: 10 additions & 5 deletions __tests__/errors.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { createRouter as newRouter, createMemoryHistory } from '../src'
import { NavigationAborted, NavigationGuardRedirect } from '../src/errors'
import { ErrorTypes } from '../src/errors-new'
import { components, tick } from './utils'
import { RouteRecord } from '../src/types'

Expand Down Expand Up @@ -47,9 +47,11 @@ describe('Errors', () => {
try {
await router.push('/foo')
} catch (err) {
expect(err).toBeInstanceOf(NavigationAborted)
expect(err.type).toBe(ErrorTypes.NAVIGATION_ABORTED)
}
expect(onError).toHaveBeenCalledWith(expect.any(NavigationAborted))
expect(onError).toHaveBeenCalledWith(
expect.objectContaining({ type: ErrorTypes.NAVIGATION_ABORTED })
)
})

it('triggers erros caused by new navigations of a next(redirect) trigered by history', async () => {
Expand All @@ -69,12 +71,15 @@ describe('Errors', () => {
expect(onError).toHaveBeenCalledTimes(2)
expect(onError).toHaveBeenNthCalledWith(
1,
expect.any(NavigationGuardRedirect)
expect.objectContaining({ type: ErrorTypes.NAVIGATION_GUARD_REDIRECT })
)
expect(onError.mock.calls[0]).toMatchObject([
{ to: { params: { p: '1' } }, from: { fullPath: '/p/0' } },
])
expect(onError).toHaveBeenNthCalledWith(2, expect.any(NavigationAborted))
expect(onError).toHaveBeenNthCalledWith(
2,
expect.objectContaining({ type: ErrorTypes.NAVIGATION_ABORTED })
)
expect(onError.mock.calls[1]).toMatchObject([
{ to: { params: { p: '1' } }, from: { params: { p: 'other' } } },
])
Expand Down
8 changes: 4 additions & 4 deletions __tests__/matcher/__snapshots__/resolve.spec.ts.snap
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Router Matcher resolve LocationAsName throws if the named route does not exists 1`] = `
[NoRouteMatchError: No match for
{"name":"Home"}]
[Error: No match for
{"name":"Home"}]
`;

exports[`Router Matcher resolve LocationAsRelative throws if the current named route does not exists 1`] = `
[NoRouteMatchError: No match for
{"params":{"a":"foo"}}
[Error: No match for
{"params":{"a":"foo"}}
while being at
{"name":"home","params":{},"path":"/","meta":{}}]
`;
4 changes: 2 additions & 2 deletions __tests__/router.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fakePromise from 'faked-promise'
import { createRouter, createMemoryHistory, createWebHistory } from '../src'
import { NavigationCancelled } from '../src/errors'
import { ErrorTypes } from '../src/errors-new'
import { createDom, components, tick } from './utils'
import {
RouteRecord,
Expand Down Expand Up @@ -226,7 +226,7 @@ describe('Router', () => {
try {
await pA
} catch (err) {
expect(err).toBeInstanceOf(NavigationCancelled)
expect(err.type).toBe(ErrorTypes.NAVIGATION_CANCELLED)
}
expect(router.currentRoute.value.fullPath).toBe('/p/b')
}
Expand Down
92 changes: 92 additions & 0 deletions src/errors-new.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
import {
MatcherLocation,
MatcherLocationNormalized,
RouteLocation,
RouteLocationNormalized,
} from './types'

// Using string enums because error codes are exposed to developers
// and number enums could collide with other error codes in runtime
export enum ErrorTypes {
MATCHER_NOT_FOUND = 'MATCHER_NOT_FOUND',
NAVIGATION_GUARD_REDIRECT = 'NAVIGATION_GUARD_REDIRECT',
NAVIGATION_ABORTED = 'NAVIGATION_ABORTED',
NAVIGATION_CANCELLED = 'NAVIGATION_CANCELLED',
}

interface RouterError extends Error {
type: ErrorTypes
}

interface MatcherError extends RouterError {
type: ErrorTypes.MATCHER_NOT_FOUND
location: MatcherLocation
currentLocation?: MatcherLocationNormalized
}

interface NavigationError extends RouterError {
type:
| ErrorTypes.NAVIGATION_GUARD_REDIRECT
| ErrorTypes.NAVIGATION_ABORTED
| ErrorTypes.NAVIGATION_CANCELLED
from: RouteLocationNormalized
to: RouteLocation | RouteLocationNormalized
fnlctrl marked this conversation as resolved.
Show resolved Hide resolved
}

type InferErrorType<Type extends ErrorTypes> = Type extends MatcherError['type']
? MatcherError
: Type extends NavigationError['type']
? NavigationError
: never

const ErrorTypeMessages = __DEV__
posva marked this conversation as resolved.
Show resolved Hide resolved
? {
[ErrorTypes.MATCHER_NOT_FOUND]({
location,
currentLocation,
}: MatcherError) {
return `No match for\n ${JSON.stringify(location)}${
currentLocation
? '\nwhile being at\n' + JSON.stringify(currentLocation)
: ''
}`
},
[ErrorTypes.NAVIGATION_GUARD_REDIRECT]({ from, to }: NavigationError) {
return `Redirected from "${from.fullPath}" to "${stringifyRoute(
to
)}" via a navigation guard`
},
[ErrorTypes.NAVIGATION_ABORTED]({ from, to }: NavigationError) {
return `Navigation aborted from "${from.fullPath}" to "${stringifyRoute(
to
)}" via a navigation guard`
},
[ErrorTypes.NAVIGATION_CANCELLED]({ from, to }: NavigationError) {
return `Navigation cancelled from "${
from.fullPath
}" to "${stringifyRoute(to)}" with a new \`push\` or \`replace\``
},
}
: undefined

export function createRouterError<Type extends ErrorTypes>(
type: Type,
params: Omit<InferErrorType<Type>, 'type' | keyof Error>
): InferErrorType<Type> {
const message = __DEV__ ? (ErrorTypeMessages as any)[type](params) : undefined
const error = Object.assign(new Error(message), { type }, params)
return (error as unknown) as InferErrorType<Type>
}

const propertiesToLog = ['params', 'query', 'hash'] as const

function stringifyRoute(to: RouteLocation | RouteLocationNormalized): string {
if (typeof to === 'string') return to
if ('fullPath' in to) return to.fullPath
if ('path' in to) return to.path
const location = {} as Record<string, unknown>
for (const key of propertiesToLog) {
if (key in to) location[key] = to[key]
}
return JSON.stringify(location, null, 2)
}
11 changes: 8 additions & 3 deletions src/matcher/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
MatcherLocationNormalized,
ListenerRemover,
} from '../types'
import { NoRouteMatchError } from '../errors'
import { createRouterError, ErrorTypes } from '../errors-new'
import { createRouteRecordMatcher, RouteRecordMatcher } from './path-matcher'
import { RouteRecordNormalized } from './types'
import {
Expand Down Expand Up @@ -188,7 +188,8 @@ export function createRouterMatcher(
if ('name' in location && location.name) {
matcher = matcherMap.get(location.name)

if (!matcher) throw new NoRouteMatchError(location)
if (!matcher)
throw createRouterError(ErrorTypes.MATCHER_NOT_FOUND, { location })

name = matcher.record.name
// TODO: merge params with current location. Should this be done by name. I think there should be some kind of relationship between the records like children of a parent should keep parent props but not the rest
Expand All @@ -214,7 +215,11 @@ export function createRouterMatcher(
matcher = currentLocation.name
? matcherMap.get(currentLocation.name)
: matchers.find(m => m.re.test(currentLocation.path))
if (!matcher) throw new NoRouteMatchError(location, currentLocation)
if (!matcher)
throw createRouterError(ErrorTypes.MATCHER_NOT_FOUND, {
location,
currentLocation,
})
name = matcher.record.name
params = location.params || currentLocation.params
path = matcher.stringify(params)
Expand Down
42 changes: 30 additions & 12 deletions src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,7 @@ import {
scrollToPosition,
} from './utils/scroll'
import { createRouterMatcher } from './matcher'
import {
NavigationCancelled,
NavigationGuardRedirect,
NavigationAborted,
} from './errors'
import { createRouterError, ErrorTypes } from './errors-new'
import {
extractComponentsGuards,
guardToPromiseFn,
Expand Down Expand Up @@ -218,17 +214,27 @@ export function createRouter({
try {
await navigate(toLocation, from)
} catch (error) {
if (NavigationGuardRedirect.is(error)) {
if (error.type === ErrorTypes.NAVIGATION_GUARD_REDIRECT) {
// push was called while waiting in guards
if (pendingLocation !== toLocation) {
triggerError(new NavigationCancelled(toLocation, from))
triggerError(
createRouterError(ErrorTypes.NAVIGATION_CANCELLED, {
from,
to: toLocation,
})
)
}
// preserve the original redirectedFrom if any
return pushWithRedirect(error.to, redirectedFrom || toLocation)
} else {
// TODO: write tests
if (pendingLocation !== toLocation) {
triggerError(new NavigationCancelled(toLocation, from))
triggerError(
createRouterError(ErrorTypes.NAVIGATION_CANCELLED, {
from,
to: toLocation,
})
)
}
}
triggerError(error)
Expand Down Expand Up @@ -345,7 +351,13 @@ export function createRouter({
) {
// a more recent navigation took place
if (pendingLocation !== toLocation) {
return triggerError(new NavigationCancelled(toLocation, from), isPush)
return triggerError(
createRouterError(ErrorTypes.NAVIGATION_CANCELLED, {
from,
to: toLocation,
}),
isPush
)
}

// remove registered guards from removed matched records
Expand Down Expand Up @@ -397,19 +409,25 @@ export function createRouter({
false
)
} catch (error) {
if (NavigationGuardRedirect.is(error)) {
if (error.type === ErrorTypes.NAVIGATION_GUARD_REDIRECT) {
// TODO: refactor the duplication of new NavigationCancelled by
// checking instanceof NavigationError (it's another TODO)
// a more recent navigation took place
if (pendingLocation !== toLocation) {
return triggerError(new NavigationCancelled(toLocation, from), false)
return triggerError(
createRouterError(ErrorTypes.NAVIGATION_CANCELLED, {
from,
to: toLocation,
}),
false
)
}
triggerError(error, false)

// the error is already handled by router.push
// we just want to avoid logging the error
pushWithRedirect(error.to, toLocation).catch(() => {})
} else if (NavigationAborted.is(error)) {
} else if (error.type === ErrorTypes.NAVIGATION_ABORTED) {
console.log('Cancelled, going to', -info.distance)
// TODO: test on different browsers ensure consistent behavior
history.go(-info.distance, false)
Expand Down
12 changes: 9 additions & 3 deletions src/utils/guardToPromiseFn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from '../types'

import { isRouteLocation } from '../types'
import { NavigationGuardRedirect, NavigationAborted } from '../errors'
import { createRouterError, ErrorTypes } from '../errors-new'

export function guardToPromiseFn(
guard: NavigationGuard,
Expand All @@ -19,9 +19,15 @@ export function guardToPromiseFn(
valid?: boolean | RouteLocation
) => {
// TODO: handle callback
if (valid === false) reject(new NavigationAborted(to, from))
if (valid === false)
reject(createRouterError(ErrorTypes.NAVIGATION_ABORTED, { from, to }))
else if (isRouteLocation(valid)) {
reject(new NavigationGuardRedirect(to, valid))
reject(
createRouterError(ErrorTypes.NAVIGATION_GUARD_REDIRECT, {
from: to,
to: valid,
})
)
} else resolve()
}

Expand Down