Improve perf

[xcarpentier]

#1048

[sibelius]

why purecomponent is not a good choice here?

[xcarpentier]

@sibelius because that cause issue on large list of messages (+200). Too much memory usage due to the deep shallow equal (blank screen, crash).

[xcarpentier]

@sibelius do you have any other suggestions ?
BTW we passed thought all component huge amount of props and shallowequal did the work and consume too much memories that our thread not display any UI. (ie. #1076)

[sibelius]

Good enough for me

[brunocascio]

Is expo snack working? Code looks good for me

[ChrisEdson]

This PR is super buggy, for a bunch of reasons.

It means that if you change any property in a message (like pending etc), the FlatList will not re-render because of the shouldComponentUpdate in MessageContainer.js. As the actual length of the messages array isn't changing, then nothing will update.

There's also bigger isssues in the PR - in that shouldComponentUpdate function, next.extraData and current.extraData is referenced. next is this.props.messages - so you're referencing the extraData property on a messages array. The same for loadEarlier.

I highly recommend using flow for things like this, it would have picked this up immediately - rather than merging something in that will break everyone's code.

[xcarpentier]

Hi @ChrisEdson,

Really sorry about that.

We surely can just change MessageContainer extends PureComponent.
I don't have any problem with that. I will change asap and please give me your feedback.

BTW what do you mean using flow?
I really think if we introduce static type checking we need TypeScript and not Flow. I'm curious to know if that caused any issue if we change that regarding contribution.

[xcarpentier]

@ChrisEdson please review: #1101

[ChrisEdson]

Ah sorry my comment sounded really mean!

I just mean adding Flow (or TypeScript). Then it would flag that you were trying to access extraData on an Array type (messages). TypeScript would do the same, but is a bit more work to add. Flow is pretty quick!

[xcarpentier]

@ChrisEdson TypeScript is just better. I really don't want to introduce Flow.

So migrate to TypeScript?!?
Challenge accepted!

Just need to know if enough contributor will continue or not?

So please vote here with an 👍 if you see it ;)

See here an example of a small package in TypeScript: https://github.com/xcarpentier/react-native-safe-image

[brunocascio]

I've no used typescript yet, but I think is better than JavaScript. I want to learn it. So, go ahead.

[xcarpentier]

#1190
cc @ChrisEdson @FaridSafi @brunocascio @sarovin @shaheem-khanzada @jamesholcomb