Skip to content
This repository has been archived by the owner on Aug 23, 2019. It is now read-only.

fix: resolve transport sort order in browsers #333

Merged
merged 2 commits into from
Apr 16, 2019
Merged

Conversation

jacobheun
Copy link
Contributor

@jacobheun jacobheun commented Apr 15, 2019

This fixes an issue with how browsers handle sorting. In Chrome, the existing code would always sort Circuit last. In Firefox, it does not. The existing logic doesn't take any consideration into the b argument of sort. The problem with this is you will get unreliable behavior because the a and b values are sorted as "equal" if a is not "Circuit".

This PR changes the logic to always prioritize a if it is not "Circuit", and if it is, de-prioritize it.

The caveat of this brief sort logic is that the other transports will have different orders in different browsers. This doesn't matter to Switch, it only cares that Circuit is last. This is why the test is set to only validate that part of the order.

required by ipfs/interop#63

@dirkmc
Copy link

dirkmc commented Apr 16, 2019

It may make testing more deterministic if the behaviour is the same across browsers (eg sort such that Circuit is last, and then by alphabetical order of transport.tag)

@jacobheun
Copy link
Contributor Author

It may make testing more deterministic if the behaviour is the same across browsers (eg sort such that Circuit is last, and then by alphabetical order of transport.tag)

That's a good point, although it shouldn't matter for non circuit transports, I went ahead and updated it to sort consistently just to be sure.

@jacobheun jacobheun merged commit 951e0c9 into master Apr 16, 2019
@jacobheun jacobheun deleted the fix/transport-sort branch April 16, 2019 09:27
@ghost ghost removed the in progress label Apr 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants