Use own interface for Socket request to allow extending the request interface independently of Express.

[alesmenzel]

Use own interface for Socket request to allow extending the request interface with custom properties without polluting the IncomingMessage type which is also used by Express/Passport.

Using socket.io with passport middlewares causes issues when using the same 'userProperty' key is used for both - for example by extending IncomingMessage { user: Express.User } we break passport's isAuthenticated() function, because now typescript thinks that the request is always of the type AuthenticatedRequest as IncomingMessage always has a user property on itself. To fix this, I believe we should be able to extend only the request type inside socket.io -> SocketRequest.

The kind of change this PR does introduce

• a bug fix
• a new feature
• an update to the documentation
• a code change that improves performance
• other - Type change

[alesmenzel]

Hi @darrachequesne, sorry to bother you, would you please review this change.

[alesmenzel]

References #4795

[darrachequesne]

Hi! I'm not sure this is needed actually, you should be able to reuse the same SessionData:

import { type Request } from "express";

declare module "express-session" {
  interface SessionData {
    count: number;
  }
}

io.on("connection", (socket) => {
  const req = socket.request as Request;

  req.session.count++;
});

I've updated the example here: https://socket.io/how-to/use-with-express-session

[alesmenzel]

Hi @darrachequesne , thanks for looking at this, but I don't think it solves the issue. You are hard-casting the request to be something it is not, an express Request.

• e.g. removing session/passport middleware would still show as if the request has session and user properties
• you have to cast every handler where you want to access session and user properties

whereas with the change suggested in this PR, we can extend the SocketRequest interface to include the fields we need - it is not the cleanest solution either, because we are still lying to TS, but in user code we don't need to cast the request everytime we want to use it.

Probably the best solution would be to add a generic to the Socket for the type of Request

class Socket<..., SocketRequest extends IncomingRequest = IncomingRequest> {

 public get request(): SocketRequest {

[alesmenzel]

Hi, sorry to bother you @darrachequesne, I still think there is something to fix. Please see my comment above.

[darrachequesne]

You are hard-casting the request to be something it is not, an express Request.

Fair enough! That being said, with your solution one would need to declare the SessionData twice, isn't it?

declare module "express-session" {
  interface SessionData {
    count: number;
  }
}

declare global {
  namespace Express {
    interface User {
      username: string;
    }
  }
}

const io = new Server<any, any, any, any, IncomingMessage & {
  session: Session & {
    count: number;
  },
  user: Express.User
}>(httpServer);

[alesmenzel]

Yes, but you can reuse the SessionData type, but it is just because express-session is done this way.
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/express-session/index.d.ts#L29

interface SocketRequest extends IncomingMessage {
  session: Session & Partial<SessionData>
  user: Express.User
}

const io = new Server<any, any, any, any, SocketRequest >

[darrachequesne]

@alesmenzel something like this then?

[alesmenzel]

@darrachequesne Yes, but I don´t understand using merging IncomingMessage & AdditionalRequestData over allowing to specify own "IncomingMessage" type.

With custom "IncomingMessage", it should be possible to just return

public get request(): SocketRequest {
    return this.client.request;
  }

without typecasting.

[darrachequesne]

If I understand correctly, you are suggesting something like this:

export class Socket<
  ListenEvents extends EventsMap = DefaultEventsMap,
  EmitEvents extends EventsMap = ListenEvents,
  ServerSideEvents extends EventsMap = DefaultEventsMap,
  SocketData = any,
  SocketRequest extends IncomingMessage = IncomingMessage
> extends StrictEventEmitter<
  ListenEvents,
  EmitEvents,
  SocketReservedEventsMap
> {

  // [...]

  public get request(): SocketRequest {
    return this.client.request;
  }

}

But in that case, TypeScript complains:

lib/socket.ts:1010:5 - error TS2322: Type 'IncomingMessage' is not assignable to type 'SocketRequest'.
  'IncomingMessage' is assignable to the constraint of type 'SocketRequest', but 'SocketRequest' could be instantiated with a different subtype of constraint 'IncomingMessage'.

1010     return this.client.request;
         ~~~~~~~~~~~~~~~~~~~~~~~~~~~

Or am I missing something? Could you please open a pull request with what you had in mind?