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.

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

this.state does't work at listView's renderRow #1133

Closed
iahu opened this issue May 4, 2015 · 27 comments
Closed

this.state does't work at listView's renderRow #1133

iahu opened this issue May 4, 2015 · 27 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@iahu
Copy link

iahu commented May 4, 2015

I update the this.state.pressTest on my onItemPress method (gist), and hope can get the newly state of pressTest (gist).But, it does't work on my code.

why? thx.

@amitverma
Copy link

you're calling setState twice. doesn't this cause it to render twice as well? maybe a race is happening?

that.setState({updatePressed: true});
that.setState({pressTest: 2});

@iahu
Copy link
Author

iahu commented May 4, 2015

that.setState({pressTest: 2}); added to test. Maybe I should comment the first line.

@tadeuzagallo
Copy link
Contributor

There's no problem in calling setState() more than once, although if it wasn't a test it'd be better to put then together.

The issue there is that setState() is async, and you're trying to access it on the next line. It doesn't mean it's not working, it means it wasn't evaluated yet. It'll just be evaluated at the end of the frame, when all the calls have been finished.

https://facebook.github.io/react/docs/component-api.html#setstate

@iahu
Copy link
Author

iahu commented May 4, 2015

I know the setState can't immediately update.
my problem is that, I can't get the newly state at renderRow (gist)

@iahu
Copy link
Author

iahu commented May 4, 2015

this.state.pressTest (at line 98) always be 0 but not 2. At the same time, I can get a 2 at line 82.

@tadeuzagallo
Copy link
Contributor

Oh, I see, renderRow is never called again...

I had a look on the source, but I'm not sure what's the best solution here. /cc @vjeux

@ide
Copy link
Contributor

ide commented May 4, 2015

Try always returning true from rowHasChanged. Better solution is not to access this.state/this.props/this.context/any non-constant instance variable or global variable from renderRow, and instead pass data through only the data source. The docs should probably mention this as a rule of thumb since there have been a couple similar issues opened.

@tadeuzagallo
Copy link
Contributor

I tried the rowHasChanged trick, but it doesn't work. It's not called again either, since the row is already rendered and the data source hasn't change, it doesn't try to re-render.

@PhilippKrone
Copy link
Contributor

I've tried to do the same thing as well some days ago and had no real luck. Some things a tried (which are kind of dirty though):

  • put everything in the datasource objects (as ide suggested)
  • re-create the datasource by changing a dummy value (so that the rows are re-rendered)
  • create a new datasource with the same content and pass it to the ListView
  • this.props.navigator.replace()

I'm currently living with the last option, although it has probably some disadvantages. Had no time to investigate it any further.

@ide
Copy link
Contributor

ide commented May 4, 2015

Good point, you need to force a data source update too. Option 3 of cloning a data source with the same content should provide the desired behavior.

For the API it would be easier if hasRowChanged were replaced with shouldComponentUpdate plus component pooling to avoid allocations. I haven't thought through the details but I think we want a React-style API instead of this UIKit-inspired one.

@PhilippKrone
Copy link
Contributor

Although I remember that option 3 (creating a new DS) did not work as well in some scenarios as the rows were not re-rendered (debugged a little bit into it and noticed that even with a new DS, the re-render method was not executed). Thats why I finally began to use option 4. I remember discussing with someone about that topic over there @ freenode about 2 or 3 days ago, but can't remember anymore. My context was adding a new item in a gridview (https://github.com/lucholaf/react-native-grid-view) on the same row, which did not make RN to re-render the row.

@dvdhsu
Copy link

dvdhsu commented May 5, 2015

I've also been having problems with this. For me, I was trying to render a list of notifications, each with a time associated. So, like like how tweets are labeled "2m ago".

I initially tried calculating the relative times inside renderRow, but that didn't work. So, I ended up cloning notifications (via JSON.parse(JSON.stringify(notification)). That worked, but would spuriously cause some of my rows to become blank whenever the row changed enough (going from "4s ago" to "5s ago" was fine, but going from "9s ago" to "10s ago" wasn't).

So, I then tried removing the time from my renderRow function, so it'd just render the notification. The time was still being updated in the background, it just wasn't being displayed. This, indeed, would make my rows more stable, and they no longer disappear randomly.

After reading @ide's suggestion above, then set put all my times into state, but still caused my rows to spuriously become blank. Once again, removing the times fixed the issue. I'm still not sure why this is. Have you encountered these problems, @PhilippKrone?

Edit: nevermind the blank text problem. @ide graciously debugged with me over IRC, and it was mentioned / fixed in #813. Thanks again, @ide!

@iahu
Copy link
Author

iahu commented May 5, 2015

clone the origin DS to force the renderRow render again works. Thanks a lot.

var _ds = JSON.parse(JSON.stringify(ds));
that.setState({
    dataSource: that.state.dataSource.cloneWithRows(_ds),
    updatePressed: true
});

@yogurt-island-331
Copy link

@iahu could you share more details on this code? What is the context?

@iahu
Copy link
Author

iahu commented Oct 30, 2015

@kevinzzz007 source code is here https://gist.github.com/iahu/0e524f4612a8925f2f9c

@yogurt-island-331
Copy link

@iahu How did you manage to force the ListView to re-render? I didn't see anywhere you created _ds...

@iahu
Copy link
Author

iahu commented Oct 30, 2015

OK, I have update my gist.

@yogurt-island-331
Copy link

@iahu thanks! That made it really clear. So in my case, I got my data from ParseReact, here is my code:

var _ds = this.data.driver_available_times;
this.setState({
        dataSource: this.state.dataSource.cloneWithRows(_ds),
    });

@iahu
Copy link
Author

iahu commented Oct 30, 2015

You need to make a clone of you original datasource. That means it's must be a "immutable object".
So as your case, you can do that like me.

var _ds = JSON.parse(JSON.stringify(this.data.driver_available_times));

@yogurt-island-331
Copy link

@iahu yeah, I tried that, an issue with that is my Parse object has date object in it, which if I run the object through
(JSON.stringify(this.data.driver_available_times));, some data will be lost(time zone, and also with these information lost, I can't run some function anymore), so I had to use
var _ds = this.data.driver_available_times;

@iahu
Copy link
Author

iahu commented Oct 30, 2015

@kevinzzz007 try this code

var d = this.data.driver_available_times;
var _ds = new Date(d); // d !== _ds, make a  clone of your Date object.

@yogurt-island-331
Copy link

@iahu thanks! I will give it a shot :-)

@fiowind
Copy link

fiowind commented Nov 5, 2015

re-create the datasource by changing a dummy value (so that the rows are re-rendered).
this fixed this propblem well.
look detail at http://stackoverflow.com/questions/29933546/updating-this-state-datasource-doesnt-update-listview

@fiowind
Copy link

fiowind commented Nov 5, 2015

this.ds = new ListView.DataSource({rowHasChanged: (row1, row2) => row1 !== row2});
this.setState({
    dataSource: this.ds.cloneWithRows(this._dataSource),
    loaded: true,
});

@yogurt-island-331
Copy link

@fiowind that makes sense, I might try this solution in the future, but I do think there should be an option for us to reload the ListView

@sahrens
Copy link
Contributor

sahrens commented Jan 4, 2016

ListView is designed to have all data for renderRow come from the data source. I recommend putting the selection state in the row data of the data source and updating that with clone and setState. Does that work?

Sounds like some documentation improvements would help...

@rammi44
Copy link

rammi44 commented Jan 2, 2018

this.state.todoItemArray = this.state.todoItemArray.concat({ 'Item': item, 'TodoItemstyle': todoItembgStyle });
this.setState({
todoItemArray: this.state.todoItemArray,
isDeleteDefaultItem: true
});

Use Concat.. it will work.. adding new list item to the datasource... Here TodoItemArray is my datasource..

@facebook facebook locked as resolved and limited conversation to collaborators May 30, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests