tejacques / crosstab

A utility library for cross-tab communication using localStorage.
Apache License 2.0
364 stars 58 forks source link

Expose crosstab.isMaster? #33

Closed chadly closed 8 years ago

chadly commented 8 years ago

I see in the code you have an isMaster function that is not being used anywhere. I was wondering why that is not exposed. It seems like it would be useful.

I'm trying to make my app only open a websocket connection on the master tab. Maybe I am approaching it wrong. My idea was to do something like this:

var crosstab = require("crosstab");

if (crosstab.isMaster()) {
  //open websocket connection & broadcast on crosstab when websocket messages come in
} else {
  //listen for crosstab events
}

How would I go about making this work?

tejacques commented 8 years ago

Unfortunately it's been a while since that was written, so I don't remember the exact reason that's not used or exposed. There are several places that use that exact check elsewhere in the code, so it might be worth consolidating them (you're welcome to make a PR!).

As for opening a websocket on the master tab -- you should actually do this using the util.eventTypes.becomeMaster event. I believe part of why I didn't expose that function is that the isMaster() function only checks whether or not you are the master at the current point in time, so it's not well suited to handling what should be done when a tab becomes the master.

There actually isn't a lostMaster event, but your comment makes me realize that there should be so that resources can be disconnected and cleaned up.

I will add that event soon.

chadly commented 8 years ago

Ah, that makes more sense, using crosstab.on(crosstab.util.eventTypes.becomeMaster).

As far as the lostMaster event - is it possible for a tab to lose it's "master status"? What would trigger that happening? I know if the master tab is closed, another tab would become master, but the closed tab ceases to exist, so does it need a lostMaster event? Is there a possibility of a new tab "stealing" the master status from the current master?

tejacques commented 8 years ago

You're right that it should mostly be unnecessary. It's primarily an edge cases / failure recovery thing.

Here's an example:

Tab 1 (Master) Tab 2

Tab 1 has a long blocking call that lasts for ~10 seconds Tab 2 declared Tab 1 has died, and becomes master Tab 1 recovers and sees that Tab 2 now believes itself to be master (a tab promoted event has occurred)

There are two possibilities from here:

  1. Tab 1 replies saying "I'm not dead yet, I'm taking master back!"
  2. Tab 1 accepts that Tab 2 is now the master

In both scenarios, we'll want to downgrade one of the tabs and perform any necessary cleanup.

tejacques commented 8 years ago

I've added a new event demotedFromMaster. Haven't released it to NPM yet, but will drop soon with some other fixes. I also tidied up the isMaster and I think for now it makes sense to not expose it.

Thanks for pointing this out!

chadly commented 8 years ago

Thanks! That makes sense.