Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Immutable.js Support #30

Closed
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion .jshintrc
Expand Up @@ -2,8 +2,10 @@
"predef": [
"define",
"module",
"Firebase"
"Firebase",
"Immutable"
],
"node": true,
"bitwise": true,
"curly": true,
"eqeqeq": true,
Expand Down
3 changes: 2 additions & 1 deletion bower.json
Expand Up @@ -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"
Expand Down
12 changes: 7 additions & 5 deletions build/header
Expand Up @@ -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);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throw/log a helpful error it's undefined.

var immutableCdn = "https://cdnjs.cloudflare.com/ajax/libs/immutable/3.4.1/"
throw new TypeError(
  'Immutable is undefined.  Please include it in your page.   '
  + 'You can get it at ' + immutableCdn + 'immutable.js or ' + immutableCdn + 'immutable.min.js'
);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should an error get thrown or should just work as it does today?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that in CommonJS/AMD it'll just fail unless you supply a fake version of Immutable with exports undefined. So the browser would behave differently in that it just works with or without it.

UMD dependencies are tricky, and this is going to require a good guide for each tool people might use (none, browserify, webpack, RequireJS).

Edit: actually I'm not sure how to do this in browserify. It's tricky to modify how things in node_modules behave with browserify.

}
}(this, function() {
}(this, function(Immutable) {
"use strict";
1 change: 1 addition & 0 deletions gulpfile.js
Expand Up @@ -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"
]
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -33,6 +33,7 @@
],
"dependencies": {
"firebase": "2.0.x",
"immutable": "3.4.x",
"react": "0.12.x"
},
"devDependencies": {
Expand Down
62 changes: 44 additions & 18 deletions src/reactfire.js
Expand Up @@ -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);
Expand All @@ -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);
}
else {
newState[bindVar] = dataSnapshot.val();
newState[bindVar] = this._toOrderedMap(dataSnapshot);
}
this.setState(newState);
}.bind(this), cancelCallback);
Expand Down Expand Up @@ -109,25 +124,36 @@ var ReactFireMixin = {
}
},


/* Returns true if the inputted object is a JavaScript array */
_isArray: function(obj) {
return (Object.prototype.toString.call(obj) === "[object Array]");
_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 a JavaScript array */
_toArray: function(obj) {
var out = [];
if (obj) {
if (this._isArray(obj)) {
out = obj;
/* Converts a Firebase object to an Immutable List */
_toList: function(snapshot) {
var out = Immutable.List();
if (snapshot) {
if (Immutable.List.isList(snapshot)) {
out = snapshot;
}
else if (typeof(obj) === "object") {
for (var key in obj) {
if (obj.hasOwnProperty(key)) {
out.push(obj[key]);
}
}
else if (typeof(snapshot) === "object") {
out = out.withMutations(function(list) {
snapshot.forEach(function(child) {
list.push(Immutable.fromJS(child.val()));
});
});
}
}
return out;
Expand Down
4 changes: 4 additions & 0 deletions tests/index.html
Expand Up @@ -15,6 +15,9 @@
<!-- Firebase -->
<script src="../bower_components/firebase/firebase.js"></script>

<!-- Immutable.js -->
<script src="../bower_components/immutable/dist/immutable.js"></script>

<!-- ReactFire -->
<script src="../src/reactfire.js"></script>

Expand All @@ -24,5 +27,6 @@
</head>

<body>
<div id="test" />
</body>
</html>
2 changes: 1 addition & 1 deletion tests/karma.conf.js
Expand Up @@ -2,7 +2,7 @@ module.exports = function(config) {
config.set({
frameworks: ["jasmine"],
autowatch: false,
singleRun: true,
singleRun: false,

preprocessors: {
"../src/*.js": "coverage"
Expand Down
71 changes: 67 additions & 4 deletions tests/specs/reactfire.spec.js
Expand Up @@ -97,6 +97,7 @@ describe("ReactFireMixin Tests:", function() {
});

it("bindAsArray() binds to remote Firebase data as an array", function(done) {
var result = Immutable.List([1, 2, 3]);
var TestComponent = React.createClass({
mixins: [ReactFireMixin],

Expand All @@ -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();
},

Expand All @@ -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],

Expand All @@ -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();
},

Expand Down Expand Up @@ -237,6 +239,7 @@ describe("ReactFireMixin Tests:", function() {
});

it("bindAsObject() binds to remote Firebase data as an object", function(done) {
var result = Immutable.OrderedMap({ a: 1, b: 2, c: 3 });
var TestComponent = React.createClass({
mixins: [ReactFireMixin],

Expand All @@ -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();
},

Expand All @@ -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.OrderedMap({ b: 2, c: 3 });
var TestComponent = React.createClass({
mixins: [ReactFireMixin],

Expand All @@ -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();
},

Expand Down Expand Up @@ -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.OrderedMap(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]];
Expand Down Expand Up @@ -482,4 +544,5 @@ describe("ReactFireMixin Tests:", function() {
React.render(new TestComponent(), document.body);
});
});

});