Improving generateId method #535
Conversation
Adding a callback to generateId method provides the program flow to continue after a new generated id is provided.
Adding callback to generateId method
lib/server.js
Outdated
| Server.prototype.generateId = function (req) { | ||
| return base64id.generateId(); | ||
| Server.prototype.generateId = function (req, callback) { | ||
| callback(base64id.generateId()); |
There was a problem hiding this comment.
How about callback(null, base64id.generateId());, so that errors can also be handled (see #518)?
There was a problem hiding this comment.
You're right! I'll update the code.
|
@efkan merged, thanks! |
That is a breaking change, which mandates a major bump.
|
Hello @darrachequesne , I've seen that this PR had to be reverted. I could not find any issue about it. |
|
@efkan I'm currently working on releasing a new version of socket.io, but yes both of your propositions (async |
|
@darrachequesne , Do you want me to apply my code again on a new PR? |
|
@efkan I'll cherry-pick it. |
|
Can we make this support Promises (async functions) as well as callbacks? |
The kind of change this PR does introduce
Current behaviour
If overwritten and assigned
generateIdfunction needs some extra operation like Redis query,idvalue passed as undefined because Javascript runs asynchronously.New behaviour
Adding a callback to
generateIdmethod provides the program flow to continue after a new generated id is provided.Other information (e.g. related issues)
Also README file has been changed according to the updates above.