grouping observers by getters in addObserver

[lyonlai]

This PR is a better implementation of the grouping observers by getters idea in

#182

The difference is observers are grouped when they are added. Also the removal of observers has been taken care of too.

[jordangarcia]

please use the style:

let updatedObserverState = observerState.updateIn(['gettersMap', getter], observerIds => {
  return observerIds ? observerIds.add(currId) : Immutable.Set.of(currId)
})

[jordangarcia]

@lyonlai I checked out your branch and couldn't get tests to pass using grunt karma:chrome and running them in debug mode.

I had initially tried an approach where the observerMap was using a getter by reference as a key, however I couldn't get it it to pass tests.

If you are able to get tests to pass I'm happy to include.

[lyonlai]

@jordangarcia I'll get back to you soon on this.

[lyonlai]

@jordangarcia looks like I've got some missing files when I tried to run the test. after I've install a bunch of missing npm packages now I've got "Cannot find module './common'"

[jordangarcia]

@lyonlai hm, ill try cloning a fresh repo and see if anything is missing

[lyonlai]

@jordangarcia while you are doing that I'll run karma manually to see if I can get the test running.

[jordangarcia]

@lyonlai it's working for me, not sure if it's becuase I have a globally installed module that maybe you don't have.

Do you have karma install globally?

Are you running on OSX?

[lyonlai]

@jordangarcia I don't have things globally. I think that's why. I'm on osx. btw what node version you are running. and also npm version?

[jordangarcia]

I'm running node v0.12.5 (npm v2.11.2)

[lyonlai]

looks like the version problem. Was using node 4. karma running after change. I'll dig into the test now.

[lyonlai]

@jordangarcia I've fixed the failing tests, added two more tests about the getters map and fixed the formatting issue.

[jordangarcia]

I dont think this is correct.

If we observe add two observer entries with the same getter and then unobserve one it will unobserve all entries with that getter.

[lyonlai]

that's why it's under the condition, only removed when there's no entries listening to that getter

if (map.getIn(['gettersMap', getter]).size <= 0) 

[lyonlai]

I can add a test case to cover it if you want.

[jordangarcia]

Thanks! This looks good.

[jordangarcia]

I'm going to release 1.2 first since i've done more extensive testing using Optimizely's integration tests.

After 1.2 is released I will merge this and put in 1.2.1

[lyonlai]

sure Thanks @jordangarcia .