fix: handle subscribed called multiple times; remove redundant line (#489)

on multiple subscribe calls, we ignore if we do not need to act instead of raising an exception

it also removes a line of code that was creating issues on channel removal: #488

Author: filipecabaco

src/RealtimeChannel.ts

@@ -134,7 +134,7 @@ export default class RealtimeChannel {
     }[]
   } = {}
   timeout: number
-  state = CHANNEL_STATES.closed
+  state: CHANNEL_STATES = CHANNEL_STATES.closed
   joinedOnce = false
   joinPush: Push
   rejoinTimer: Timer
@@ -217,9 +217,7 @@ export default class RealtimeChannel {
     if (!this.socket.isConnected()) {
       this.socket.connect()
     }
-    if (this.joinedOnce) {
-      throw `tried to subscribe multiple times. 'subscribe' can only be called a single time per channel instance`
-    } else {
+    if (this.state == CHANNEL_STATES.closed) {
       const {
         config: { broadcast, presence, private: isPrivate },
       } = this.params

src/RealtimeClient.ts

@@ -282,7 +282,6 @@ export default class RealtimeClient {
     channel: RealtimeChannel
   ): Promise<RealtimeRemoveChannelResponse> {
     const status = await channel.unsubscribe()
-    this.channels = this.channels.filter((c) => c._joinRef !== channel._joinRef)
 
     if (this.channels.length === 0) {
       this.disconnect()

test/channel.test.ts

@@ -138,15 +138,27 @@ describe('subscribe', () => {
     assert.ok(channel.joinedOnce)
   })
 
-  test('throws if attempting to join multiple times', () => {
+  test('if attempting to join multiple times, ignores calls', () => {
     channel.subscribe()
+    while (channel.state == CHANNEL_STATES.closed) clock.tick(100)
+    const state = channel.state
 
-    assert.throws(
-      () => channel.subscribe(),
-      /tried to subscribe multiple times/
-    )
+    for (let i = 0; i < 10; i++) channel.subscribe()
+
+    assert.equal(channel.state, state)
   })
+  test('if subscription closed and then subscribe, it will rejoin', () => {
+    channel.subscribe()
+    while (channel.state == CHANNEL_STATES.closed) clock.tick(100)
+
+    channel.unsubscribe()
+    while ((channel.state as CHANNEL_STATES) !== CHANNEL_STATES.closed) {
+      clock.tick(100)
+    }
+    channel.subscribe()
 
+    assert.equal(channel.state, CHANNEL_STATES.joining)
+  })
   test('updates join push payload access token', () => {
     socket.accessTokenValue = 'token123'
 

test/client.test.ts

@@ -283,6 +283,24 @@ describe('channel', () => {
     assert.ok(disconnectStub.called)
   })
 
+  test('does not remove other channels when removing one', async () => {
+    const connectStub = sinon.stub(socket, 'connect')
+    const disconnectStub = sinon.stub(socket, 'disconnect')
+    const channel1 = socket.channel('chan1').subscribe()
+    const channel2 = socket.channel('chan2').subscribe()
+
+    channel1.subscribe()
+    channel2.subscribe()
+    assert.equal(socket.getChannels().length, 2)
+    assert.ok(connectStub.called)
+
+    await socket.removeChannel(channel1)
+
+    assert.equal(socket.getChannels().length, 1)
+    assert.ok(!disconnectStub.called)
+    assert.deepStrictEqual(socket.getChannels()[0], channel2)
+  })
+
   test('removes all channels', async () => {
     const disconnectStub = sinon.stub(socket, 'disconnect')