composer function re-run after every prop change

[arunoda]

Currently, composer function will re-run for every prop change. This is a continuation of the discussion started at: facebook/react#3398 (comment)

[arunoda]

@oztune Okay, you suggested to return a function and a key from the composer function and function will re-run if the key is different from the last time.

I assume here's the reason for this:

• Your data fetching code (the logic inside the component) is live inside another file.
• It may change the need of props time to time.
• So, if we go with the option I suggested (mention props, when composing the component),
• If I need to use a new prop, I need to change in two places. That may leads to errors.
• I can't change the re-run logic runtime.

---

So, I get it.
I like to add this rather making it something mandatory. If someone is getting some issues with this, he can go with that.

Just like how we implement shouldComponentUpdate.

What do you think?

[ccorcos]

@arunoda, don't you think that this is still imperative programming? You've isolated the side-effect to a container component, but your entire UI isnt side-effect free anymore and is imperatively tied to the lifecycle of the components.

Take this analogy. We used to do this:

turnOn = (e) => {
  e.target.addClass('on')
}
turnOff = (e) => {
  e.target.removeClass('on')
}

And now we do this:

render = (state, props) => {
  return <div className={state.on ? 'on' : 'off'}></div>
}

The the second example, we just declare what we want and let React parse the component tree and handle any mutations.

The way you're proposing to fetch data in containers (and which I think just about everyone is doing) looks a lot like this:

componentWillMount = () => {
  fetchData()
}
componentWillUnmount = () => {
  cleanup()
}

This is the same imperative style as before. What I think we should be doing is declaratively specifying what is needed and letting some other service parse and do all the imperative heavy lifting.

Relay is a step in the right direction. But I think its pretty convoluted how the Relay container talks to the React component and how the Relay container data gets accumulated into one declarative top-level request.

Just like how we bind callback functions to vdom in the render function, what if we did the same for declarative data fetching?

fetch = (state, props) => {
  return {
    query: someGraphQLQuery,
    onData: (data) => this.setState({data}),
    onError: (error) => this.setState({error})
  }
}

fetch is run everytime render runs and is diffed just like the vdom is diffed. If some request disappears, then its cleaned up automatically. If something is already fetched, then it doesnt re-fetch it.

[arunoda]

This project is not something I propose to introduce declarative data fetching. But just create a shorthand method for what everyone do it right now.

declaritive data fetching needs to done in the caching layer. But it should not need to tie up with any UI layer. This is how we do it in our meteor-graphql example: https://github.com/kadira-samples/meteor-graphql-demo/blob/master/client/containers/blog_post.jsx

Even though component, re-render it won't fetch data again. This can be true for Meteor as well, with subs-manager like projects.

I believe in multiple data stores which manages the state for our apps. That's why I started this project.

[ccorcos]

Yeah, thats the same pattern I used when I built findashindig.com: https://medium.com/@chetcorcos/shindig-react-data-component-aa0d2c82059e#.r2jveywu7

If you're using GraphQL, then it seems like you should compose your queries all the way up to the top-level and have a single container for all your data. Then everything inside the container is just the UI and its pure 👍

[arunoda]

It's upto the user. Its not 100% possible, but can do it.
Specially for apps which only render data.

[ccorcos]

Why isnt it possible?

[arunoda]

In apps, we may have to load data from other data sources not just from GraphQL. Then we need to deal with them.

[venturecommunism]

Does JSON-LD have any applications to this problem?

[patrickml]

I seem to be having the same issue where shallow compare isn't helping at all.

http://cl.ly/1V0Y1r0m3f0c

in this video, you can see that the props do not change as I am returning the same data every time. With that being said you can also see that using the react console that the components are re-rendering. Any ideas why?

[clayne11]

Can you post the repo here so we can run it for ourselves?

[patrickml]

Sadly I cannot,

I will try and make a repo to replicate the issue however — it will be sometime this weekend.

[sahanDissanayake]

Anything new with this ?

I have facebook like layout. 100 posts 1 like button on each post. when one like button is clicked. the composer function for the like button is running 100 times before the like actually takes place.. it sucks.

[sahanDissanayake]

Not sure if I'm having this same issue but the one i have is an issue. Once the data gets big enough there is a huge lag because of all the computations

Anyone ever come across this ?

[sahanDissanayake]

Is this the only solution ? https://atmospherejs.com/meteor/react-meteor-data

[arunoda]

@sahanDissanayake your issue is not related to this. It seems like a reactive issue.
Just create a new issue with a sample repo.

[sahanDissanayake]

Sorry to mention my issue everywhere.. I will create a new repo in another 4 hours or so and will create a new issue and ping you. Thanks

I did some bad coding and hacking and limited the re-renders using shouldComponentUpdate.. But im sure there is a better solution

[markshust]

Can someone post a sample repo to reproduce the problem? I'm experiencing random issues with react-komposer, and everything seems to be leading back to this ticket.

[clayne11]

We have a PR (#99) waiting to be pulled in that completely fixes this issue.