moscajs / aedes

Barebone MQTT broker that can run on any stream server, the node way
MIT License
1.75k stars 229 forks source link

[bug] TypeScript error due to incompatible aedes.handle function for Node net module v16 #801

Open haci-xplora opened 1 year ago

haci-xplora commented 1 year ago

System Information

Describe the bug Function aedes.handle is not compatible with listener for net.createServer. So, it causes TypeScript compile error.

Sample code:

import Aedes from 'aedes';
import { createServer } from 'net';
const mqttServer = new Aedes();
const netServer = createServer(mqttServer.handle);
netServer.listen(1883, () => {
  console.info('ready at 1883');
});

Error:

src/index.ts:4:32 - error TS2769: No overload matches this call.
  Overload 1 of 2, '(connectionListener?: ((socket: Socket) => void) | undefined): Server', gave the following error.
    Argument of type '(stream: Connection, request: IncomingMessage) => Client' is not assignable to parameter of type '(socket: Socket) => void'.
  Overload 2 of 2, '(options?: ServerOpts | undefined, connectionListener?: ((socket: Socket) => void) | undefined): Server', gave the following error.
    Type '(stream: Connection, request: IncomingMessage) => Client' has no properties in common with type 'ServerOpts'.

4 const netServer = createServer(mqttServer.handle);
                                 ~~~~~~~~~~~~~~~~~
Found 1 error.

To Reproduce Steps to reproduce the behavior is not much different than the sample code on Examples page

  1. try to compile project with sample code

Expected behavior Project should compile as expected without any errors.

robertsLando commented 1 year ago

@haci-xplora this is a type issue, could you please send a PR to fix it?

haci-xplora commented 1 year ago

hi Robert.

the type of handle function is based on the signature of implementation, so it is correct. but if request parameter (IncomingMessage) is not in use (as I can see it is not used), it should be omitted. that is why it should not be assigned as listener of net server. shall I do that and remove request parameter?

or we can make it optional/nullable.

otherwise, multiple adapter clients are needed for net server, and other servers like websocket server. see Client.md it says request parameter is for that.

one class cannot handle those variations well.

let me create a PR and discuss further.

haci-xplora commented 1 year ago

request parameter is now optional