refactor: attempt to resolve the most stubborn types

vaadin · Oct 3, 2024 · 3f8a596 · 3f8a596
1 parent a2ce3fe
commit 3f8a596
Show file tree

Hide file tree

Showing 7 changed files with 105 additions and 78 deletions.
diff --git a/src/resolver/resolveRoute.ts b/src/resolver/resolveRoute.ts
@@ -13,7 +13,7 @@ import { isFunction } from './utils.js';
 export default function resolveRoute<T, R extends AnyObject, C extends AnyObject>(
   context: RouteContext<T, R, C>,
 ): MaybePromise<ActionResult<T>> {
-  if (isFunction(context.route?.action)) {
+  if (isFunction(context.route.action)) {
     return context.route.action(context);
   }
   return undefined;

diff --git a/src/resolver/types.d.ts b/src/resolver/types.d.ts
@@ -19,17 +19,16 @@ export type ChildrenCallback<T, R extends AnyObject, C extends AnyObject> = (
   context: RouteContext<T, R, C>,
 ) => MaybePromise<ReadonlyArray<Route<T, R, C>>>;
 
-export type BasicRoutePart<T, R extends AnyObject, C extends AnyObject> = Readonly<{
-  children?: ReadonlyArray<Route<T, R, C>> | ChildrenCallback<T, R, C>;
-  name?: string;
-  path: string;
-  action?(this: Route<T, R, C>, context: RouteContext<T, R, C>): MaybePromise<ActionResult<T>>;
-}> & {
+export interface BasicRoutePart<T, R extends AnyObject, C extends AnyObject> {
+  readonly children?: ReadonlyArray<Route<T, R, C>> | ChildrenCallback<T, R, C>;
+  readonly name?: string;
+  readonly path: string;
   __children?: ReadonlyArray<Route<T, R, C>>;
   __synthetic?: true;
   parent?: Route<T, R, C>;
   fullPath?: string;
-};
+  action?(this: Route<T, R, C>, context: RouteContext<T, R, C>): MaybePromise<ActionResult<T>>;
+}
 
 export type Route<T = unknown, R extends AnyObject = EmptyObject, C extends AnyObject = EmptyObject> = BasicRoutePart<
   T,
@@ -49,32 +48,31 @@ export type ChainItem<T, R extends AnyObject, C extends AnyObject> = {
   route: Route<T, R, C>;
 };
 
-export type ResolveContext<C extends AnyObject = EmptyObject> = C &
-  Readonly<{
-    pathname: string;
-  }>;
+export type ResolveContext<C extends AnyObject = EmptyObject> = Readonly<{
+  pathname: string;
+}> &
+  C;
 
-export type RouteContext<T, R extends AnyObject = EmptyObject, C extends AnyObject = EmptyObject> = ResolveContext<C> &
-  Readonly<{
-    hash?: string;
-    search?: string;
-    chain?: Array<ChainItem<T, R, C>>;
-    params: IndexedParams;
-    resolver?: Resolver<T, R, C>;
-    redirectFrom?: string;
-    route?: Route<T, R, C>;
-    next?(
-      resume?: boolean,
-      parent?: Route<T, R, C>,
-      prevResult?: ActionResult<RouteContext<T, R, C>>,
-    ): Promise<ActionResult<RouteContext<T, R, C>>>;
-  }> & {
-    __divergedChainIndex?: number;
-    __redirectCount?: number;
-    __renderId: number;
-    __skipAttach?: boolean;
-    result?: T | RouteContext<T, R, C>;
-  };
+export type RouteContext<T, R extends AnyObject = EmptyObject, C extends AnyObject = EmptyObject> = Readonly<{
+  hash?: string;
+  search?: string;
+  chain?: Array<ChainItem<T, R, C>>;
+  params: IndexedParams;
+  resolver?: Resolver<T, R, C>;
+  redirectFrom?: string;
+  route: Route<T, R, C>;
+  next?(
+    resume?: boolean,
+    parent?: Route<T, R, C>,
+    prevResult?: ActionResult<RouteContext<T, R, C>>,
+  ): Promise<ActionResult<RouteContext<T, R, C>>>;
+}> & {
+  __divergedChainIndex?: number;
+  __redirectCount?: number;
+  __renderId: number;
+  __skipAttach?: boolean;
+  result?: T | RouteContext<T, R, C>;
+} & ResolveContext<C>;
 
 export type PrimitiveParamValue = string | number | null;
 

diff --git a/src/router.ts b/src/router.ts
@@ -1,6 +1,6 @@
 /* eslint-disable @typescript-eslint/consistent-return */
 import { compile } from 'path-to-regexp';
-import type { EmptyObject, Writable } from 'type-fest';
+import type { EmptyObject } from 'type-fest';
 import generateUrls from './resolver/generateUrls.js';
 import Resolver from './resolver/resolver.js';
 import './router-config.js';
@@ -166,7 +166,7 @@ export class Router<R extends AnyObject = EmptyObject, C extends AnyObject = Emp
   private async __resolveRoute(context: RouteContext<R, C>): Promise<ActionResult | RouteContext<R, C>> {
     const { route } = context;
 
-    if (isFunction(route?.children)) {
+    if (isFunction(route.children)) {
       let children = await route.children(context);
 
       // The route.children() callback might have re-written the
@@ -190,7 +190,7 @@ export class Router<R extends AnyObject = EmptyObject, C extends AnyObject = Emp
 
     return await Promise.resolve()
       .then(async () => {
-        if (this.__isLatestRender(context) && route) {
+        if (this.__isLatestRender(context)) {
           return await maybeCall(route.action, route, context, commands);
         }
       })
@@ -204,15 +204,15 @@ export class Router<R extends AnyObject = EmptyObject, C extends AnyObject = Emp
           }
         }
 
-        if (isString(route?.redirect)) {
+        if (isString(route.redirect)) {
           return commands.redirect(route.redirect);
         }
       })
       .then((result) => {
         if (result != null) {
           return result;
         }
-        if (isString(route?.component)) {
+        if (isString(route.component)) {
           return commands.component(route.component);
         }
       });
@@ -384,10 +384,6 @@ export class Router<R extends AnyObject = EmptyObject, C extends AnyObject = Emp
       // Find the first route that resolves to a non-empty result
       const ctx = await this.resolve(context);
 
-      if (!ctx || ctx === notFoundResult) {
-        return this.location;
-      }
-
       // Process the result of this.resolve() and handle all special commands:
       // (redirect / prevent / component). If the result is a 'component',
       // then go deeper and build the entire chain of nested components matching
@@ -502,7 +498,7 @@ export class Router<R extends AnyObject = EmptyObject, C extends AnyObject = Emp
         if (isFound) {
           // ...but original context is already fully matching - use it
           return context;
-        } else if (parent?.parent != null) {
+        } else if (parent.parent != null) {
           // ...and there is no full match yet - step up to check siblings
           return await findNextContextIfAny(context, parent.parent, nextContext);
         }
@@ -515,7 +511,9 @@ export class Router<R extends AnyObject = EmptyObject, C extends AnyObject = Emp
     const nextContext = await findNextContextIfAny(contextAfterRedirects);
 
     if (nextContext == null || nextContext === notFoundResult) {
-      throw getNotFoundError(topOfTheChainContextAfterRedirects);
+      throw getNotFoundError<ActionValue, RouteExtension<R, C>, ContextExtension<R, C>>(
+        topOfTheChainContextAfterRedirects,
+      );
     }
 
     return nextContext !== contextAfterRedirects
@@ -697,6 +695,8 @@ export class Router<R extends AnyObject = EmptyObject, C extends AnyObject = Emp
     return context.__renderId === this.__lastStartedRenderId;
   }
 
+  declare ['resolve']: (context: RouteContext<R, C>) => Promise<RouteContext<R, C> & RedirectContextInfo>;
+
   private async __redirect(
     redirectData: RedirectContextInfo,
     counter: number = 0,
@@ -707,7 +707,7 @@ export class Router<R extends AnyObject = EmptyObject, C extends AnyObject = Emp
     }
 
     return await this.resolve({
-      ...rootContext,
+      ...(rootContext as RouteContext<R, C>),
       pathname: this.urlForPath(redirectData.pathname, redirectData.params),
       redirectFrom: redirectData.from,
       __redirectCount: counter + 1,

diff --git a/src/routerUtils.ts b/src/routerUtils.ts
@@ -1,6 +1,7 @@
 import { compile } from 'path-to-regexp';
 import type Resolver from './resolver/resolver.js';
 import { isFunction, isObject, isString, log, toArray } from './resolver/utils.js';
+import type { Router } from './router.js';
 import type {
   ActionResult,
   AnyObject,
@@ -85,6 +86,15 @@ export function getRoutePath<R extends AnyObject, C extends AnyObject>(chain: Re
   return getMatchedPath(chain.map((chainItem) => chainItem.route));
 }
 
+export type ResolverOnlyContext<R extends AnyObject, C extends AnyObject> = Readonly<{ resolver: Router<R, C> }>;
+
+export function createLocation<R extends AnyObject, C extends AnyObject>({
+  resolver,
+}: ResolverOnlyContext<R, C>): RouterLocation<R, C>;
+export function createLocation<R extends AnyObject, C extends AnyObject>(
+  context: RouteContext<R, C>,
+  route?: Route<R, C>,
+): RouterLocation<R, C>;
 export function createLocation<R extends AnyObject, C extends AnyObject>(
   { chain = [], hash = '', params = {}, pathname = '', redirectFrom, resolver, search = '' }: RouteContext<R, C>,
   route?: Route<R, C>,
@@ -125,7 +135,7 @@ export function renderElement<R extends AnyObject, C extends AnyObject, E extend
 ): E {
   element.location = createLocation(context);
 
-  if (context.chain && context.route) {
+  if (context.chain) {
     const index = context.chain.map((item) => item.route).indexOf(context.route);
     context.chain[index].element = element;
   }

diff --git a/src/types.d.ts b/src/types.d.ts
@@ -78,10 +78,11 @@ export type ChainItem<R extends AnyObject, C extends AnyObject> = _ChainItem<
     element?: WebComponentInterface<R, C>;
   }>;
 
-export type ContextExtension<R extends AnyObject, C extends AnyObject> = C &
-  Readonly<{
-    chain?: ReadonlyArray<ChainItem<R, C>>;
-  }>;
+export type ContextExtension<R extends AnyObject, C extends AnyObject> = Readonly<{
+  resolver?: Router<R, C>;
+  chain?: Array<ChainItem<R, C>>;
+}> &
+  R;
 // Readonly<{
 //   next(resume?: boolean): Promise<ActionResult>;
 // }>;
@@ -92,21 +93,20 @@ export type ChildrenCallback<R extends AnyObject, C extends AnyObject> = _Childr
   ContextExtension<R, C>
 >;
 
-export type RouteExtension<R extends AnyObject, C extends AnyObject> = R &
-  Readonly<
-    RequireAtLeastOne<{
-      children?: ChildrenCallback<R, C> | ReadonlyArray<Route<R, C>>;
-      component?: string;
-      redirect?: string;
-      action?(
-        this: Route<R, C>,
-        context: RouteContext<R, C>,
-        commands: Commands,
-      ): MaybePromise<ActionResult | RouteContext<R, C>>;
-    }>
-  > & {
-    animate?: AnimateCustomClasses | boolean;
-  };
+export type RouteExtension<R extends AnyObject, C extends AnyObject> = Readonly<
+  RequireAtLeastOne<{
+    children?: ChildrenCallback<R, C> | ReadonlyArray<Route<R, C>>;
+    component?: string;
+    redirect?: string;
+    action?(
+      this: Route<R, C>,
+      context: RouteContext<R, C>,
+      commands: Commands,
+    ): MaybePromise<ActionResult | RouteContext<R, C>>;
+  }>
+> & {
+  animate?: AnimateCustomClasses | boolean;
+} & R;
 
 export type RouteContext<R extends AnyObject = EmptyObject, C extends AnyObject = EmptyObject> = _RouteContext<
   ActionValue,

diff --git a/test/router/router.spec.ts b/test/router/router.spec.ts
@@ -1,8 +1,14 @@
 import { expect } from '@esm-bundle/chai';
 import sinon from 'sinon';
-import type { Writable } from 'type-fest';
+import type { EmptyObject, Writable } from 'type-fest';
 import { Router } from '../../src/router.js';
-import type { Commands, Route, RouteContext, WebComponentInterface } from '../../src/types.js';
+import {
+  ChildrenCallback,
+  type Commands,
+  type Route,
+  type RouteContext,
+  type WebComponentInterface,
+} from '../../src/types.js';
 import '../setup.js';
 import { checkOutletContents, cleanup, onBeforeEnterAction } from './test-utils.js';
 
@@ -336,8 +342,10 @@ describe('Router', () => {
         const firstResult = await spy.firstCall.returnValue;
         expect(firstResult).to.have.property('result').that.deep.equals(result);
 
-        expect(spy.secondCall.args[0].redirectFrom).to.equal(from);
-        expect(spy.secondCall.args[0].pathname).to.equal(pathname);
+        const secondArg = spy.secondCall.args[0] as RouteContext;
+
+        expect(secondArg.redirectFrom).to.equal(from);
+        expect(secondArg.pathname).to.equal(pathname);
       });
 
       it('should handle multiple redirects', async () => {
@@ -771,17 +779,19 @@ describe('Router', () => {
         });
 
         it('should work in onBeforeEnter lifecycle method', async () => {
+          const callback = sinon.stub(() => {
+            expect(() => {
+              router.location.getUrl();
+            }).to.not.throw();
+          });
           await router.setRoutes([
             {
               path: '/',
-              action: onBeforeEnterAction('x-foo', () => {
-                expect(() => {
-                  router.location.getUrl();
-                }).to.not.throw();
-              }),
+              action: onBeforeEnterAction('x-foo', callback),
             },
           ]);
           await router.ready;
+          expect(callback).to.have.been.calledOnce;
         });
 
         // cannot mock the call to `compile()` from the 'pathToRegexp' package
@@ -2111,7 +2121,7 @@ describe('Router', () => {
       });
 
       it('should be able to override the route `children` property instead of returning a value', async () => {
-        const children = sinon.spy((_context: RouteContext) => {
+        const children = sinon.spy((_context: RouteContext, _commands: Commands) => {
           (_context.route as Writable<Route>).children = [{ path: '/:user', component: 'x-user-profile' }];
         });
 

diff --git a/test/router/test-utils.ts b/test/router/test-utils.ts
@@ -3,6 +3,7 @@
 
 import { expect } from '@esm-bundle/chai';
 import type { Commands, RouteContext, Router, WebComponentInterface } from '../../src/index.js';
+import type { AnyObject, Route } from '../../src/resolver/types.js';
 
 export async function waitForNavigation(): Promise<void> {
   return await new Promise((resolve) => {
@@ -16,12 +17,20 @@ export function cleanup(element: Element): void {
 
 export function verifyActiveRoutes(router: Router, expectedSegments: string[]): void {
   // @ts-expect-error: __previousContext is a private property
-  expect(router.__previousContext?.chain?.map((item) => item.route?.path)).to.deep.equal(expectedSegments);
+  expect(router.__previousContext?.chain?.map((item) => item.route.path)).to.deep.equal(expectedSegments);
 }
 
 function createWebComponentAction<T extends keyof WebComponentInterface>(method: T) {
-  return (componentName: string, callback: WebComponentInterface[T], name: string = 'unknown') =>
-    (_context: RouteContext, commands: Commands): WebComponentInterface => {
+  return <R extends AnyObject, C extends AnyObject>(
+    componentName: string,
+    callback: WebComponentInterface<R, C>[T],
+    name: string = 'unknown',
+  ) =>
+    function lifecycleCallback(
+      this: Route<R, C>,
+      _context: RouteContext<R, C>,
+      commands: Commands,
+    ): WebComponentInterface<R, C> {
       const component = commands.component(componentName) as WebComponentInterface;
       component.name = name;
       component[method] = callback;