jemise111/react-native-swipe-list-view

Cannot close row: keyExtractor doesn't appear to be honored in rowMap when 'key' attribute present in row item

rpattcorner opened this issue · 10 comments

I've got a corner case perhaps ... my application is a music application, so its data.item object has an attribute called, lterally, 'key'.

  • As an aside, this doc seems germane, but results in unswipable rows when inserted in my code
  • FWIW, the screen is a functional component, not class based per most examples

Per #449 and general practice, I use a keyExtractor in the <SwipeListView>, just like I do in the <FlatList> that I'm trying to replace. FlatList works fine.

In the FlatList my keyExtractor looks like keyExtractor={item => item.tune_id}, but I've changed it to keyExtractor={(item, index) => item.tune_id} per #449 just to follow available advice. Makes no difference to the problem, which is an inability to close a row.

So, for completeness, we have:

<SwipeListView
                    ListEmptyComponent={ListEmptyView}
                    ItemSeparatorComponent={RenderSeparator}
                    data={theTuneList}
                    keyExtractor={(item, index) => item.tune_id}
                    renderItem={renderItem}
                    renderHiddenItem={renderHiddenItem}
                    extraData={state.initialized}
                    leftOpenValue={75}
                    rightOpenValue={-150}
                    previewRowKey={'0'}
                    previewOpenValue={-40}
                    previewOpenDelay={3000}
                    onRowDidOpen={onRowDidOpen}
                    disableRightSwipe={true}
                />

and you'd think all would be well. As you can guess from the title of this issue, my item looks like:

{
   tune_id: SomeUniqueId
   key: 'G'
   meter: 'reel'
   ...
}

and in my vanilla closeRow function:

    const closeRow = (rowMap, rowKey) => {
        if (rowMap[rowKey]) {
            rowMap[rowKey].closeRow();
        }
    };

The rowMap appears to be ignoring the keyExtractor. Have a look at this debug view from Chrome, in whose underlying data there are 3 tune items, two in the key of G and one in the key of D, with the 'D' tune slid to the left, and a click on the close button that invokes closeRow. Note that the second 'G' tune does not show up, and more importantly, the rowMap seems to be keying off the literal 'key' attribute of the item instead of the keyExtractor mapping:

image

As you can see there are two rows in the rowMap, not the expected 3. Possibly because two of the tunes have the same (literal) key attribute. This could be a hint, or an effect, not sure.

image

And there's no way that rowMap[rowKey], which should literally be rowMap['@tune_id.482'] is ever going to find a row to close.

image

@jemise111 am I (hopefully) doing something simple and obviously wrong here?

Hey @rpattcorner, I never thought that if objects had a key they wouldn't be unique! I guess if you had a music app or a locksmith app there's definitely an edge case I never thought of 😂 !

Currently a key takes precedence over a keyExtractor so that certainly could be the issue. Let's do this.. can you manually make a change to the code and if it works I'll push a release with the fix.

Can you please go to node_modules/react-native-swipe-list-view/components/SwipeListView.js => line 378 and change the following code

let { key } = item;
if (!key && this.props.keyExtractor) {
    key = this.props.keyExtractor(item, index);
}

to

let key = this.props.keyExtractor(item, index);

And let me know if that fixes your issues, thanks!

@jemise111 , thanks for the quick response! Yes, the proposed one line substitution fixes the problem for my app! Any reason to hold out for a push before sending my next build to testers?

(hold on a minute, seeing a problem, likely unrelated)

@rpattcorner I just released version v3.2.7 which will now favor keyExtractor over key, you should be able to use that version without issue

I will leave this issue open per your latest comment, but please close this if it is unrelated, thanks!

Thanks! It looks to me like the issue I experienced (a freeze on navigation) is unrelated, and seems to correlate with running both an older Android emulator and a real Android device at the same time, but I'll download the new version and bang on it awhile, then close if all still seems well.

Again, much appreciated. Those edge cases are like 'who's on first'.

I did see this, which is new and possibly relates, just FWIW:

[11:27:07] VirtualizedList: You have a large list that is slow to update - make sure your renderItem function renders components that follow React performance best practices like PureComponent, shouldComponentUpdate, etc. Object {
[11:27:07]   "contentLength": 2770.28564453125,
[11:27:07]   "dt": 759,
[11:27:07]   "prevDt": 1032,
[11:27:07] }

@rpattcorner IMO that shouldn't be something related to this lib but I'll keep an eye out for other reports

Looks good, many thanks! Closing.

I found that rowMap is empty object if I use not directly in renderItem function, but enclose it with another intermediate component. Why?

@code-by hitting the same issue. Have you managed to figure it out somehow?