From d3a675d602a2690296ce0d31af5d1895dd75398d Mon Sep 17 00:00:00 2001 From: Justin Tulloss Date: Sat, 27 Dec 2014 16:08:27 -0800 Subject: [PATCH 1/6] Use `renderIntoDocument` react test helper This allows us to view the jasmine results since the components are rendered off screen instead of blowing away the jasmine test runner. --- tests/specs/reactfire.spec.js | 40 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/tests/specs/reactfire.spec.js b/tests/specs/reactfire.spec.js index 0066f028..14029469 100644 --- a/tests/specs/reactfire.spec.js +++ b/tests/specs/reactfire.spec.js @@ -30,7 +30,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("bindAsArray() throws errors given invalid bind variables", function() { @@ -55,7 +55,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("bindAsArray() does not throw errors given valid inputs", function() { @@ -75,7 +75,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("bindAsArray() does not throw an error given a limit query", function() { @@ -93,7 +93,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("bindAsArray() binds to remote Firebase data as an array", function(done) { @@ -118,7 +118,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("bindAsArray() binds to remote Firebase data as an array (limit query)", function(done) { @@ -143,7 +143,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); }); @@ -170,7 +170,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("bindAsObject() throws errors given invalid bind variables", function() { @@ -195,7 +195,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("bindAsObject() does not throw errors given valid inputs", function() { @@ -215,7 +215,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("bindAsObject() does not throw an error given a limit query", function() { @@ -233,7 +233,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("bindAsObject() binds to remote Firebase data as an object", function(done) { @@ -258,7 +258,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("bindAsObject() binds to remote Firebase data as an object (limit query)", function(done) { @@ -283,7 +283,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); }); @@ -310,7 +310,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("unbind() throws errors given unbound bind variable", function() { @@ -335,7 +335,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("unbind() does not throw errors given valid bind variables", function() { @@ -356,7 +356,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("unbind() does not throw an error given a limit query", function() { @@ -377,7 +377,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("unbind() unbinds the state bound to Firebase as an array", function(done) { @@ -404,7 +404,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("unbind() unbinds the state bound to Firebase as an object", function(done) { @@ -431,7 +431,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); }); @@ -460,7 +460,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); it("_bind() does not throw error given valid inputs", function() { @@ -479,7 +479,7 @@ describe("ReactFireMixin Tests:", function() { } }); - React.render(new TestComponent(), document.body); + ReactTestUtils.renderIntoDocument(new TestComponent()); }); }); }); From d567a9b8edc7ebe0e2516f493b2d946a00fd4097 Mon Sep 17 00:00:00 2001 From: Justin Tulloss Date: Sat, 27 Dec 2014 16:38:59 -0800 Subject: [PATCH 2/6] Revert "Use `renderIntoDocument` react test helper" This reverts commit d3a675d602a2690296ce0d31af5d1895dd75398d. --- tests/specs/reactfire.spec.js | 40 +++++++++++++++++------------------ 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/tests/specs/reactfire.spec.js b/tests/specs/reactfire.spec.js index 14029469..0066f028 100644 --- a/tests/specs/reactfire.spec.js +++ b/tests/specs/reactfire.spec.js @@ -30,7 +30,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("bindAsArray() throws errors given invalid bind variables", function() { @@ -55,7 +55,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("bindAsArray() does not throw errors given valid inputs", function() { @@ -75,7 +75,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("bindAsArray() does not throw an error given a limit query", function() { @@ -93,7 +93,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("bindAsArray() binds to remote Firebase data as an array", function(done) { @@ -118,7 +118,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("bindAsArray() binds to remote Firebase data as an array (limit query)", function(done) { @@ -143,7 +143,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); }); @@ -170,7 +170,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("bindAsObject() throws errors given invalid bind variables", function() { @@ -195,7 +195,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("bindAsObject() does not throw errors given valid inputs", function() { @@ -215,7 +215,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("bindAsObject() does not throw an error given a limit query", function() { @@ -233,7 +233,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("bindAsObject() binds to remote Firebase data as an object", function(done) { @@ -258,7 +258,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("bindAsObject() binds to remote Firebase data as an object (limit query)", function(done) { @@ -283,7 +283,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); }); @@ -310,7 +310,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("unbind() throws errors given unbound bind variable", function() { @@ -335,7 +335,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("unbind() does not throw errors given valid bind variables", function() { @@ -356,7 +356,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("unbind() does not throw an error given a limit query", function() { @@ -377,7 +377,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("unbind() unbinds the state bound to Firebase as an array", function(done) { @@ -404,7 +404,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("unbind() unbinds the state bound to Firebase as an object", function(done) { @@ -431,7 +431,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); }); @@ -460,7 +460,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); it("_bind() does not throw error given valid inputs", function() { @@ -479,7 +479,7 @@ describe("ReactFireMixin Tests:", function() { } }); - ReactTestUtils.renderIntoDocument(new TestComponent()); + React.render(new TestComponent(), document.body); }); }); }); From 44655d0e32f53b8c04e3171a9bcdbcf0d7bc9c92 Mon Sep 17 00:00:00 2001 From: Justin Tulloss Date: Sat, 27 Dec 2014 17:51:06 -0800 Subject: [PATCH 3/6] Support Immutable.js in ReactFireMixin ReactFireMixin can be rather slow with many items causing many change events which can trigger many rerenders. By using Immutable data structures we can cheaply check the entire tree and only rerender the components that have actually changed their data. This patch makes bindAsObject bind an Immutable.Map to the specified prop and bindAsArray bind an Immutable.List to the specified prop. It also changes the tests to verify that the data returned is correct. Finally, the patch adds a `boundVarsHaveUpdated` method that will cheaply check for changed state in the bound state variables. It's intended to be used in a more complex `shouldComponentUpdate` method. Tests for the new method are added. --- .jshintrc | 3 +- bower.json | 3 +- gulpfile.js | 1 + src/reactfire.js | 43 +++++++++++++-------- tests/index.html | 4 ++ tests/karma.conf.js | 2 +- tests/specs/reactfire.spec.js | 71 +++++++++++++++++++++++++++++++++-- 7 files changed, 104 insertions(+), 23 deletions(-) diff --git a/.jshintrc b/.jshintrc index 8249e5be..32332897 100644 --- a/.jshintrc +++ b/.jshintrc @@ -2,7 +2,8 @@ "predef": [ "define", "module", - "Firebase" + "Firebase", + "Immutable" ], "bitwise": true, "curly": true, diff --git a/bower.json b/bower.json index e7940415..1471e41f 100644 --- a/bower.json +++ b/bower.json @@ -33,7 +33,8 @@ ], "dependencies": { "react": "0.12.x", - "firebase": "2.0.x" + "firebase": "2.0.x", + "immutable": "3.4.x" }, "devDependencies": { "jasmine": "~2.0.0" diff --git a/gulpfile.js b/gulpfile.js index 0c66962d..788d7643 100644 --- a/gulpfile.js +++ b/gulpfile.js @@ -44,6 +44,7 @@ var paths = { "bower_components/firebase/firebase.js", "tests/phantomjs-es5-shim.js", "bower_components/react/react-with-addons.js", + "bower_components/immutable/dist/immutable.js", "src/*.js", "tests/specs/*.spec.js" ] diff --git a/src/reactfire.js b/src/reactfire.js index 9bb1b9c5..d124c7f5 100644 --- a/src/reactfire.js +++ b/src/reactfire.js @@ -31,6 +31,21 @@ var ReactFireMixin = { this._bind(firebaseRef, bindVar, cancelCallback, false); }, + /* Checks all bound vars to see if they have changed. The check is cheap + * because we are using immutable data structures. Returns true if any of + * the bound vars are updated in `nextState`, otherwise false. + * + * This is intended to be used in a more comprehensive `shouldComponentUpdate`. + */ + boundVarsHaveUpdated: function(nextState) { + for (var varName in nextState) { + if (this.firebaseRefs[varName] && !Immutable.is(this.state && this.state[varName], nextState[varName])) { + return true; + } + } + return false; + }, + /* Creates a binding between Firebase and the inputted bind variable as either an array or object */ _bind: function(firebaseRef, bindVar, cancelCallback, bindAsArray) { this._validateBindVar(bindVar); @@ -55,10 +70,10 @@ var ReactFireMixin = { this.firebaseListeners[bindVar] = firebaseRef.on("value", function(dataSnapshot) { var newState = {}; if (bindAsArray) { - newState[bindVar] = this._toArray(dataSnapshot.val()); + newState[bindVar] = this._toList(dataSnapshot.val()); } else { - newState[bindVar] = dataSnapshot.val(); + newState[bindVar] = Immutable.fromJS(dataSnapshot.val()); } this.setState(newState); }.bind(this), cancelCallback); @@ -109,25 +124,21 @@ var ReactFireMixin = { } }, - - /* Returns true if the inputted object is a JavaScript array */ - _isArray: function(obj) { - return (Object.prototype.toString.call(obj) === "[object Array]"); - }, - - /* Converts a Firebase object to a JavaScript array */ - _toArray: function(obj) { - var out = []; + /* Converts a Firebase object to an Immutable List */ + _toList: function(obj) { + var out = Immutable.List(); if (obj) { - if (this._isArray(obj)) { + if (Immutable.List.isList(obj)) { out = obj; } else if (typeof(obj) === "object") { - for (var key in obj) { - if (obj.hasOwnProperty(key)) { - out.push(obj[key]); + out = out.withMutations(function(list) { + for (var key in obj) { + if (obj.hasOwnProperty(key)) { + list.push(obj[key]); + } } - } + }); } } return out; diff --git a/tests/index.html b/tests/index.html index c51064ec..7b713909 100644 --- a/tests/index.html +++ b/tests/index.html @@ -15,6 +15,9 @@ + + + @@ -24,5 +27,6 @@ +
diff --git a/tests/karma.conf.js b/tests/karma.conf.js index 84a25ee7..3e6108c3 100644 --- a/tests/karma.conf.js +++ b/tests/karma.conf.js @@ -2,7 +2,7 @@ module.exports = function(config) { config.set({ frameworks: ["jasmine"], autowatch: false, - singleRun: true, + singleRun: false, preprocessors: { "../src/*.js": "coverage" diff --git a/tests/specs/reactfire.spec.js b/tests/specs/reactfire.spec.js index 0066f028..375e763b 100644 --- a/tests/specs/reactfire.spec.js +++ b/tests/specs/reactfire.spec.js @@ -97,6 +97,7 @@ describe("ReactFireMixin Tests:", function() { }); it("bindAsArray() binds to remote Firebase data as an array", function(done) { + var result = Immutable.fromJS([1, 2, 3]); var TestComponent = React.createClass({ mixins: [ReactFireMixin], @@ -109,7 +110,7 @@ describe("ReactFireMixin Tests:", function() { }, componentDidUpdate: function(prevProps, prevState) { - expect(this.state).toEqual({ items: [1, 2, 3] }); + expect(Immutable.is(this.state.items, result)).toBe(true); done(); }, @@ -122,6 +123,7 @@ describe("ReactFireMixin Tests:", function() { }); it("bindAsArray() binds to remote Firebase data as an array (limit query)", function(done) { + var result = Immutable.List([2, 3]); var TestComponent = React.createClass({ mixins: [ReactFireMixin], @@ -134,7 +136,7 @@ describe("ReactFireMixin Tests:", function() { }, componentDidUpdate: function(prevProps, prevState) { - expect(this.state).toEqual({ items: [2, 3] }); + expect(Immutable.is(this.state.items, result)).toBe(true); done(); }, @@ -237,6 +239,7 @@ describe("ReactFireMixin Tests:", function() { }); it("bindAsObject() binds to remote Firebase data as an object", function(done) { + var result = Immutable.fromJS({ a: 1, b: 2, c: 3 }); var TestComponent = React.createClass({ mixins: [ReactFireMixin], @@ -249,7 +252,7 @@ describe("ReactFireMixin Tests:", function() { }, componentDidUpdate: function(prevProps, prevState) { - expect(this.state).toEqual({ items: { a: 1, b: 2, c: 3 } }); + expect(Immutable.is(this.state.items, result)).toBe(true); done(); }, @@ -262,6 +265,7 @@ describe("ReactFireMixin Tests:", function() { }); it("bindAsObject() binds to remote Firebase data as an object (limit query)", function(done) { + var result = Immutable.Map({ b: 2, c: 3 }); var TestComponent = React.createClass({ mixins: [ReactFireMixin], @@ -274,7 +278,7 @@ describe("ReactFireMixin Tests:", function() { }, componentDidUpdate: function(prevProps, prevState) { - expect(this.state).toEqual({ items: { b: 2, c: 3 } }); + expect(Immutable.is(this.state.items, result)).toBe(true); done(); }, @@ -435,6 +439,64 @@ describe("ReactFireMixin Tests:", function() { }); }); + describe("boundVarsHaveUpdated(): ", function() { + it("boundVarsHaveUpdated() returns false if bound objects haven't changed", function(done) { + var data = { a: 1, b: 2, c: 3 }; + var TestComponent = React.createClass({ + mixins: [ReactFireMixin], + + componentWillMount: function() { + this.bindAsObject(firebaseRef, "items"); + }, + + componentDidMount: function() { + firebaseRef.set(data); + }, + + componentDidUpdate: function(prevProps, prevState) { + expect(this.boundVarsHaveUpdated({ + items: Immutable.Map(data) + })).toBe(false); + done(); + }, + + render: function() { + return React.DOM.div(null, "Testing"); + } + }); + + React.render(new TestComponent(), document.body); + }); + + it("boundVarsHaveUpdated() returns true if bound objects have changed", function(done) { + var data = { a: 1, b: 2, c: 3 }; + var TestComponent = React.createClass({ + mixins: [ReactFireMixin], + + componentWillMount: function() { + this.bindAsObject(firebaseRef, "items"); + }, + + componentDidMount: function() { + firebaseRef.set(data); + }, + + componentDidUpdate: function(prevProps, prevState) { + expect(this.boundVarsHaveUpdated({ + items: Immutable.Map({ a: 5, b: 6 }) + })).toBe(true); + done(); + }, + + render: function() { + return React.DOM.div(null, "Testing"); + } + }); + + React.render(new TestComponent(), document.body); + }); + }); + describe("_bind():", function() { it("_bind() throws errors given invalid third input parameter", function() { var nonBooleanParams = [null, undefined, [], {}, 0, 5, "", "a", {a : 1}, ["hi", 1]]; @@ -482,4 +544,5 @@ describe("ReactFireMixin Tests:", function() { React.render(new TestComponent(), document.body); }); }); + }); From 09e6eb9754603f6de7446ae07979349b0edc4210 Mon Sep 17 00:00:00 2001 From: Justin Tulloss Date: Sun, 28 Dec 2014 14:56:18 -0800 Subject: [PATCH 4/6] Maintain order returned by data snapshots - Iterate over snapshots using `forEach` to retain order. - Use OrderedMap instead of Map for bindAsObject --- src/reactfire.js | 39 ++++++++++++++++++++++++----------- tests/specs/reactfire.spec.js | 8 +++---- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/src/reactfire.js b/src/reactfire.js index d124c7f5..4a59281c 100644 --- a/src/reactfire.js +++ b/src/reactfire.js @@ -70,10 +70,10 @@ var ReactFireMixin = { this.firebaseListeners[bindVar] = firebaseRef.on("value", function(dataSnapshot) { var newState = {}; if (bindAsArray) { - newState[bindVar] = this._toList(dataSnapshot.val()); + newState[bindVar] = this._toList(dataSnapshot); } else { - newState[bindVar] = Immutable.fromJS(dataSnapshot.val()); + newState[bindVar] = this._toOrderedMap(dataSnapshot); } this.setState(newState); }.bind(this), cancelCallback); @@ -124,20 +124,35 @@ var ReactFireMixin = { } }, + _toOrderedMap: function(snapshot) { + var out = Immutable.OrderedMap(); + if (snapshot) { + if (Immutable.OrderedMap.isOrderedMap(snapshot)) { + out = snapshot; + } + else if (typeof(snapshot) === "object") { + out = out.withMutations(function(map) { + snapshot.forEach(function(child) { + map.set(child.key(), Immutable.fromJS(child.val())); + }); + }); + } + } + return out; + }, + /* Converts a Firebase object to an Immutable List */ - _toList: function(obj) { + _toList: function(snapshot) { var out = Immutable.List(); - if (obj) { - if (Immutable.List.isList(obj)) { - out = obj; + if (snapshot) { + if (Immutable.List.isList(snapshot)) { + out = snapshot; } - else if (typeof(obj) === "object") { + else if (typeof(snapshot) === "object") { out = out.withMutations(function(list) { - for (var key in obj) { - if (obj.hasOwnProperty(key)) { - list.push(obj[key]); - } - } + snapshot.forEach(function(child) { + list.push(Immutable.fromJS(child.val())); + }); }); } } diff --git a/tests/specs/reactfire.spec.js b/tests/specs/reactfire.spec.js index 375e763b..6db2cd3e 100644 --- a/tests/specs/reactfire.spec.js +++ b/tests/specs/reactfire.spec.js @@ -97,7 +97,7 @@ describe("ReactFireMixin Tests:", function() { }); it("bindAsArray() binds to remote Firebase data as an array", function(done) { - var result = Immutable.fromJS([1, 2, 3]); + var result = Immutable.List([1, 2, 3]); var TestComponent = React.createClass({ mixins: [ReactFireMixin], @@ -239,7 +239,7 @@ describe("ReactFireMixin Tests:", function() { }); it("bindAsObject() binds to remote Firebase data as an object", function(done) { - var result = Immutable.fromJS({ a: 1, b: 2, c: 3 }); + var result = Immutable.OrderedMap({ a: 1, b: 2, c: 3 }); var TestComponent = React.createClass({ mixins: [ReactFireMixin], @@ -265,7 +265,7 @@ describe("ReactFireMixin Tests:", function() { }); it("bindAsObject() binds to remote Firebase data as an object (limit query)", function(done) { - var result = Immutable.Map({ b: 2, c: 3 }); + var result = Immutable.OrderedMap({ b: 2, c: 3 }); var TestComponent = React.createClass({ mixins: [ReactFireMixin], @@ -455,7 +455,7 @@ describe("ReactFireMixin Tests:", function() { componentDidUpdate: function(prevProps, prevState) { expect(this.boundVarsHaveUpdated({ - items: Immutable.Map(data) + items: Immutable.OrderedMap(data) })).toBe(false); done(); }, From 009c993d5482148fffc2c8c68cf5596b491f5212 Mon Sep 17 00:00:00 2001 From: Justin Tulloss Date: Sun, 28 Dec 2014 15:06:30 -0800 Subject: [PATCH 5/6] Fix build to work with npm Immutable needs to be imported in environments that enforce that. This changes the build to require Immutable appropriately, and lists it as a dependency of the node module. --- .jshintrc | 1 + build/header | 12 +++++++----- package.json | 1 + 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.jshintrc b/.jshintrc index 32332897..100fcc36 100644 --- a/.jshintrc +++ b/.jshintrc @@ -5,6 +5,7 @@ "Firebase", "Immutable" ], + "node": true, "bitwise": true, "curly": true, "eqeqeq": true, diff --git a/build/header b/build/header index 6a63bef9..b3d3222b 100644 --- a/build/header +++ b/build/header @@ -10,17 +10,19 @@ ;(function (root, factory) { "use strict"; + var Immutable = root.Immutable; if (typeof define === "function" && define.amd) { // AMD - define([], function() { - return (root.ReactFireMixin = factory()); + define(["../bower_components/immutable/dist/immutable.min.js"], function(Immutable) { + return (root.ReactFireMixin = factory(Immutable)); }); } else if (typeof exports === "object") { // CommonJS - module.exports = factory(); + Immutable = require("immutable"); + module.exports = factory(Immutable); } else { // Global variables - root.ReactFireMixin = factory(); + root.ReactFireMixin = factory(Immutable); } -}(this, function() { +}(this, function(Immutable) { "use strict"; diff --git a/package.json b/package.json index 778f01f5..feafaaad 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ ], "dependencies": { "firebase": "2.0.x", + "immutable": "3.4.x", "react": "0.12.x" }, "devDependencies": { From 391dad3c9967287a0d31d97a8de06115e40c128b Mon Sep 17 00:00:00 2001 From: Justin Tulloss Date: Tue, 27 Jan 2015 16:28:47 -0800 Subject: [PATCH 6/6] Save a reference to the snapshot on the immutable value This allows us to later get a firebase reference to it and to get it's priority, among other nice things. --- src/reactfire.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/reactfire.js b/src/reactfire.js index 4a59281c..d520452a 100644 --- a/src/reactfire.js +++ b/src/reactfire.js @@ -133,7 +133,9 @@ var ReactFireMixin = { else if (typeof(snapshot) === "object") { out = out.withMutations(function(map) { snapshot.forEach(function(child) { - map.set(child.key(), Immutable.fromJS(child.val())); + var immutableChild = Immutable.fromJS(child.val()); + immutableChild.snapshot = child; + map.set(child.key(), immutableChild); }); }); } @@ -151,7 +153,9 @@ var ReactFireMixin = { else if (typeof(snapshot) === "object") { out = out.withMutations(function(list) { snapshot.forEach(function(child) { - list.push(Immutable.fromJS(child.val())); + var immutableChild = Immutable.fromJS(child.val()); + immutableChild.snapshot = child; + list.push(immutableChild); }); }); }