Skip to content

Conversation

Shiranuit
Copy link
Contributor

@Shiranuit Shiranuit commented Aug 10, 2021

What does this PR do ?

This PR make a slight changes in the way we loop over the Map.keys() in the saveSubscription, resubscribe and removeSubscriptions method, because Webpack is incorrectly transpiling the loops making those methods not working properly.

Instead of iterating this way

for (const roomId of this._subscriptions.keys()) {
  // Body
}

The iteration is now done using Map.forEeach

this._subscriptions.forEach((rooms, roomId) => {
  // Body
});

Why those changes

Like I said, those changes are made to fix an issue where the resubscription mecanism wasn't working in the JS SDK but only in the browser.
This was caused by a bad transpilation of the loops using Webpack:

for (const roomId of this._subscriptions.keys())

That were transpiled to:

for(let e=0,t=this._subscriptions.keys();e<t.length;e++)

Since _subscriptions is a Map object, and Map.keys() return a MapIterator that doesn't have the property length, the loop wasn't executed since the condition was invalid.

@codecov
Copy link

codecov bot commented Aug 10, 2021

Codecov Report

Merging #658 (06fcc90) into 7-dev (1f04b94) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 06fcc90 differs from pull request most recent head aa81470. Consider uploading reports for the commit aa81470 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##            7-dev     #658   +/-   ##
=======================================
  Coverage   85.65%   85.65%           
=======================================
  Files          36       36           
  Lines        1708     1708           
  Branches      309      309           
=======================================
  Hits         1463     1463           
  Misses        183      183           
  Partials       62       62           
Impacted Files Coverage Δ
src/controllers/Realtime.ts 87.87% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1f04b94...aa81470. Read the comment docs.

@Shiranuit Shiranuit changed the title Subscription not working in the browser because of a bad transpilation Resubscription not working in the browser because of a bad transpilation Aug 10, 2021
@xbill82
Copy link
Contributor

xbill82 commented Aug 10, 2021

Looks like it's not a bug from Webpack but rather Babel (or any other transpiler you are using through Webpack). Are we considering submitting this issue to the author of the transpiler?

@Shiranuit
Copy link
Contributor Author

Looks like it's not a bug from Webpack but rather Babel (or any other transpiler you are using through Webpack). Are we considering submitting this issue to the author of the transpiler?

You're right I made a shortcut here, we're using babel, for now I didn't tried to research whether that was a known bug or not, but it might be good to make an issue if that was not already addressed.

@Shiranuit Shiranuit merged commit 06e33b9 into 7-dev Aug 10, 2021
@Shiranuit Shiranuit mentioned this pull request Aug 10, 2021
@rolljee rolljee deleted the fix-resubscription-not-working-in-browser branch August 29, 2023 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants