Skip to content
This repository has been archived by the owner on Dec 31, 2020. It is now read-only.

Idea: Introduce Observer as higher order component #138

Closed
mweststrate opened this issue Oct 22, 2016 · 9 comments
Closed

Idea: Introduce Observer as higher order component #138

mweststrate opened this issue Oct 22, 2016 · 9 comments

Comments

@mweststrate
Copy link
Member

mweststrate commented Oct 22, 2016

Alternative syntax for using observer as decorator / higher order component:

class Component extends React.Component {
  render() {
     return (
         <Observer>
             {() => <div>{this.props.person.name}</div>}
         </Observer>
     )
  }
}

// Or
const Component = ({person}) => <Observer>{() => <div>{person.name}</div>}</Observer>

Pros:

  1. No need for wrapping function / decorator. Benefits of that:
    1. No / less issues with React component names
    2. No conflicts with other decorators
  2. Doesn't influence / change lifecycle methods of Component
  3. Marks a region as reactive
  4. Fits nicely in the composability philosophy of React

Cons:

  1. Confusion with this / this.props / might be dereferencing accidentally in host component instead of the observer region
  2. Additional wrapping component
  3. More then one-way-to-do-things
  4. More confusing syntax? (pass children as thunk)
  5. More verbose

N.b. implementation would simply be (not tested, might be missing something):

export const Observer = observer(({ children }) => children())
@urugator
Copy link
Contributor

A little enhancement maybe?

export const Observer = observer(props => React.createElement(props.children, props, null));

Should work the same way, but one can provide any react component (aside from function) and pass props:

<Observer propForMyCmp="hello">{MyCmp}</Observer>

@mweststrate
Copy link
Member Author

@urugator personally I think that is a bit confusing to read?

@urugator
Copy link
Contributor

urugator commented Nov 8, 2016

@mweststrate Its the same as in your example, you can still write the render callback inline, it just exploits the fact that render callback is actually valid react (stateless) component (also the restrictions are the same - for example the render callback must return single react element). So you can replace render callback by any smarter (class based, unobservable) react component anytime.
I just think there is no need to introduce a concept of "render callback" when we can simply say that Observer can wrap any react component (including stateless functional components). Or are there any disadvantages/problems I am not aware of?

@mweststrate mweststrate mentioned this issue Nov 9, 2016
13 tasks
@mweststrate
Copy link
Member Author

@urugator you might be right, gonna give it a try :)

mweststrate added a commit that referenced this issue Nov 9, 2016
@KaySackey
Copy link

I like it, so long as it doesn't become the default. More options for integration are better for a library, and lots of people (though not I), prefer using HoC style in React. Personally, though, I simply have a top level component that subclasses React.Component, and applies @observer onto it, then all my application components extend from that.

@mweststrate
Copy link
Member Author

@urugator played with your suggestion, but it was more confusing to read, and also kill static typing on props etc. So I'll keep it as is. Thanks for the suggestion though :)

@mweststrate
Copy link
Member Author

Implemented in mobx-react@4.0.0-rc.1

@mweststrate
Copy link
Member Author

Shipped with 4.0.0 :)

@mhaagens
Copy link

mhaagens commented Mar 2, 2017

Just wanted to say this pattern works great for me, most important is the fact that it doesn't alter the lifecycle methods which I use for server rendering.

@github-actions github-actions bot mentioned this issue Oct 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants