socket.io icon indicating copy to clipboard operation
socket.io copied to clipboard

Namespaces are contravariant over SocketData because `Namespace['_fns']` is not `private`

Open ammut opened this issue 1 year ago • 2 comments

Describe the bug

I have a package that exports a function requiring a typed Namespace to do its thing. This Namespace has a SocketData value of { connectionId: string }. In my actual application I then set up such a namespace to provide said setup function with a such a namespace.

However, because the actual namespace I set up has additional properties in SocketData, for other parts of the application. This results in a TypeScript compiler error when trying to apply said actual namespace to that setup function.

The error stems from the fact that the _fns property on Namespace is contravariant over SocketData.

To Reproduce

Server

import type { Namespace } from "socket.io";
import { Server } from "socket.io";

const io = new Server(3000, {});
const ns = io.of('/my-ns') as Namespace<any, any, any, { connectionId: string; otherStuff: string }>;

const nsForMyPackage: Namespace<any, any, any, { connectionId: string }> = ns; // <- THIS ERRORS

Additional context

In theory, because _fns is not marked private, I can do something like this:

declare const nsA: Namespace<any, any, any, { foo: string; bar: string }>;

nsA.use((socket, next) => {
  console.log(socket.data.bar.toUpperCase());
  next();
});

const nsB: Namespace<any, any, any, { foo: string }> = nsA; // BAD ASSIGNMENT

const nsC: Namespace<any, any, any, { foo: string }>;

nsC.on('connection', (socketC) => {
  nsB._fns[0](socketC, () => {}); // BAD CALL
});

To the TypeScript compiler, BAD CALL looks fine. nsB._fns contains functions that require a Socket with SocketData of { foo: string }. nsC.on('connection', ...) provides just such a Socket. However, that middleware will try to call socket.data.bar.toUpperCase() and produce a TypeError: bar is undefined. That is why the compiler warns me at BAD ASSIGNMENT.

HOWEVER, I should never actually know anything about _fns, even less access it. That would make it impossible to do any such mischief. Then the assignment at BAD ASSIGNMENT would be safe. As even in that case, code like this would be sound:


declare const nsA: Namespace<any, any, any, { foo: string; bar: string }>;
nsA.use((socket, next) => {
  // setup socket data
  socket.data.foo = 'foo';
  socket.data.bar = 'bar';
});

const nsB: Namespace<any, any, any, { foo: string }> = nsA; // This would be a valid assignment if `_fns` was `private`

nsB.on('connection', (socket) => {
  console.log(socket.data.foo);
  console.log(socket.data.bar); // This would be a type error, but still work at runtime
});

I took a quick glance at the Namespace source and it looks like you're keeping it public (with a /** @private */ JSDoc comment) because you need to access it in ParentNamespace.createChild. Two suggestions to deal with that after making _fns private:

  1. Use namespace['_fns'] = this._fns.slice() in createChild
  2. Provide a protected static setFns(ns: Namespace, fns: Array<...>) { ns._fns = fns; } in Namespace. This would have the added benefit of working even with JS private fields (#_fns), and because it's protected will only be accessible to Namespace or it's subclasses.

Let me know if one of those options sounds good to you and you want me to make a PR.

ammut avatar Aug 30 '24 15:08 ammut

@ammut hi, thanks for the details :+1:

Option 1 sounds good to me, with _fns as protected maybe? Could you please open a PR?

darrachequesne avatar Sep 16 '24 07:09 darrachequesne

Will do, however _fns needs to become private so the types get erased in the .d.ts.

ammut avatar Sep 16 '24 07:09 ammut