deepkit / deepkit-framework

A new full-featured and high-performance TypeScript framework
https://deepkit.io/
MIT License
3.23k stars 123 forks source link

Unable to use Express middleware due to request type incompatible #285

Open NexZhu opened 2 years ago

NexZhu commented 2 years ago

Hello

The documentations and this issue say that Express/Connect middlewares are supported but looking through the code it seems like request type is not converted to what Express middlewars expect, I see in the HttpRequest type from @deepkit/http there is no get(headerName) method from Express docs here.

With latest version of the framework "@deepkit/http": "1.0.1-alpha.71", when I tried to use this middleware: https://github.com/auth0/express-openid-connect I got the following error:

/root/dev/web/deepkit-starter/node_modules/.pnpm/express@4.18.1/node_modules/express/lib/router/layer.js:95
    fn(req, res, next);
    ^

TypeError: req.get is not a function
    at /root/dev/web/deepkit-starter/node_modules/.pnpm/express-openid-connect@2.7.3_express@4.18.1/node_modules/express-openid-connect/lib/appSession.js:269:37
    at Layer.handle [as handle_request] (/root/dev/web/deepkit-starter/node_modules/.pnpm/express@4.18.1/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/root/dev/web/deepkit-starter/node_modules/.pnpm/express@4.18.1/node_modules/express/lib/router/index.js:328:13)
    at /root/dev/web/deepkit-starter/node_modules/.pnpm/express@4.18.1/node_modules/express/lib/router/index.js:286:9
    at Function.process_params (/root/dev/web/deepkit-starter/node_modules/.pnpm/express@4.18.1/node_modules/express/lib/router/index.js:346:12)
    at next (/root/dev/web/deepkit-starter/node_modules/.pnpm/express@4.18.1/node_modules/express/lib/router/index.js:280:10)
    at Function.handle (/root/dev/web/deepkit-starter/node_modules/.pnpm/express@4.18.1/node_modules/express/lib/router/index.js:175:3)
    at OidcMiddleware.router [as expressOpenid] (/root/dev/web/deepkit-starter/node_modules/.pnpm/express@4.18.1/node_modules/express/lib/router/index.js:47:12)
    at OidcMiddleware.execute (/root/dev/web/deepkit-starter/src/auth/file:/root/dev/web/deepkit-starter/src/auth/oidc.ts:29:10)
    at Object.fn (eval at build (/root/dev/web/deepkit-starter/node_modules/.pnpm/@deepkit+core@1.0.1-alpha.65/node_modules/@deepkit/core/src/compiler.ts:95:20), <anonymous>:61:100)
[ERROR] 12:25:21 TypeError: req.get is not a function
Child got SIGTERM, exiting.
devonpmack commented 2 years ago

I also can reproduce with the libraries compression and forcedomain. But it works as expected if you just cast to any.

NexZhu commented 2 years ago

Casting to any didn't work for me with https://github.com/auth0/express-openid-connect

marcj commented 2 years ago

I think we have to adjust https://github.com/deepkit/deepkit-framework/blob/master/packages/http/src/model.ts#L179 in order to make it compatible with the request object interface required by those middlewares.

marcj commented 2 years ago

I wonder though why in the stack trace is express code node_modules/.pnpm/express@4.18.1/node_modules/express/lib/router/layer.js:95. Are you using express and Deepkit at the same time?

NexZhu commented 2 years ago

No, I'm just using a type from express (needed by the express-openid-connect middleware, here's my code:

import { http, HttpRequest, HttpResponse } from '@deepkit/http'
import type { RequestHandler } from 'express'
import { auth } from 'express-openid-connect'

import { Config } from '~/config'

@http.middleware()
export class OidcMiddleware {
  private expressOpenid: RequestHandler

  constructor(protected config: Config['oidc']) {
    this.expressOpenid = auth(config)
  }

  execute(
    req: HttpRequest,
    res: HttpResponse,
    next: (err?: unknown) => void,
  ): void {
    // @ts-expect-error: req's param type is incompatible but correct
    // eslint-disable-next-line @typescript-eslint/no-unsafe-argument,@typescript-eslint/no-explicit-any
    this.expressOpenid(req as any, res, next)
  }
}
NexZhu commented 2 years ago

Casting the arguments to the exact types expected by the middleware still results in the same error:

@http.middleware()
export class OidcMiddleware {
  private expressOpenid: RequestHandler

  constructor(protected config: Config['oidc']) {
    this.expressOpenid = auth(config)
  }

  execute(
    req: HttpRequest,
    res: HttpResponse,
    next: (err?: unknown) => void,
  ): void {
    this.expressOpenid(
      req as unknown as Parameters<RequestHandler>[0],
      res as unknown as Parameters<RequestHandler>[1],
      next,
    )
  }
}
NexZhu commented 2 years ago

I also can reproduce with the libraries compression and forcedomain. But it works as expected if you just cast to any.

@devonpmack Could you please share your code?

devonpmack commented 2 years ago

Ah maybe it's because compression and forcedomain don't try to call req.get. My code was just complaining about a type error in the IDE, but it runs properly.

middlewares: [
    httpMiddleware.for(compression() as any),
    httpMiddleware.for(
      forceDomain(domain(process.env.NODE_ENV as NodeEnv)) as any
    ),
  ],

If I take out as any

image
NexZhu commented 2 years ago

Yeah, seems to be different cases, my error happened in run time.