From 666903cc3df22e3f5821f9a5d01d276794260d93 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Wed, 5 Feb 2020 13:12:09 +0100 Subject: [PATCH] refactor: remove redirect normalized record --- __tests__/matcher/resolve.spec.ts | 3 +- src/matcher/index.ts | 55 +++-------- src/matcher/path-matcher.ts | 4 +- src/matcher/types.ts | 24 +---- src/router.ts | 147 ++++++------------------------ src/types/index.ts | 2 + 6 files changed, 53 insertions(+), 182 deletions(-) diff --git a/__tests__/matcher/resolve.spec.ts b/__tests__/matcher/resolve.spec.ts index bb412d6a..0de5afe6 100644 --- a/__tests__/matcher/resolve.spec.ts +++ b/__tests__/matcher/resolve.spec.ts @@ -375,7 +375,8 @@ describe('Router Matcher', () => { ) }) - describe('redirects', () => { + // TODO: replace tests with a transformation check to the `beforeEnter` guard + describe.skip('redirects', () => { function assertRedirect( records: RouteRecord[], location: MatcherLocation, diff --git a/src/matcher/index.ts b/src/matcher/index.ts index 75278b9f..a225c58c 100644 --- a/src/matcher/index.ts +++ b/src/matcher/index.ts @@ -2,7 +2,7 @@ import { RouteRecord, MatcherLocation, MatcherLocationNormalized, - MatcherLocationRedirect, + RouteRecordRedirect, } from '../types' import { NoRouteMatchError, InvalidRouteMatch } from '../errors' import { createRouteRecordMatcher, RouteRecordMatcher } from './path-matcher' @@ -19,7 +19,7 @@ interface RouterMatcher { resolve: ( location: Readonly, currentLocation: Readonly - ) => MatcherLocationNormalized | MatcherLocationRedirect + ) => MatcherLocationNormalized } const TRAILING_SLASH_RE = /(.)\/+$/ @@ -47,21 +47,22 @@ export function createRouterMatcher( routes: RouteRecord[], globalOptions: PathParserOptions ): RouterMatcher { + // normalized ordered array of matchers const matchers: RouteRecordMatcher[] = [] function addRoute( record: Readonly, parent?: RouteRecordMatcher ): void { - const mainNormalizedRecord: RouteRecordNormalized = normalizeRouteRecord( - record - ) + const mainNormalizedRecord = normalizeRouteRecord(record) const options: PathParserOptions = { ...globalOptions, ...record.options } // TODO: can probably be removed now that we have our own parser and we handle this correctly if (!options.strict) mainNormalizedRecord.path = removeTrailingSlash(mainNormalizedRecord.path) // generate an array of records to correctly handle aliases - const normalizedRecords: RouteRecordNormalized[] = [mainNormalizedRecord] + const normalizedRecords: Array< + RouteRecordNormalized | RouteRecordRedirect + > = [mainNormalizedRecord] if ('alias' in record && record.alias) { const aliases = typeof record.alias === 'string' ? [record.alias] : record.alias @@ -130,7 +131,7 @@ export function createRouterMatcher( function resolve( location: Readonly, currentLocation: Readonly - ): MatcherLocationNormalized | MatcherLocationRedirect { + ): MatcherLocationNormalized { let matcher: RouteRecordMatcher | undefined let params: PathParams = {} let path: MatcherLocationNormalized['path'] @@ -147,20 +148,6 @@ export function createRouterMatcher( // params are automatically encoded // TODO: try catch to provide better error messages path = matcher.stringify(params) - - if ('redirect' in matcher.record) { - const { redirect } = matcher.record - return { - redirect, - normalizedLocation: { - name, - path, - matched: [], - params, - meta: matcher.record.meta || {}, - }, - } - } } else if ('path' in location) { matcher = matchers.find(m => m.re.test(location.path)) // matcher should have a value after the loop @@ -177,20 +164,6 @@ export function createRouterMatcher( path = location.path name = matcher.record.name - if ('redirect' in matcher.record) { - const { redirect } = matcher.record - return { - redirect, - normalizedLocation: { - name, - path, - // TODO: verify this is good or add a comment - matched: [], - params, - meta: matcher.record.meta || {}, - }, - } - } // location is a relative path } else { // match by name or path of current route @@ -242,16 +215,10 @@ export function createRouterMatcher( */ export function normalizeRouteRecord( record: Readonly -): RouteRecordNormalized { +): RouteRecordNormalized | RouteRecordRedirect { if ('redirect' in record) { - return { - path: record.path, - redirect: record.redirect, - name: record.name, - beforeEnter: record.beforeEnter, - meta: record.meta, - leaveGuards: [], - } + // TODO: transform redirect into beforeEnter and remove type above + return record } else { return { path: record.path, diff --git a/src/matcher/path-matcher.ts b/src/matcher/path-matcher.ts index fdbb10b2..e8e1a53f 100644 --- a/src/matcher/path-matcher.ts +++ b/src/matcher/path-matcher.ts @@ -5,6 +5,7 @@ import { PathParserOptions, } from './path-parser-ranker' import { tokenizePath } from './path-tokenizer' +import { RouteRecordRedirect } from '../types' export interface RouteRecordMatcher extends PathParser { record: RouteRecordNormalized @@ -14,7 +15,7 @@ export interface RouteRecordMatcher extends PathParser { } export function createRouteRecordMatcher( - record: Readonly, + record: Readonly, parent: RouteRecordMatcher | undefined, options?: PathParserOptions ): RouteRecordMatcher { @@ -22,6 +23,7 @@ export function createRouteRecordMatcher( return { ...parser, + // @ts-ignore: TODO: adapt tokenstoparser record, parent, } diff --git a/src/matcher/types.ts b/src/matcher/types.ts index 82a45aa0..cf44035e 100644 --- a/src/matcher/types.ts +++ b/src/matcher/types.ts @@ -1,30 +1,16 @@ -import { - RouteRecordMultipleViews, - RouteRecordRedirect, - NavigationGuard, -} from '../types' +import { RouteRecordMultipleViews, NavigationGuard } from '../types' export interface RouteRecordNormalizedCommon { leaveGuards: NavigationGuard[] } -type RouteRecordRedirectNormalized = RouteRecordNormalizedCommon & - Pick< - RouteRecordRedirect, - 'path' | 'name' | 'redirect' | 'beforeEnter' | 'meta' - > - -type RouteRecordViewNormalized = RouteRecordNormalizedCommon & +// TODO: rename or refactor the duplicated type +// normalize component/components into components +export type RouteRecordNormalized = RouteRecordNormalizedCommon & Pick< RouteRecordMultipleViews, 'path' | 'name' | 'components' | 'children' | 'meta' | 'beforeEnter' > -// normalize component/components into components -// How are RouteRecords stored in a matcher -export type RouteRecordNormalized = - | RouteRecordRedirectNormalized - | RouteRecordViewNormalized - // When Matching a location, only RouteRecordView is possible, because redirections never end up in `matched` -export type RouteRecordMatched = RouteRecordViewNormalized +export type RouteRecordMatched = RouteRecordNormalized diff --git a/src/router.ts b/src/router.ts index b7f8409f..62a93a32 100644 --- a/src/router.ts +++ b/src/router.ts @@ -10,7 +10,6 @@ import { TODO, Immutable, RouteParams, - MatcherLocationNormalized, } from './types' import { RouterHistory, @@ -70,8 +69,7 @@ export interface Router { beforeEach(guard: NavigationGuard): ListenerRemover afterEach(guard: PostNavigationGuard): ListenerRemover - // TODO: also return a ListenerRemover - onError(handler: ErrorHandler): void + onError(handler: ErrorHandler): ListenerRemover isReady(): Promise install(app: App): void @@ -79,33 +77,6 @@ export interface Router { const isClient = typeof window !== 'undefined' -async function runGuardQueue(guards: Lazy[]): Promise { - for (const guard of guards) { - await guard() - } -} - -function extractChangingRecords( - to: RouteLocationNormalized, - from: RouteLocationNormalized -) { - const leavingRecords: RouteRecordMatched[] = [] - const updatingRecords: RouteRecordMatched[] = [] - const enteringRecords: RouteRecordMatched[] = [] - - // TODO: could be optimized with one single for loop - for (const record of from.matched) { - if (to.matched.indexOf(record) < 0) leavingRecords.push(record) - else updatingRecords.push(record) - } - - for (const record of to.matched) { - if (from.matched.indexOf(record) < 0) enteringRecords.push(record) - } - - return [leavingRecords, updatingRecords, enteringRecords] -} - export function createRouter({ history, routes, @@ -168,12 +139,11 @@ export function createRouter({ ): RouteLocationNormalized { // const objectLocation = routerLocationAsObject(location) if (typeof location === 'string') { - // TODO: remove as cast when redirect is removed from matcher let locationNormalized = parseURL(parseQuery, location) let matchedRoute = matcher.resolve( { path: locationNormalized.path }, currentLocation - ) as MatcherLocationNormalized + ) return { ...locationNormalized, @@ -187,14 +157,13 @@ export function createRouter({ // relative or named location, path is ignored // for same reason TS thinks location.params can be undefined - // TODO: remove cast like above let matchedRoute = matcher.resolve( hasParams ? // we know we have the params attribute { ...location, params: encodeParams((location as any).params) } : location, currentLocation - ) as MatcherLocationNormalized + ) // put back the unencoded params as given by the user (avoid the cost of decoding them) matchedRoute.params = hasParams @@ -214,87 +183,6 @@ export function createRouter({ } } - // function oldresolveLocation( - // location: MatcherLocation & Required, - // currentLocation?: RouteLocationNormalized, - // redirectedFrom?: RouteLocationNormalized - // // ensure when returning that the redirectedFrom is a normalized location - // ): RouteLocationNormalized { - // currentLocation = currentLocation || currentRoute.value - // // TODO: still return a normalized location with no matched records if no location is found - // const matchedRoute = matcher.resolve(location, currentLocation) - - // if ('redirect' in matchedRoute) { - // const { redirect } = matchedRoute - // // target location normalized, used if we want to redirect again - // const normalizedLocation: RouteLocationNormalized = { - // ...matchedRoute.normalizedLocation, - // ...normalizeLocation({ - // path: matchedRoute.normalizedLocation.path, - // query: location.query, - // hash: location.hash, - // }), - // redirectedFrom, - // meta: {}, - // } - - // if (typeof redirect === 'string') { - // // match the redirect instead - // return resolveLocation( - // normalizeLocation(redirect), - // currentLocation, - // normalizedLocation - // ) - // } else if (typeof redirect === 'function') { - // const newLocation = redirect(normalizedLocation) - - // if (typeof newLocation === 'string') { - // return resolveLocation( - // normalizeLocation(newLocation), - // currentLocation, - // normalizedLocation - // ) - // } - - // // TODO: should we allow partial redirects? I think we should not because it's impredictable if - // // there was a redirect before - // // if (!('path' in newLocation) && !('name' in newLocation)) throw new Error('TODO: redirect canot be relative') - - // return resolveLocation( - // { - // ...newLocation, - // query: newLocation.query || {}, - // hash: newLocation.hash || '', - // }, - // currentLocation, - // normalizedLocation - // ) - // } else { - // return resolveLocation( - // { - // ...redirect, - // query: redirect.query || {}, - // hash: redirect.hash || '', - // }, - // currentLocation, - // normalizedLocation - // ) - // } - // } else { - // // add the redirectedFrom field - // const url = normalizeLocation({ - // path: matchedRoute.path, - // query: location.query, - // hash: location.hash, - // }) - // return { - // ...matchedRoute, - // ...url, - // redirectedFrom, - // } - // } - // } - async function push(to: RouteLocation): Promise { const toLocation: RouteLocationNormalized = (pendingLocation = resolve(to)) @@ -313,7 +201,6 @@ export function createRouter({ if (NavigationGuardRedirect.is(error)) { // push was called while waiting in guards if (pendingLocation !== toLocation) { - // TODO: trigger onError as well triggerError(new NavigationCancelled(toLocation, currentRoute.value)) } // TODO: setup redirect stack @@ -321,7 +208,6 @@ export function createRouter({ return push(error.to) } else { // TODO: write tests - // triggerError as well if (pendingLocation !== toLocation) { // TODO: trigger onError as well triggerError(new NavigationCancelled(toLocation, currentRoute.value)) @@ -610,3 +496,30 @@ function applyRouterPlugin(app: App, router: Router) { app.provide('router', router) app.provide('route', router.currentRoute) } + +async function runGuardQueue(guards: Lazy[]): Promise { + for (const guard of guards) { + await guard() + } +} + +function extractChangingRecords( + to: RouteLocationNormalized, + from: RouteLocationNormalized +) { + const leavingRecords: RouteRecordMatched[] = [] + const updatingRecords: RouteRecordMatched[] = [] + const enteringRecords: RouteRecordMatched[] = [] + + // TODO: could be optimized with one single for loop + for (const record of from.matched) { + if (to.matched.indexOf(record) < 0) leavingRecords.push(record) + else updatingRecords.push(record) + } + + for (const record of to.matched) { + if (from.matched.indexOf(record) < 0) enteringRecords.push(record) + } + + return [leavingRecords, updatingRecords, enteringRecords] +} diff --git a/src/types/index.ts b/src/types/index.ts index ed70d9aa..cbb6c489 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -107,9 +107,11 @@ export interface RouteRecordCommon { path: string alias?: string | string[] name?: string + // TODO: beforeEnter has no effect with redirect, move and test beforeEnter?: NavigationGuard | NavigationGuard[] meta?: Record // TODO: only allow a subset? + // TODO: RFC: remove this and only allow global options options?: PathParserOptions } -- 2.39.5