microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.03k stars 12.49k forks source link

Type checking of extended mapped type #31176

Closed TheDSCPL closed 5 years ago

TheDSCPL commented 5 years ago

Typescript version: 3.4.5.

I'm writing the boilerplate code for type-safe Express routes in my projects, possibly even create a npm library, if it works as expected. The idea is to make sure all the API responses have the shape

{result: T, error?: any}

where T is the Response type of the route, or

{error: any}

if there is an error and declare the type of each routes' request and response types in a declarative way.

The API is routed under the /api route. I'm accomplishing this with app.use('/api', indexRouter.router), where indexRouter is an instance of WrappedRouter (which is defined at the end of this bug report) which is imported from the file routes/index.ts.

Contents of routes/index.ts:

import authRouter from './auth';
import makeRouter from "../utils/RestRoute";

const router = makeRouter();

router.makeSubRoute('/auth',authRouter);

export default router;

Contents of routes/auth/types.ts:

import {ITypes} from '../../utils/RestRoute';

export interface Types extends ITypes {
    '/sayHello': {
        request: void,
        response: string
    }
}

The idea is that I can now write this: Contents of routes/auth/index.ts:

import makeRouter from '../../utils/RestRoute';
import {Types} from "./types";

const router = makeRouter<Types>();

router.route('/sayHello',args => ({
    result: 'Hello World!'
}));

export default router.router;

Contents of RestRoute.ts (which contains the wrapper around Express.JS's Route and most of the boilerplate logic):

import express from 'express';

export interface ITypes {
    [key: string]: {
        request: any,
        response: any
    }
}

interface HandlerFuncionArgs<Req> {
    cookies: express.Request["cookies"],
    hostname: express.Request["hostname"],
    ip: express.Request["ip"],
    clearCookie: express.Response["clearCookie"],
    cookie: express.Response["cookie"],
    secure: express.Request["secure"],
    body: Req,
    params: express.Request["params"]
}

export type APIResultType<T> = {result: T, error?: any} | {error: any};

export type HandlerFunction<Req,Res> = (args: HandlerFuncionArgs<Req>)=>APIResultType<Res> & {status?: number};

class WrappedRouter<Types extends ITypes> {
    public constructor(public readonly router: express.Router) {}

    private static proxyFunction<Req,Res>(handlerFunction: HandlerFunction<Req,Res>): express.RequestHandler {
        return (req, res, next) => {
            res.type('application/json');
            try {
                const ret = handlerFunction({
                    cookies: req.cookies,
                    hostname: req.hostname,
                    ip: req.ip,
                    clearCookie: res.clearCookie,
                    cookie: res.cookie,
                    secure: req.secure,
                    body: req.body,
                    params: req.params
                });
                res.status(ret.status || 200);
                delete ret.status;
                res.json(ret as APIResultType<Res>);
            } catch(e) {
                res.status(500);
                res.json({error: e} as APIResultType<Res>);
            } finally {
                res.end();
            }
        };
    }

    public route<Path extends keyof Types>(path: Path, handlerFunction: HandlerFunction<Types[Path]["request"],Types[Path]["response"]>):void {
        this.router.post(path as string,WrappedRouter.proxyFunction(handlerFunction));
    }

    public makeSubRoute(path: string, subRouter: express.Router): void;

    public makeSubRoute<SubRouterTypes extends ITypes>(path: string, subRouter: WrappedRouter<SubRouterTypes>): void;

    public makeSubRoute<SubRouterTypes extends ITypes>(path: string, subRouter: express.Router|WrappedRouter<SubRouterTypes>) {
        if(subRouter instanceof WrappedRouter) {
            this.router.use(path,subRouter.router);
        } else {
            this.router.use(path,subRouter);
        }
    }
}

export function wrapRouter<Types extends ITypes>(router: express.Router) {
    return new WrappedRouter<Types>(router);
}

export function makeRouter<Types extends ITypes>() {
    return new WrappedRouter<Types>(express.Router({caseSensitive: true, mergeParams: true}));
}

export default makeRouter;

(ITypes is supposed to be the generic shape of every route's type: each key, representing a route, should contain an object with "response" and "request" as its only keys. ITypes isn't supposed to be instantiated, only extended by other types)

Expected behavior: Correctly checks the types of the request and response of each route AND doesn't allow routes which aren't defined in that sub-route's Type.

Actual behaviour: It works well in checking the types of the route: ie. if I try to return result: 1 in the /api/auth/sayHello route, the compiler fails and says that number isn't compatible with string. The problem is when I mistype /sayHello for, say, /sayHelo. There is no error reported from the compiler even though WrappedRouter.route's first argument is of type Path extends keyof Types. It just simply accepts any type in the request and any type in the response... It should be an error because /sayHelo is not in keyof Type, which is equal to /sayHelo.

I'm guessing the problem is in extending the interface because it is first declared as having string keys but doesn't specify the keys' type when it is extended. I think keyof Types computes to "/sayHello" | string.

I've tried to change the generic type of the route function to <Path extends Exclude<keyof Types, string>> but it doesn't work because it also deletes "/sayHello" from the union type. Then the router.route('/sayHello' (...) line in the auth/index.ts file reports the error TS2345: Argument of type '"/sayHello"' is not assignable to parameter of type 'number'.

Is there any workaround or a hacky way of accomplishing this while this isn't fix this in a future version of TS?

dragomirtitian commented 5 years ago

That is one long question, might I suggest in the future:

  1. Isolating a smaller code snippet illustrating your issue.
  2. Trying stackoverflow first. This forum is generally reserved for bugs, or questions that SO didn't or can't answer (you will probably get answers there just as fast here and by the same people)

That being said, your root cause is that you force Types to have an index signature. If Types has an index signature, keyof Types will be string allowing any string in path.

You can change the definitions a bit, you actually want to constrain ITypes to have only properties of type { request: any, response: any } regardless of if they have an index signature.

A solution that seems to work (although I have not tested it a lot) would be:


//RestRoute.ts
import express from 'express';

export type ITypes<K extends PropertyKey> = Record<K, { request: any, response: any }>

interface HandlerFuncionArgs<Req> {
    cookies: express.Request["cookies"],
    hostname: express.Request["hostname"],
    ip: express.Request["ip"],
    clearCookie: express.Response["clearCookie"],
    cookie: express.Response["cookie"],
    secure: express.Request["secure"],
    body: Req,
    params: express.Request["params"]
}

export type APIResultType<T> = {result: T, error?: any} | {error: any};

export type HandlerFunction<Req,Res> = (args: HandlerFuncionArgs<Req>)=>APIResultType<Res> & {status?: number};

class WrappedRouter<Types extends ITypes<keyof Types>> {
    public constructor(public readonly router: express.Router) {}

    private static proxyFunction<Req,Res>(handlerFunction: HandlerFunction<Req,Res>): express.RequestHandler {
        return (req, res, next) => {
            res.type('application/json');
            try {
                const ret = handlerFunction({
                    cookies: req.cookies,
                    hostname: req.hostname,
                    ip: req.ip,
                    clearCookie: res.clearCookie,
                    cookie: res.cookie,
                    secure: req.secure,
                    body: req.body,
                    params: req.params
                });
                res.status(ret.status || 200);
                delete ret.status;
                res.json(ret as APIResultType<Res>);
            } catch(e) {
                res.status(500);
                res.json({error: e} as APIResultType<Res>);
            } finally {
                res.end();
            }
        };
    }

    public route<Path extends keyof Types>(path: Path, handlerFunction: HandlerFunction<Types[Path]["request"],Types[Path]["response"]>):void {
        this.router.post(path as string,WrappedRouter.proxyFunction(handlerFunction));
    }

    public makeSubRoute(path: string, subRouter: express.Router): void;

    public makeSubRoute<SubRouterTypes extends ITypes<keyof SubRouterTypes>>(path: string, subRouter: WrappedRouter<SubRouterTypes>): void;

    public makeSubRoute<SubRouterTypes extends ITypes<keyof SubRouterTypes>>(path: string, subRouter: express.Router|WrappedRouter<SubRouterTypes>) {
        if(subRouter instanceof WrappedRouter) {
            this.router.use(path,subRouter.router);
        } else {
            this.router.use(path,subRouter);
        }
    }
}

export function wrapRouter<Types extends ITypes<keyof Types>>(router: express.Router) {
    return new WrappedRouter<Types>(router);
}

export function makeRouter<Types extends ITypes<keyof Types>>() {
    return new WrappedRouter<Types>(express.Router({caseSensitive: true, mergeParams: true}));
}

export default makeRouter;

// types.ts

import {ITypes} from '../../utils/RestRoute';

export interface Types extends ITypes<keyof Types> { // ensures all properties have the correct shape
    '/sayHello': {
        request: void,
        response: string
    }
}

//auth/index.ts
import makeRouter from '../../utils/RestRoute';

import {Types} from "./types";

const router = makeRouter<Types>();

//ok
router.route('/sayHello',args => ({
    result: 'Hello World!'
}));

//err
router.route('/sayHelloo',args => ({
    result: 'Hello World!'
}));

export default router.router;
TheDSCPL commented 5 years ago

Uhm, ok! I didn't know you could do this kind of recursive operations in the types definition! I thought this was a bug because I dind't know <Types extends ITypes<keyof Types>> was valid syntax. Thank you very much!