nuclear-js
nuclear-js copied to clipboard
optimizely
grouping observers by getters in addObserver
This PR is a better implementation of the grouping observers by getters idea in
https://github.com/optimizely/nuclear-js/pull/182
The difference is observers are grouped when they are added. Also the removal of observers has been taken care of too.
@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.
@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'"
@lyonlai hm, ill try cloning a fresh repo and see if anything is missing
@jordangarcia while you are doing that I'll run karma manually to see if I can get the test running.
@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?
@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?
looks like the version problem. Was using node 4. karma running after change. I'll dig into the test now.
@jordangarcia I've fixed the failing tests, added two more tests about the getters map and fixed the formatting issue.
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