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

Add shallowCompare module and use it in PureRenderMixin + tests #3355

Merged
merged 1 commit into from Mar 13, 2015

Conversation

Projects
None yet
6 participants
@cpojer
Contributor

cpojer commented Mar 10, 2015

This adds shallowCompare and adds tests for PureRenderMixin.

This allows the following pattern:

class MyComponent extends React.Component {
  shouldComponentUpdate(nextProps, nextState) {
    if (!React.addons.shallowCompare(this, nextProps, nextState)) {
      return myCustomCheck(…);
    }
    return false;
 }
}

Not super happy about this API. Component.prototype.shouldComponentUpdate = React.addons.shallowCompare seems like the better choice for the common case, but then using it would require calling React.addons.shallowCompare.apply(this, arguments).

sebmarkbage added a commit that referenced this pull request Mar 13, 2015

Merge pull request #3355 from cpojer/shallowCompare
Add shallowCompare module and use it in PureRenderMixin + tests

@sebmarkbage sebmarkbage merged commit ca8d7cb into facebook:master Mar 13, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@cpojer cpojer deleted the cpojer:shallowCompare branch Mar 17, 2015

@cpojer cpojer referenced this pull request Mar 25, 2015

Merged

Add npm-react-codemod #3506

@3den

This comment has been minimized.

Show comment
Hide comment
@3den

3den Nov 13, 2015

@cpojer @sebmarkbage why dont we just create an class that inherits from Component?

e.g:

class PureCompoment extends React.Component {
     shouldComponentUpdate(nextProps, nextState) {
         return !React.addons.shallowCompare(this, nextProps, nextState)
      }
}

class MyComponent extends PureCompoment {  
    // it gets shouldComponentUpdate for free
}

3den commented Nov 13, 2015

@cpojer @sebmarkbage why dont we just create an class that inherits from Component?

e.g:

class PureCompoment extends React.Component {
     shouldComponentUpdate(nextProps, nextState) {
         return !React.addons.shallowCompare(this, nextProps, nextState)
      }
}

class MyComponent extends PureCompoment {  
    // it gets shouldComponentUpdate for free
}
@dapetcu21

This comment has been minimized.

Show comment
Hide comment
@dapetcu21

dapetcu21 Nov 13, 2015

@3den Because Javascript doesn't support multiple inheritance and using multiple "mixins" like this would be impossible.

dapetcu21 commented Nov 13, 2015

@3den Because Javascript doesn't support multiple inheritance and using multiple "mixins" like this would be impossible.

@3den

This comment has been minimized.

Show comment
Hide comment
@3den

3den Nov 14, 2015

@dapetcu21 I think we don't need multiple inheritance nor mixins. I am suggesting that react should offer 2 kinds of components: React.Component and React.PureCompoment. So on all components that you want it to be "pure" you can extend PureCompoment instead of Component.

3den commented Nov 14, 2015

@dapetcu21 I think we don't need multiple inheritance nor mixins. I am suggesting that react should offer 2 kinds of components: React.Component and React.PureCompoment. So on all components that you want it to be "pure" you can extend PureCompoment instead of Component.

@jimfb

This comment has been minimized.

Show comment
Hide comment
@jimfb

jimfb Nov 14, 2015

Contributor

@3den What happens when you want to be both a PureComponent and a GraphqlComponent? You can't extend both.

Contributor

jimfb commented Nov 14, 2015

@3den What happens when you want to be both a PureComponent and a GraphqlComponent? You can't extend both.

@3den

This comment has been minimized.

Show comment
Hide comment
@3den

3den Nov 14, 2015

@jimfb true, I see your point. But I think it would be nice to have a something like a decorator just so that we don't have to copy and paste that function over and over...

3den commented Nov 14, 2015

@jimfb true, I see your point. But I think it would be nice to have a something like a decorator just so that we don't have to copy and paste that function over and over...

@jimfb

This comment has been minimized.

Show comment
Hide comment
@jimfb

jimfb Nov 14, 2015

Contributor

@3den Yes, that's called a "mixin".

Contributor

jimfb commented Nov 14, 2015

@3den Yes, that's called a "mixin".

@pierluigi pierluigi referenced this pull request Nov 24, 2015

Closed

Migrate to React 0.14 #2710

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 4, 2016

Member

Also, sometimes copy-pasting a trivial three-line function can be a better solution than introducing abstraction and indirection. It makes it obvious where the method comes from, and easy to change its implementation if necessary later.

Member

gaearon commented Jan 4, 2016

Also, sometimes copy-pasting a trivial three-line function can be a better solution than introducing abstraction and indirection. It makes it obvious where the method comes from, and easy to change its implementation if necessary later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment