Skip to content

Commit

Permalink
Fix crash with SectionList.scrollToLocation and private property munging
Browse files Browse the repository at this point in the history 
Summary:
Changelog:
[General][Fixed] - Remove illegal private property access in VirtualizedSectionList.scrollToLocation

`VirtualizedSectionList.scrollToLocation` internally uses `VirtualizedList`'s `_getFrameMetricsApprox` method. This method is private by convention (since it's `_`-prefixed), but under certain build setups this is also enforced at runtime, so using `scrollToLocation` can throw an error.

Here, we rename this internal method to `__getFrameMetricsApprox` (adding another leading underscore) which opts it out of being treated as private, while still communicating that it's not part of the public API. We also delete the Flow error suppression that masked this issue.

For reference: This convention for private methods (including the double-underscore opt out) has its roots in Facebook's pre-Babel [JSTransform](https://github.com/facebookarchive/jstransform/blob/master/visitors/es6-class-visitors.js) compiler and is implemented in Flow as [`munge_underscores=true`](https://flow.org/en/docs/config/options/#toc-munge-underscores-boolean).

Reviewed By: yungsters

Differential Revision: D33982339

fbshipit-source-id: 498563c59d42549c94fe90d363677d6d3ea35d2d
  • Loading branch information
@motiz88 @facebook-github-bot
motiz88 authored and facebook-github-bot committed Feb 4, 2022
1 parent 9cd4334 commit b2f871a
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 21 deletions.
36 changes: 17 additions & 19 deletions Libraries/Lists/VirtualizedList.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
scrollToEnd(params?: ?{animated?: ?boolean, ...}) {
const animated = params ? params.animated : true;
const veryLast = this.props.getItemCount(this.props.data) - 1;
const frame = this._getFrameMetricsApprox(veryLast);
const frame = this.__getFrameMetricsApprox(veryLast);
const offset = Math.max(
0,
frame.offset +
Expand Down Expand Up @@ -458,7 +458,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
});
return;
}
const frame = this._getFrameMetricsApprox(index);
const frame = this.__getFrameMetricsApprox(index);
const offset =
Math.max(
0,
Expand Down Expand Up @@ -950,8 +950,8 @@ class VirtualizedList extends React.PureComponent<Props, State> {
// See if there are any sticky headers in the virtualized space that we need to render.
for (let ii = firstAfterInitial - 1; ii > lastInitialIndex; ii--) {
if (stickyIndicesFromProps.has(ii + stickyOffset)) {
const initBlock = this._getFrameMetricsApprox(lastInitialIndex);
const stickyBlock = this._getFrameMetricsApprox(ii);
const initBlock = this.__getFrameMetricsApprox(lastInitialIndex);
const stickyBlock = this.__getFrameMetricsApprox(ii);
const leadSpace =
stickyBlock.offset -
initBlock.offset -
Expand All @@ -968,7 +968,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
inversionStyle,
);
const trailSpace =
this._getFrameMetricsApprox(first).offset -
this.__getFrameMetricsApprox(first).offset -
(stickyBlock.offset + stickyBlock.length);
cells.push(
<View key="$sticky_trail" style={{[spacerKey]: trailSpace}} />,
Expand All @@ -979,9 +979,9 @@ class VirtualizedList extends React.PureComponent<Props, State> {
}
}
if (!insertedStickySpacer) {
const initBlock = this._getFrameMetricsApprox(lastInitialIndex);
const initBlock = this.__getFrameMetricsApprox(lastInitialIndex);
const firstSpace =
this._getFrameMetricsApprox(first).offset -
this.__getFrameMetricsApprox(first).offset -
(initBlock.offset + initBlock.length);
cells.push(
<View key="$lead_spacer" style={{[spacerKey]: firstSpace}} />,
Expand All @@ -1005,14 +1005,14 @@ class VirtualizedList extends React.PureComponent<Props, State> {
this._hasWarned.keys = true;
}
if (!isVirtualizationDisabled && last < itemCount - 1) {
const lastFrame = this._getFrameMetricsApprox(last);
const lastFrame = this.__getFrameMetricsApprox(last);
// Without getItemLayout, we limit our tail spacer to the _highestMeasuredFrameIndex to
// prevent the user for hyperscrolling into un-measured area because otherwise content will
// likely jump around as it renders in above the viewport.
const end = this.props.getItemLayout
? itemCount - 1
: Math.min(itemCount - 1, this._highestMeasuredFrameIndex);
const endFrame = this._getFrameMetricsApprox(end);
const endFrame = this.__getFrameMetricsApprox(end);
const tailSpacerLength =
endFrame.offset +
endFrame.length -
Expand Down Expand Up @@ -1419,16 +1419,16 @@ class VirtualizedList extends React.PureComponent<Props, State> {
const framesInLayout = [];
const itemCount = this.props.getItemCount(this.props.data);
for (let ii = 0; ii < itemCount; ii++) {
const frame = this._getFrameMetricsApprox(ii);
const frame = this.__getFrameMetricsApprox(ii);
/* $FlowFixMe[prop-missing] (>=0.68.0 site=react_native_fb) This comment
* suppresses an error found when Flow v0.68 was deployed. To see the
* error delete this comment and run Flow. */
if (frame.inLayout) {
framesInLayout.push(frame);
}
}
const windowTop = this._getFrameMetricsApprox(this.state.first).offset;
const frameLast = this._getFrameMetricsApprox(this.state.last);
const windowTop = this.__getFrameMetricsApprox(this.state.first).offset;
const frameLast = this.__getFrameMetricsApprox(this.state.last);
const windowLen = frameLast.offset + frameLast.length - windowTop;
const visTop = this._scrollMetrics.offset;
const visLen = this._scrollMetrics.visibleLength;
Expand Down Expand Up @@ -1642,15 +1642,15 @@ class VirtualizedList extends React.PureComponent<Props, State> {
// Mark as high priority if we're close to the start of the first item
// But only if there are items before the first rendered item
if (first > 0) {
const distTop = offset - this._getFrameMetricsApprox(first).offset;
const distTop = offset - this.__getFrameMetricsApprox(first).offset;
hiPri =
hiPri || distTop < 0 || (velocity < -2 && distTop < scrollingThreshold);
}
// Mark as high priority if we're close to the end of the last item
// But only if there are items after the last rendered item
if (last < itemCount - 1) {
const distBottom =
this._getFrameMetricsApprox(last).offset - (offset + visibleLength);
this.__getFrameMetricsApprox(last).offset - (offset + visibleLength);
hiPri =
hiPri ||
distBottom < 0 ||
Expand Down Expand Up @@ -1752,7 +1752,7 @@ class VirtualizedList extends React.PureComponent<Props, State> {
maxToRenderPerBatchOrDefault(this.props.maxToRenderPerBatch),
windowSizeOrDefault(this.props.windowSize),
state,
this._getFrameMetricsApprox,
this.__getFrameMetricsApprox,
this._scrollMetrics,
);
}
Expand Down Expand Up @@ -1815,13 +1815,11 @@ class VirtualizedList extends React.PureComponent<Props, State> {
return {index, item, key: this._keyExtractor(item, index), isViewable};
};

_getFrameMetricsApprox = (
index: number,
): {
__getFrameMetricsApprox: (index: number) => {
length: number,
offset: number,
...
} => {
} = index => {
const frame = this._getFrameMetrics(index);
if (frame && frame.index === index) {
// check for invalid frames due to row re-ordering
Expand Down
3 changes: 1 addition & 2 deletions Libraries/Lists/VirtualizedSectionList.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,7 @@ class VirtualizedSectionList<
return;
}
if (params.itemIndex > 0 && this.props.stickySectionHeadersEnabled) {
// $FlowFixMe[prop-missing] Cannot access private property
const frame = this._listRef._getFrameMetricsApprox(
const frame = this._listRef.__getFrameMetricsApprox(
index - params.itemIndex,
);
viewOffset += frame.length;
Expand Down

0 comments on commit b2f871a

Please sign in to comment.