Android Issue: Exception in native call from JS

[geo-vi]

This occurs on Android when changing FlatList data too rapidly

In addition to that there is a ref issue in the code - you assign an unnecessary ref and also block on the way the forwarded ref

<FlatList
        {...props}
        ref={(ref) => {
          // @ts-ignore
          flRef.current = ref;
          typeof forwardedRef === 'function' && forwardedRef(ref);
        }}
      />

I'll submit a PR for the refs however, I still would like an insight on why it drops that error

[vishalnarkhede]

@geo-vi I have published a patch. That should handle the case of forwardRef.

I haven't yet managed to look into the native error that you mentioned here. Are there any steps to reproduce this?

[flexsurfer]

hi, I've faced the same issue, with changing FlatList data too rapidly

[flexsurfer]

i don't have an example, but i guess you can try to reproduce with a timer, just add new elements in the interval

[flexsurfer]

i'm trying native module without js part, and i noticed that each time i change data for my list, ref handler is called, and if i call enable in ref i have this error, so i enable only once, and don't have this error, its something to do with ref and enable/disable functions calls

[vishalnarkhede]

@flexsurfer can you confirm what version are you using? 0.0.4?

[flexsurfer]

yes 0.0.4

[k4rriL]

@vishalnarkhede At least for me, the "Exception in a native call from JS" -issue results from calling the enable or disable methods many times quickly. This is because the if-statement in the resetMvcpIfNeeded function are not comparing the right things. They are comparing the current value of the refs to the values in the props, but if some props are not defined (either autoscrollToTopThreshold or minIndexForVisible), then the refs are assigned a default value, which will always be different to the value in the props. This will result to the if-statement resolving to false each time resetMvcpIfNeeded is ran, causing the disable and enable functions to run too often. I fixed this by changing the ||-assignments from the ref.current assignments to the enableMaintainVisibleContentPosition function call.

autoscrollToTopThreshold.current = mvcp.autoscrollToTopThreshold;
minIndexForVisible.current = mvcp.minIndexForVisible;
const viewTag = flRef.current.getScrollableNode();

cleanupPromiseRef.current = MvcpScrollViewManager.enableMaintainVisibleContentPosition(
  viewTag,
  autoscrollToTopThreshold.current  || -Number.MAX_SAFE_INTEGER,
  minIndexForVisible.current || 0
);

[vishalnarkhede]

Thanks @k4rriL I will look into it today :)

[rgbedin]

@vishalnarkhede Thanks for looking into this. Any updates on this issue?

[vishalnarkhede]

@rgbedin I am looking into in as I type this :D

[vishalnarkhede]

@k4rriL Thanks for pointing this out, I see the issue now. Poor testing from my side, apologies :)

[vishalnarkhede]

Published v0.0.8

[rgbedin]

It seems to be working now. I am not getting this crash anymore. However is it expected that my FlatList is not able to extract the ref out? If I use the implementation from react-native my ref works just fine. If I use from this package, it seems that ref.current is not defined. I am posting here as a comment because I am not sure if that is a side-effect of these changes or not:

// listRef.current is always undefined
<FlatList
     ref={listRef}
... />

[karri-lehtiranta]

@rgbedin I noticed this as well and traced it to the else if part of the ref-function in FlatList.tsx:
else if (forwardedRef?.current) { forwardedRef.current = ref; }
As forwardedRef?.current is always null at start, the if-clause never validates to true so the forwarded ref never gets set. By changing forwardedRef?.current to forwardedRef?.current !== undefined worked for me.

[karri-lehtiranta]

@vishalnarkhede Thank you, it does not crash anymore 👍 But now I have another issue where the Flatlist does not initially maintain its position. When I live reload or use it for a while and refresh the application it starts working. This is my current config:
maintainVisibleContentPosition={{ minIndexForVisible: 0 }}

It would also seem that in the row 45 of flatlist, this expression: minIndexForVisible.current || 1 evaluates my config to 1 instead of the zero I would like to use. Maybe using minIndexForVisible.current !== undefined would solve the issue

[vishalnarkhede]

It seems to be working now. I am not getting this crash anymore. However is it expected that my FlatList is not able to extract the ref out? If I use the implementation from react-native my ref works just fine. If I use from this package, it seems that ref.current is not defined. I am posting here as a comment because I am not sure if that is a side-effect of these changes or not:

// listRef.current is always undefined
<FlatList
     ref={listRef}
... />

@rgbedin Can you try ref as callback?

[vishalnarkhede]

@karri-lehtiranta I will take a look soon. Thanks for brining it up :)

[rgbedin]

It seems to be working now. I am not getting this crash anymore. However is it expected that my FlatList is not able to extract the ref out? If I use the implementation from react-native my ref works just fine. If I use from this package, it seems that ref.current is not defined. I am posting here as a comment because I am not sure if that is a side-effect of these changes or not:

// listRef.current is always undefined
<FlatList
     ref={listRef}
... />

@rgbedin Can you try ref as callback?

That does work. Code for reference:

   <FlatList
      ref={(r) => (listRef.current = r)}
      ... 
   />

However I actually get the same bug @karri-lehtiranta reported. My list does not maintain its position properly and I have the same configurations reported.

[karri-lehtiranta]

It seemed like the resetMvcpIfNeeded function needed to fully run multiple times in order for the maintaining of the position to work which was previously achieved because of the issue in the if-clauses that caused the crashes. And now while that is fixed the flatlist fails to maintain the position.

[vishalnarkhede]

hmm this is interesting. I think the error was coming from MvcpScrollViewManager.disableMaintainVisibleContentPosition(handle);, because we didn't catch any errors in it (on native side). I added a try-catch as part of 0.0.8.

So maybe we need to switch back to executing enableMaintainVisibleContentPosition, everytime ref callback gets executed. And error won't appear since we have try-catch now. Can't really think of any other way around this.

---- Update

Tomorrow I will try to dig a bit more on native side, if we can find some solution there

[vishalnarkhede]

@karri-lehtiranta @rgbedin any thoughts?

[karri-lehtiranta]

I removed the if-clause that was checking whether to run enableMaintainVisibleContentPosition and disableMaintainVisibleContentPosition and the crashes came back for me so I think that the try-catch clause did not fix the problem.

When I initially debugged the problem, the native exception that was thrown in native side was java.util.ConcurrentModificationException which I think occurs when trying to edit a list while its been iterated which was happening with the native UIManager which I deduced the cause to be that the UIManagerListeners were added and removed so quickly that the ConcurrentModificationException was thrown. Hopefully this helps you somehow @vishalnarkhede

[vishalnarkhede]

@karri-lehtiranta thanks alot. I think that definitely helps.

Bdw, do you have some onScroll handler in your app?

[karri-lehtiranta]

Yes, I have a onScrollEndDrag handler.

[vishalnarkhede]

@karri-lehtiranta @rgbedin So I was trying to mess around some logic, and this looks like a decent middle ground: https://github.com/GetStream/flat-list-mvcp/pull/11/files

I have published 0.0.9-dev.0 and seems to be working well. Could you give it a shot ... and check if it works for your cases as well? :)

Basically if you have some onScroll handler which makes a call to setState - then FlatList invokes ref callback, which results in call to resetMvcpIfNeeded. So I guess little debounce will do a trick here.

Thanks in advance

[karri-lehtiranta]

Yeah it works much better now, thank you @vishalnarkhede, seems debounce did the trick.

I just have a little problem still. The list does not maintain the position the first time it changes, after that it works well. So when I scroll upwards and load more content, the list jumps to the start of that new content once and if I scroll upwards again, it will correctly stay at the right position when I prepend more data to the list.

[vishalnarkhede]

Thanks for checking @karri-lehtiranta , I am looking into it. I am able to see the issue as well. Scroll jumps halfway for me on first pagination call.

[vishalnarkhede]

But happy to know that atleast partly its fixed

[vishalnarkhede]

@karri-lehtiranta so couple of things:

• Upgrade to 0.0.9-dev.1 and test again. Have added few more improvements.
• When you load list for first time, how many items do you populate? While testing in example app, what I noticed was - if I start with 10 items then scroll jumps (first time). But if I start with 30 items, scroll doesn't jump. So please try to start with a bit more items from beginning in the list.

Please let me know how it goes!! The implementation is not exactly pretty to look at, but I want to first resolve all these issues, and then can clean up the code haha :)

[vishalnarkhede]

cc @rgbedin @geo-vi

[karri-lehtiranta]

I usually have 5-10 items in the list in the beginning, but with 0.0.9-dev.1 it seems to work most of the time (with the first one working every time). Around 80% of the times it works for me but once it even crashed with the same error if I scrolled very quickly in both directions. Haven't been able to make it crash again though. For my usecase, this is probably good enough, but might not be for everyone.

EDIT: 80% might be around 90-95% instead, it is working quite well now.

[vishalnarkhede]

@karri-lehtiranta

but once it even crashed with the same error

Thats wierd, I am trying really hard to reproduce crash after new changes ... but not able to :D

[vishalnarkhede]

Have published 0.0.9 with latest changes. Thanks once again :)

[lc3t35]

I have the same error with "@stream-io/flat-list-mvcp": "^0.0.9", "react-native-bidirectional-infinite-scroll": "^0.3.2",

[vishalnarkhede]

@karri-lehtiranta Could you please upgrade to 0.10.0-beta.0, you shouldn't see this issue at all now!!

[vishalnarkhede]

Also @geo-vi @flexsurfer ^^

[vishalnarkhede]

Published 0.10.0. Please reopen if you still manage to reproduce this.!!