Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ammut opened this issue Aug 30, 2024 · 2 comments
Labels
enhancement New feature or request package:socket.io This concerns the "socket.io" package

Comments

@ammut
Copy link
Contributor

ammut commented Aug 30, 2024

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 ammut added the to triage Waiting to be triaged by a member of the team label Aug 30, 2024
@darrachequesne
Copy link
Member

@ammut hi, thanks for the details 👍

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

@darrachequesne darrachequesne added enhancement New feature or request package:socket.io This concerns the "socket.io" package and removed to triage Waiting to be triaged by a member of the team labels Sep 16, 2024
@ammut
Copy link
Contributor Author

ammut commented Sep 16, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request package:socket.io This concerns the "socket.io" package
Projects
None yet
Development

No branches or pull requests

2 participants