]> git.ipfire.org Git - thirdparty/vuejs/router.git/commitdiff
refactor: remove redirect normalized record
authorEduardo San Martin Morote <posva13@gmail.com>
Wed, 5 Feb 2020 12:12:09 +0000 (13:12 +0100)
committerEduardo San Martin Morote <posva13@gmail.com>
Wed, 5 Feb 2020 13:09:41 +0000 (14:09 +0100)
__tests__/matcher/resolve.spec.ts
src/matcher/index.ts
src/matcher/path-matcher.ts
src/matcher/types.ts
src/router.ts
src/types/index.ts

index bb412d6a3d378c83ca7f3c0f1769fca4c02b13f9..0de5afe6127b6611c21c4532ca5eaa07806942a8 100644 (file)
@@ -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,
index 75278b9f5f3a79321f4d78f15d722a0ea6cf6285..a225c58ca4a64fb48675c1a08782263cb393023f 100644 (file)
@@ -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<MatcherLocation>,
     currentLocation: Readonly<MatcherLocationNormalized>
-  ) => 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<RouteRecord>,
     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<MatcherLocation>,
     currentLocation: Readonly<MatcherLocationNormalized>
-  ): 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<RouteRecord>
-): 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,
index fdbb10b2f7ffe662a38525f1174d9699c2ada47e..e8e1a53f952d33d08c5cab3d6675b87dc4cec98a 100644 (file)
@@ -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<RouteRecordNormalized>,
+  record: Readonly<RouteRecordNormalized | RouteRecordRedirect>,
   parent: RouteRecordMatcher | undefined,
   options?: PathParserOptions
 ): RouteRecordMatcher {
@@ -22,6 +23,7 @@ export function createRouteRecordMatcher(
 
   return {
     ...parser,
+    // @ts-ignore: TODO: adapt tokenstoparser
     record,
     parent,
   }
index 82a45aa0aff03f8802e75a9c99f6c67fd73fadbf..cf44035ee33bb5e1a95543adb90f56673f154792 100644 (file)
@@ -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
index b7f8409f28834289b962f5156a78027a388325e2..62a93a3233a2183bd0d3af1aba78eb01cc143cca 100644 (file)
@@ -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<void>
 
   install(app: App): void
@@ -79,33 +77,6 @@ export interface Router {
 
 const isClient = typeof window !== 'undefined'
 
-async function runGuardQueue(guards: Lazy<any>[]): Promise<void> {
-  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<RouteQueryAndHash>,
-  //   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<RouteLocationNormalized> {
     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<any>[]): Promise<void> {
+  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]
+}
index ed70d9aa6d4903b3be380db3cdec6342d7738a4c..cbb6c48962806510ac62947f8ce2efa8e216c3b6 100644 (file)
@@ -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<string | number | symbol, any>
   // TODO: only allow a subset?
+  // TODO: RFC: remove this and only allow global options
   options?: PathParserOptions
 }