If an observer is removed during notify, then some observers will not be run.

[c-spencer]

Offending line: https://github.com/optimizely/nuclear-js/blob/master/src/change-observer.js#L30

__observers is iterated over using a simple forEach, meaning that mutations to __observers during iteration will lead to skipping of some later observers.

Minimal reproduction:

var testReactor = new Reactor();

var test = new Nuclear.Store({
  getInitialState() {
    return 1;
  },

  initialize() {
    this.on('TEST', (_state, v) => v)
  }
});

testReactor.registerStores({ test });

// Setup a first logger.
testReactor.observe(['test'], () => console.log('logger 1'));
console.log('-- test 1 --');
testReactor.dispatch('TEST', 2);
// All above runs fine

// Setup an unregistering observer and a second logger.
var unreg = testReactor.observe(['test'], () => unreg());
testReactor.observe(['test'], () => console.log('logger 2'));

// No logger 2 on this trigger
console.log('-- test 2 --');
testReactor.dispatch('TEST', 3);

// Now fine
console.log('-- test 3 --');
testReactor.dispatch('TEST', 4);

Prints:

-- test 1 --
logger 1
-- test 2 --
logger 1
-- test 3 --
logger 1
logger 2

Missing a logging observation on test 2. This removing of observers during a notify is very likely to occur when using with React, where a state change causes the displayed components to unmount. (This manifested in my application as an inner component properly re-rendering but an outer one depending on the same state to not do so, leading to broken UI. Confusingly, this only occurred when a particular state was first loaded, due to the dependency on ordering of observers for the bug to occur, much fun.)

[bhamodi]

Thanks for filing this @c-spencer.