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.

Sign up for GitHub

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

Added support of shouldComponentUpdate #39

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Added support of shouldComponentUpdate #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e7d97fd
Added support of shouldComponentUpdate
medyo May 10, 2015
00d68b5
added shouldComponentUpdate documentation
medyo May 12, 2015
c5d7301
fixed Trigger Update
medyo May 12, 2015
fbbe8e4
Fixed tests and add shouldComponentUpdate tests
medyo May 12, 2015
2c4287d
fixed a little mistake
medyo May 12, 2015
ddfab83
fixed a little mistake
medyo May 12, 2015
e693bd5
removed an unnecessary verification
medyo May 12, 2015
05ee271
added new test for forceUpdate
medyo May 12, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,22 @@ React.BackboneMixin({
});
```

If in some cases you need to control whatever a change should re-render the view or not:
```javascript
var UserViewComponent = React.createBackboneClass({
shouldComponentUpdate: function() {
return this.getModel().previous('name') != this.getModel().get('name');
},
render: function() {
return (
<div>
<h1>{this.getModel().get("name")}</h1>
</div>
);
}
});
```

If your Collection or Model class does not inherit directly from Backbone.Model
or Backbone.Collection, you may customize the behavior on a library level by
overriding the React.BackboneMixin.ConsiderAsCollection function.
Expand Down
117 changes: 97 additions & 20 deletions __tests__/react.backbone-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,23 +20,23 @@ describe('react.backbone', function() {

it('renders model as-is', function() {
var user = new Backbone.Model({name: "Clay"});
var userViewRef = UserView({model: user});
var userViewRef = React.createFactory(UserView)({model: user});
var userView = TestUtils.renderIntoDocument(userViewRef);

var header = TestUtils.findRenderedDOMComponentWithTag(userView, 'h1');
expect(header.getDOMNode().textContent).toEqual('Clay');
});

it('renders model after changes to property', function() {
var user = new Backbone.Model({name: "Clay"});
var userViewRef = UserView({model: user});
var userViewRef = React.createFactory(UserView)({model: user});
var userView = TestUtils.renderIntoDocument(userViewRef);
user.set("name", "David");

var header = TestUtils.findRenderedDOMComponentWithTag(userView, 'h1');
expect(header.getDOMNode().textContent).toEqual('David');
});

describe("with changeOptions", function() {
var UserView = React.createBackboneClass({
changeOptions: "change:name",
Expand All @@ -51,7 +51,7 @@ describe('react.backbone', function() {

it('doesnt render if other property is changed', function() {
var user = new Backbone.Model({name: "Clay", age: "80"});
var userViewRef = UserView({model: user});
var userViewRef = React.createFactory(UserView)({model: user});
var userView = TestUtils.renderIntoDocument(userViewRef);
user.set("age", "60");

Expand All @@ -61,22 +61,65 @@ describe('react.backbone', function() {

it('does render if valid property is changed', function() {
var user = new Backbone.Model({name: "Clay", age: "80"});
var userViewRef = UserView({model: user});
var userViewRef = React.createFactory(UserView)({model: user});
var userView = TestUtils.renderIntoDocument(userViewRef);
user.set("name", "David");

var header = TestUtils.findRenderedDOMComponentWithTag(userView, 'h1');
expect(header.getDOMNode().textContent).toEqual('David 80');
});

});

describe("with shouldComponentUpdate", function() {
var UserView = React.createBackboneClass({
shouldComponentUpdate: function(){
return this.getModel().previous('name') != this.getModel().get('name');
},
render: function() {
return (
<div>
<h1>{this.getModel().get("name")} {this.getModel().get("age")}</h1>
</div>
);
}
});

it('doesnt auto-update if shouldComponentUpdate is false', function() {

var user = new Backbone.Model({name: "Mehdi", age: "80"});
var userViewRef = React.createFactory(UserView)({model: user});
var userView = TestUtils.renderIntoDocument(userViewRef);
user.set("age", "100");

var header = TestUtils.findRenderedDOMComponentWithTag(userView, 'h1');
expect(header.getDOMNode().textContent).toEqual('Mehdi 80');
});

it('forces update even if shouldComponentUpdate is false', function() {
var user = new Backbone.Model({name: "Mehdi", age: "80"});
var userViewRef = React.createFactory(UserView)({model: user});
var userView = TestUtils.renderIntoDocument(userViewRef);
user.set("age", "100");

var header = TestUtils.findRenderedDOMComponentWithTag(userView, 'h1');
expect(header.getDOMNode().textContent).toEqual('Mehdi 80');

userView.forceUpdate()
expect(header.getDOMNode().textContent).toEqual('Mehdi 100');
});




});
});

describe("with :collection key", function() {
var UsersListView = React.createBackboneClass({
render: function() {
var usersList = this.getCollection().map(function(user) {
return <li>{user.get("name")}</li>;
var usersList = this.getCollection().map(function(user, index) {
return <li key={index} >{user.get("name")}</li>;
});

return (
Expand All @@ -89,7 +132,7 @@ describe('react.backbone', function() {

it('renders collection as-is', function() {
var usersList = new Backbone.Collection([{name: "Clay"}, {name: "David"}]);
var usersListViewRef = UsersListView({collection: usersList});
var usersListViewRef = React.createFactory(UsersListView)({collection: usersList});
var usersListView = TestUtils.renderIntoDocument(usersListViewRef);

jest.runOnlyPendingTimers();
Expand All @@ -102,7 +145,7 @@ describe('react.backbone', function() {
it('renders collection on adding', function() {

var usersList = new Backbone.Collection([{name: "Clay"}, {name: "David"}]);
var usersListViewRef = UsersListView({collection: usersList});
var usersListViewRef = React.createFactory(UsersListView)({collection: usersList});
var usersListView = TestUtils.renderIntoDocument(usersListViewRef);
usersList.add({name: "Jack"});

Expand All @@ -112,6 +155,41 @@ describe('react.backbone', function() {
expect(list.getDOMNode().childNodes.length).toEqual(3);
expect(list.getDOMNode().childNodes[2].textContent).toEqual("Jack");
});

describe("with shouldComponentUpdate", function() {
it('doesnt auto-update if shouldComponentUpdate is false', function() {

var UsersListView = React.createBackboneClass({
shouldComponentUpdate: function(){
return false;
},
render: function() {
var usersList = this.getCollection().map(function(user, index) {
return <li key={index} >{user.get("name")}</li>;
});

return (
<ul>
{ usersList }
</ul>
);
}
});

var usersList = new Backbone.Collection([{name: "Mehdi"}, {name: "David"}]);
var usersListViewRef = React.createFactory(UsersListView)({collection: usersList});
var usersListView = TestUtils.renderIntoDocument(usersListViewRef);
usersList.add({name: "Jack"});

jest.runOnlyPendingTimers();

var list = TestUtils.findRenderedDOMComponentWithTag(usersListView, 'ul');
expect(list.getDOMNode().childNodes.length).toEqual(2);
expect(list.getDOMNode().childNodes[2]).toEqual(null);

});

});
});

describe("with mixins", function() {
Expand All @@ -133,7 +211,7 @@ describe('react.backbone', function() {
it("should render mixins as-is", function() {
var user = new Backbone.Model({name: "Clay"});
var wall = new Backbone.Model({post_count: 5});
var profileViewRef = ProfileView({user: user, wall: wall});
var profileViewRef = React.createFactory(ProfileView)({user: user, wall: wall});
var profileView = TestUtils.renderIntoDocument(profileViewRef);

var header = TestUtils.findRenderedDOMComponentWithTag(profileView, 'h1');
Expand All @@ -147,7 +225,7 @@ describe('react.backbone', function() {
it("should re-render if either mixin model is changed", function() {
var user = new Backbone.Model({name: "Clay"});
var wall = new Backbone.Model({post_count: 5});
var profileViewRef = ProfileView({user: user, wall: wall});
var profileViewRef = React.createFactory(ProfileView)({user: user, wall: wall});
var profileView = TestUtils.renderIntoDocument(profileViewRef);

user.set("name", "David");
Expand Down Expand Up @@ -177,7 +255,7 @@ describe('react.backbone', function() {
});
it("should use that prop", function() {
var user = new Backbone.Model({name: "Clay"});
var userViewRef = UserView({user_model: user});
var userViewRef = React.createFactory(UserView)({user_model: user});
var userView = TestUtils.renderIntoDocument(userViewRef);

var header = TestUtils.findRenderedDOMComponentWithTag(userView, 'h1');
Expand All @@ -203,7 +281,7 @@ describe('react.backbone', function() {
});
it("should use that event", function() {
var user = new Backbone.Model({name: "Clay", age: 25});
var userViewRef = UserView({user_model: user});
var userViewRef = React.createFactory(UserView)({user_model: user});
var userView = TestUtils.renderIntoDocument(userViewRef);
var header;

Expand Down Expand Up @@ -235,7 +313,7 @@ describe('react.backbone', function() {
});
it("should use that return value", function() {
var user = new Backbone.Model({name: "Clay"});
var userViewRef = UserView({user_model: user});
var userViewRef = React.createFactory(UserView)({user_model: user});
var userView = TestUtils.renderIntoDocument(userViewRef);

var header = TestUtils.findRenderedDOMComponentWithTag(userView, 'h1');
Expand Down Expand Up @@ -268,7 +346,7 @@ describe('react.backbone', function() {
expect(object).toEqual(users);
return true;
};
var userViewRef = UserView({ collection: users });
var userViewRef = React.createFactory(UserView)({collection: users});
TestUtils.renderIntoDocument(userViewRef);
});
it("should pass model to config function for test", function () {
Expand All @@ -278,7 +356,7 @@ describe('react.backbone', function() {
expect(object).toEqual(user);
return false;
};
var userViewRef = UserView({ model: user });
var userViewRef = React.createFactory(UserView)({model: user});
TestUtils.renderIntoDocument(userViewRef);
});

Expand All @@ -296,7 +374,7 @@ describe('react.backbone', function() {
React.BackboneMixin.ConsiderAsCollection = function (object) {
return false;
};
var userViewRef = UserView({ model: usersModel });
var userViewRef = React.createFactory(UserView)({model: usersModel});
TestUtils.renderIntoDocument(userViewRef);
});

Expand All @@ -306,13 +384,12 @@ describe('react.backbone', function() {
React.BackboneMixin.ConsiderAsCollection = function (object) {
return true;
};
var userViewRef = UserView({ collection: usersCollection });
var userViewRef = React.createFactory(UserView)({collection: usersCollection});
TestUtils.renderIntoDocument(userViewRef);
});

});

});
});

});
12 changes: 10 additions & 2 deletions react.backbone.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
updateScheduler: _.identity
};

var updatable = function (component){
if (component.shouldComponentUpdate !== null){
return component.shouldComponentUpdate();
}
return true;
}

var subscribe = function(component, modelOrCollection, customChangeOptions) {
if (!modelOrCollection) {
return;
Expand All @@ -33,7 +40,7 @@
var behavior = React.BackboneMixin.ConsiderAsCollection(modelOrCollection) ? collectionBehavior : modelBehavior;

var triggerUpdate = behavior.updateScheduler(function() {
if (component.isMounted()) {
if (component.isMounted() && updatable(component)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Couldn't you just add component.shouldComponentUpdate() here instead of calling helper function?

Copy link
Author

Choose a reason for hiding this comment

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

shouldComponentUpdate isn't always defined, for that reason i've added updatable function to handle condition there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok yes makes sense - strange that React doesn't have a default implementation for shouldComponentUpdate{return true;} and having the spec override the implementation (like other lifecycle methods). But looks like most implementations on React are like this and not Backbone esq where you override default methods with your own etc.

(component.onModelChange || component.forceUpdate).call(component);
}
});
Expand Down Expand Up @@ -79,7 +86,7 @@

unsubscribe(this, modelOrCollection(this.props));
subscribe(this, modelOrCollection(nextProps), customChangeOptions);

if (typeof this.componentWillChangeModel === 'function') {
this.componentWillChangeModel();
}
Expand All @@ -98,6 +105,7 @@
componentWillUnmount: function() {
// Ensure that we clean up any dangling references when the component is destroyed.
unsubscribe(this, modelOrCollection(this.props));

}
};
};
Expand Down