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

Optional parameters (opt) are overridden when initializing Emitter with {host, port} object. #95

Closed
xprudhomme opened this issue Mar 11, 2021 · 1 comment
Milestone

Comments

@xprudhomme
Copy link

xprudhomme commented Mar 11, 2021

Issue
When trying to initialize the Emitter with an {host, port} object and the key option, I noticed nothing was published as expected on Redis, by running the Redis monitor: the key used remains the same as the default one socket.io :

1615484908.414247 [0 127.0.0.1:58230] "publish" "socket.io#/waiting_lines#" "\x93\xa7emitter\x83\xa4type\x02\xa4data\x92\xa4list\x84\xa4data\x94\x8d\xa2id\xd9$1eb84855-3081-4067-90a5-6bb2cb5e20d6\xaacreated_at\xb82020-09-23T11:44:08.000Z\xaaupdated_at\xb82021-01-29T06:33:42.000Z

I have the following packages:
"socket.io": "^2.3.0", "socket.io-emitter": "^3.2.0", "socket.io-redis": "^5.4.0",

I initialized the Emitter, as follows:

import ioEmitter, { SocketIOEmitter } from "socket.io-emitter";
const io = ioEmitter({host: '127.0.0.1', port: 6379 }, { key: "some-socket.io-key" });

What I've noticed in socket.io-emitter/index.js file, in the init function (adding some console.log for debug purposes):

/**
 * Socket.IO redis based emitter.
 *
 * @param {Object} redis client (optional)
 * @param {Object} options
 * @api public
 */

function init(redis, opts){
  console.log(` [SOCKET.IO-EMITTER] Options: `, opts);
  opts = opts || {};

  console.log(` [SOCKET.IO-EMITTER] Options (2): `, opts);

  if ('string' == typeof redis) {
    redis = client(redis);
  }

  if (redis && !redis.hset) {
    opts = redis;
    redis = null;
  }

  console.log(` [SOCKET.IO-EMITTER] Options (3): `, opts);

  if (!redis) {
    if (!opts.socket && !opts.host) throw new Error('Missing redis `host`');
    if (!opts.socket && !opts.port) throw new Error('Missing redis `port`');
    redis = opts.socket
      ? client(opts.socket)
      : client(opts.port, opts.host);
  }

  var prefix = opts.key || 'socket.io';
  console.log(` [SOCKET.IO-EMITTER] Prefix used: `, prefix);
  return new Emitter(redis, prefix, '/');
}

Here's what the console shown:
[SOCKET.IO-EMITTER] Options: { key: 'some-socket.io-key' }
[SOCKET.IO-EMITTER] Options (2): { key: 'some-socket.io-key' }
[SOCKET.IO-EMITTER] Options (3): { host: '127.0.0.1', port: 6379 }
[SOCKET.IO-EMITTER] Prefix used: socket.io

To conclude with:
Basically , I cannot config a custom key, as the opts object is overriden. I suppose this is not how it's supposed to work, right?

Workaround
To be able to get it working as expected, I had to do as follows:

    let redisOptions: SocketIORedisOptions = {
      host,
      port,
    };
    // Unable to set key prop in previous object as TypeScript 
    // would throw an error saying 'key' does not exist in SocketIORedisOptions type 
    redisOptions["key"] = "some-socket.io-key";
    const io = ioEmitter(redisOptions);
darrachequesne added a commit that referenced this issue Mar 17, 2021
The codebase is now written in TypeScript.

Additional features:

- https://socket.io/docs/v4/migrating-from-3-x-to-4-0/#Allow-excluding-specific-rooms-when-broadcasting
- https://socket.io/docs/v4/migrating-from-3-x-to-4-0/#Additional-utility-methods
- https://socket.io/docs/v4/migrating-from-3-x-to-4-0/#Typed-events

BREAKING CHANGE: the "redis" package is not installed by default
anymore, you'll now need to create your own redis client and pass it to
the Emitter constructor

Before:

```js
const io = require("socket.io-emitter")({ host: "127.0.0.1", port: 6379 });
```

After:

```js
const { Emitter } = require("socket.io-emitter");
const { createClient } = require("redis");

const redisClient = createClient();
const io = new Emitter(redisClient);
```

Related:

- #89
- #90
- #95
@darrachequesne
Copy link
Member

Yes, this is for this kind of problems that the redis client won't be created by the library anymore (a70db12)

Before (socket.io-emitter@3):

const io = require("socket.io-emitter")({ host: "127.0.0.1", port: 6379 });

After (@socket.io/redis-emitter@4):

const { Emitter } = require("@socket.io/redis-emitter");
const { createClient } = require("redis");

const redisClient = createClient();
const io = new Emitter(redisClient);

@darrachequesne darrachequesne added this to the 4.0.0 milestone Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants