ReadableArray getMap pops the item out of the array.

[SudoPlz]

Thus if we run a for loop to iterate through all the maps within a ReadableArray, our array is left empty.

Now I'm not aware of a way to clone a ReadableArray, so I think there should be away to access the children of a ReadableArray without deleting them afterwards.

Is this a bug or is it intended behaviour?

[grabbou]

CC: @cjhopman I can see you have a TODO in a native implementation of getMap. Maybe you have more details regarding this issue.

[cjhopman]

I mean, it's intended behavior, but just because nobody wanted to take the time to implement behavior that makes more sense. My preference would be that getMap() (and similar) not pop out the item.

The issue is that these java-c++ hybrid objects all are set up so that the java object owns the underlying c++ object (and so the c++ object is deleted when there are no more references to the java object). To support getMap() and similar having the better behavior it would probably be easiest to in some way reference count (or strong java reference type ownership) the underlying outer folly::dynamic and keep pointers into it for the thing that you want. That is, a ReadableNativeArray will have one strong reference into the outermost folly::dynamic and then some reference into the folly::dynamic that corresponds to that array.

Because of the way these are passed around, it is probably easiest to make the c++ part of these look something like:

struct DynamicOwner {
  std::unique_ptr<folly::dynamic> obj;
}

struct JReadableNativeArray : HybridClass<JReadableNativeArray, ReadableArray> {
  std::shared_ptr<DynamicOwner> outer;
  folly::dynamic* obj;
}

where obj is a pointer into outer (and in the constructed from c++ case, outer == obj).

[rigdern]

I hit this bug as well. It's very surprising that getMap pops. I also noticed that the npm module react-native-contacts made the same bad assumption about the behavior of getMap.

@cjhopman did getMap use to behave differently? If yes, what's the first release of RN to include the popping behavior? Are there any other ReadableArray/ReadableMap methods that also pop even though their names would suggest otherwise?

[cjhopman]

@rigdern I'm not sure how long this has been the behavior of getMap (I think it's always been the behavior, but I haven't looked back to confirm that). I think only getMap() does this, all the others (get map/array from map/array) all create copies (which is also rather bad).

[cjhopman]

But yeah, I still think that popping the map is crazy. We should just have getMap() create a copy like all the others. It's possible that for configuration of the bridge (i.e. creating the batched bridge config) making copies will be a significant perf hit, but we can just add a popMap() if that's the case.

[rigdern]

@cjhopman I just did a quick test with a couple of versions of my app. It looks like the behavior of getMap did change:

• getMap in ~0.25 does not pop
• getMap in 0.32.0-rc.0 pops

I'm not sure exactly when the behavioral change occurred. If it was a recent change, I think the behavior should be switched back before doing a release to avoid breaking apps.

[cjhopman]

This is fixed in 9a5d3ba

[grabbou]

Closing as @cjhopman points to a commit that fixes the issue described.