Merge pull request #67 from ThunderCatsJS/fix/infinite-loop

[fix] Prevent infinite loops

Author: Berkeley Martinez

CHANGELOG.md

@@ -1,3 +1,20 @@
+# December 22, 2015 v0.5.1
+
+This release fixes the following situation.
+
+If an action were to take place during a `componentWillMount`, this
+will cause an infinite loop as the action triggers `combineLatest`
+to emit an event, which then triggers another `renderToString`
+which starts the whole thing over again.
+
+There is a `first` filter in the stream, but internally the `onNext` of
+the observer is called before the `onCompleted` of the observer can be
+called, meaning the `first` filter never has a chance to call `onCompleted`
+
+Moral of the story? Beware of Zalgo!
+
+* [08411fe](../../commit/08411fe) [fix] Prevent infinite loops
+
 # December 20, 2015 v0.5.0
 
 This release is a breaking change. It depends on internal API changes introduced

package.json

@@ -1,7 +1,7 @@
 {
   "name": "thundercats-react",
   "description": "Thundercats addon for use with React",
-  "version": "0.5.0",
+  "version": "0.5.1",
   "homepage": "http://thundercats.js.org",
   "keywords": [
     "alwaysUseTwoSpaces",

src/Render.js

@@ -82,14 +82,16 @@ export function renderToString$(cat, Component) {
   }
 
   return fetch$(fetchMap)
+    // move fetch to next event loop to prevent
+    // synchronous actions from causing infiniti loop
+    .delay(50)
     .map((fetchMap) => {
       const markup = renderToString(Burrito);
       return {
         markup,
         fetchMap
       };
     })
-    .first()
     .tapOnNext(() => cat.fetchMap = null);
 }
 

test/render.js

@@ -230,6 +230,78 @@ describe('renderToString$', function() {
         );
     });
 
+    it('should prevent infinite loops', done => {
+      const element = React.createElement(createContainer(
+        {
+          actions: ['catActions'],
+          store: 'catStore',
+          fetchAction: 'catActions.doAction',
+          getPayload: () => ({})
+        },
+        createClass({
+          displayName: 'Test',
+          propTypes: {
+            catActions: React.PropTypes.object
+          },
+          componentWillMount() {
+            this.props.catActions.doThis();
+          },
+
+          render() {
+            return React.createElement('h1', null, 'hello world');
+          }
+        })
+      ));
+
+      const CatActions = Actions({
+        refs: { displayName: 'catActions' },
+        doAction() {
+          return Observable.just({ set: { foo: 'baz' } }).delay(500);
+        },
+        doThis() {
+          return { set: { qux: 'quo' } };
+        }
+      });
+
+      const CatStore = Store({
+        refs: {
+          value: { foo: 'bar' },
+          displayName: 'CatStore'
+        },
+        init({ instance: store, args: [cat] }) {
+          const actions = cat.getActions('catActions');
+          store.register(actions.doAction);
+          store.register(actions.doThis);
+        }
+      });
+
+
+      const cat = Cat()();
+
+      cat.register(CatActions);
+      cat.register(CatStore, null, cat);
+
+      renderToString$(cat, element)
+        .flatMap(
+          dehydrate(cat),
+        )
+        .doOnNext(({ CatStore })=> {
+          assert.equal(
+            CatStore.foo,
+            'baz',
+            'foo did not equal baz'
+          );
+
+          assert.isNotOk(CatStore.qux);
+        })
+        .subscribe(
+          () => {},
+          done,
+          done
+        );
+
+    });
+
     it('should error on fetch errors', (done) => {
       renderToString$(cat, 'not the momma')
         .subscribe(