[STREAM-357] - Maintain JID across hard reconnects/stanza instances from the same client. #282
Conversation
…rom the same client.
| .then(res => res.data.chat.jabberId); | ||
| // Use stored JID if we have one, otherwise use the provided JID or grab one. | ||
| if (!this.initialJid) { | ||
| if (this.config.jid) { |
There was a problem hiding this comment.
I'm not sure if this is actually needed, it may be redundant because we end up assigning this.config.jid = this.initialJid later too.
There was a problem hiding this comment.
This does feel really weird actually...
So if initialJid is falsy and config.jid = 'a' for example. Then it'll be:
initialJid = config.jid = 'a'
config.jid = initialJid = 'a'
What happens if you remove this part?
There was a problem hiding this comment.
I think the behavior is mostly the same as it was before.
The idea was - if we don't have a saved JID, then we would try to use the one in the config, but if we don't have one there then we'd fetch one.
And if we fetched one we needed to assign it to the config/saved jid. I'm not sure there's harm in it as it is, but its definitely a bit confusing.
There was a problem hiding this comment.
Yeah I'm following the intent here and I think you're right. I don't think there are any issues as is but it is strange. I'll still approve it because I think we should still be fine but out of curiosity, would there be any benefit to this.config.jid = this.initialJid being moved into the else block here? Spitballing more than anything and definitely not saying it's a necessary change, just curious more than anything.
| @@ -643,19 +646,21 @@ export class Client extends EventEmitter { | |||
| } | |||
|
|
|||
| if (this.hardReconnectRequired) { | |||
| let jidPromise: Promise<any>; | |||
| if (this.config.jid) { | |||
There was a problem hiding this comment.
In the previous code, we would automatically resolve our promise if a JID was passed in via config. What's weird though is we also grabbed the value from the promise and assigned it to config.jid even though that value is the same if we end up resolving without needing to fetch via api.
No description provided.