-
Notifications
You must be signed in to change notification settings - Fork 70
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
composer function re-run after every prop change #2
Comments
@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:
So, I get it. Just like how we implement What do you think? |
@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})
}
}
|
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. |
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 👍 |
It's upto the user. Its not 100% possible, but can do it. |
Why isnt it possible? |
In apps, we may have to load data from other data sources not just from GraphQL. Then we need to deal with them. |
Does JSON-LD have any applications to this problem? |
I seem to be having the same issue where shallow compare isn't helping at all. 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? |
Can you post the repo here so we can run it for ourselves? |
Sadly I cannot, I will try and make a repo to replicate the issue however — it will be sometime this weekend. |
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. |
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 ? |
Is this the only solution ? https://atmospherejs.com/meteor/react-meteor-data |
@sahanDissanayake your issue is not related to this. It seems like a reactive issue. |
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 |
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. |
We have a PR (#99) waiting to be pulled in that completely fixes this issue. |
Currently, composer function will re-run for every prop change. This is a continuation of the discussion started at: facebook/react#3398 (comment)
The text was updated successfully, but these errors were encountered: