Skip to content

Commit

Permalink
fix(35280): added a fix to trigger on viewable items changed when nul…
Browse files Browse the repository at this point in the history
…l or 0 is passed in first element (#36009)

Summary:
Since currently the check was for null , and that too not === check. So added a check , only for item !== undefined, since null is an assigned value, and we can have null as values in the array for flatlist,
 undefined is in absence of any data, hence if its only undefined we should assign falsy to frame variable, since null is an assigned value, sometimes null can be passed to data in the dataset

 Hence added a check on top of Sam's previous commit to fix it

 UPDATE:

 Now after discussing with NickGerleman , removed the check for item with nullish/undefined.
 Now directly frames value is being controlled by _keyExtractor function

 This is already an issue [#35280

Currently in my project, even [0,1] -> didnt trigger onViewableItemsChanged since, it was considered as falsy value,
went to check the node modules code for flatlist, debugged this.
When pulled latest main branch, saw it was partially fixed , but for null as values it wasnt fixed. So added that check .

## Changelog

[General] [Fixed] Fix VirtualizedList onViewableItemsChanged won't trigger if first item in data  is null

```
const frame =
      item !== undefined ? this._frames[this._keyExtractor(item, index, props)]
        : undefined;
```

      in place of existing which is

```
const frame =
      item != null ? this._frames[this._keyExtractor(item, index, props)]
        : undefined;
```

Update:

`const frame = this._frames[this._keyExtractor(item, index, props)]`

Finally this is the one used for getting frames value

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

Pull Request resolved: #36009

Test Plan:
Checked out in my local , wrt changes , will share video if required

Update:
After the new changes too, checked in local, working fine

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

Reviewed By: NickGerleman

Differential Revision: D42934757

Pulled By: skinsshark

fbshipit-source-id: cb5622a79523bccbdfbc15470baf84422f635b33
  • Loading branch information
gauravroy1995 authored and facebook-github-bot committed Feb 6, 2023
1 parent 7f2dd1d commit 011ea33
Showing 1 changed file with 1 addition and 4 deletions.
5 changes: 1 addition & 4 deletions Libraries/Lists/VirtualizedList.js
Original file line number Diff line number Diff line change
Expand Up @@ -1822,10 +1822,7 @@ export default class VirtualizedList extends StateSafePureComponent<
'Tried to get frame for out of range index ' + index,
);
const item = getItem(data, index);
const frame =
item != null
? this._frames[this._keyExtractor(item, index, props)]
: undefined;
const frame = this._frames[this._keyExtractor(item, index, props)];
if (!frame || frame.index !== index) {
if (getItemLayout) {
/* $FlowFixMe[prop-missing] (>=0.63.0 site=react_native_fb) This comment
Expand Down

0 comments on commit 011ea33

Please sign in to comment.