Skip to content

Commit d6bef53

Browse files
MelvinBotsituchan
MelvinBot
and
situchan
committed
Restructure GPA 2.6.4 patches into 4 focused patches and add AddressSearch list test
Co-authored-by: Situ Chandra Shil <situchan@users.noreply.github.com>
1 parent 1fc9490 commit d6bef53

7 files changed

Lines changed: 462 additions & 406 deletions

‎patches/react-native-google-places-autocomplete/details.md‎

Lines changed: 102 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1,112 +1,110 @@
11
# `react-native-google-places-autocomplete` patches
22

3-
### [react-native-google-places-autocomplete+2.6.4+001+fix-tdz-crash.patch](react-native-google-places-autocomplete+2.6.4+001+fix-tdz-crash.patch)
3+
### [react-native-google-places-autocomplete+2.6.4+001+keyboard-navigation.patch](react-native-google-places-autocomplete+2.6.4+001+keyboard-navigation.patch)
44

55
- Reason:
66

77
```
8-
Fixes TDZ crashes and reorders forward-referenced declarations in v2.6.4:
9-
10-
1. `useRef(_request)` on line 161 references `_request` before its `const`
11-
declaration on line 466, causing
12-
`ReferenceError: Cannot access '_request' before initialization`.
13-
Fix: replace the initial value with `null` (safe because
14-
`requestRef.current` is reassigned to `_request` every render before
15-
it can be invoked).
16-
17-
2. `_disableRowLoaders` (a `const useCallback`) was declared on line 653
18-
but referenced in the `useCallback` dependency arrays of
19-
`_requestNearby` (line 450) and `getCurrentLocation` (line 629),
20-
causing `ReferenceError: Cannot access '_disableRowLoaders' before
21-
initialization`. Fix: move `_disableRowLoaders` above
22-
`_requestNearby` so it is defined before first use.
23-
24-
3. Several functions were declared after their callers, creating forward
25-
references. While these only execute lazily (not during render), they
26-
are reordered for clarity and consistency:
27-
- `_renderDescription`, `hideListView`, `isNewFocusInAutocompleteResultList`,
28-
`_onBlur`, `_onFocus` moved before `_onPress` (which calls them).
29-
- `debounceData` moved before `_onChangeText` (which calls it).
30-
31-
4. v2.6.4 added `hideListView(true)` at the start of `_onPress` in the
32-
`fetchDetails` branch. This immediately hides the entire FlatList
33-
before `_enableRowLoader` can display the per-row loading spinner,
34-
causing the loading indicator to be invisible during place detail
35-
fetches. Fix: remove `hideListView(true)` from the `fetchDetails`
36-
branch of `_onPress`, restoring v2.5.6 behavior where the list
37-
stays visible with the row spinner until the detail request completes.
38-
The `hideListView(true)` calls in the `isCurrentLocation` and
39-
predefined-place branches are left intact since those don't need
40-
to show a row-level loading indicator.
41-
42-
5. v2.6.4 changed the `_getFlatList` visibility condition from
43-
`stateText !== ''` (v2.5.6) to `dataSource.length > 0`. This
44-
prevents the FlatList from mounting when the user is typing but
45-
results haven't arrived yet, so the `ListEmptyComponent` loading
46-
spinner is never shown during in-flight requests. Fix: replace
47-
`dataSource.length > 0` with the v2.5.6-style condition
48-
`(stateText !== '' || predefinedPlaces.length > 0 || currentLocation === true)`
49-
so the FlatList renders while results are loading. This matches
50-
v2.5.6 production behavior where existing results stay visible
51-
while new results load — `_request` does NOT clear `dataSource`
52-
before sending the XHR, so previous results remain visible until
53-
the new response arrives.
54-
55-
6. v2.6.4 added `stateText` to the dependency array of the
56-
query-change `useEffect` (the effect that reloads search when
57-
`props.query` changes). This causes the cleanup function
58-
(`_abortRequests`) to fire on every keystroke, aborting in-flight
59-
XHRs before results can arrive. When the user types fast, results
60-
from previous keystrokes never complete, `dataSource` stays empty,
61-
and the `ListEmptyComponent` loading spinner flashes on every key.
62-
Fix: remove `stateText` from the dependency array. Per-keystroke
63-
requests are already handled by `_onChangeText` → `debounceData`,
64-
and `_request` itself calls `_abortRequests()` at the top, so old
65-
requests are properly aborted when a new one fires.
66-
67-
7. In v2.6.4, `requestsRef` is a `useRef` that persists across renders
68-
(unlike v2.5.6 where `_requests` was a plain `let` re-initialized
69-
every render). This means `_abortRequests()` inside `_request` now
70-
actually aborts the previous in-flight XHR. When the user types
71-
faster than the API responds, each keystroke aborts the prior XHR
72-
before it populates `dataSource`, keeping `dataSource` empty and
73-
causing `ListEmptyComponent` to show the loading spinner on every
74-
keystroke. Fix: only call `setListLoaderDisplayed(true)` in the
75-
`onreadystatechange` handler when `resultsRef.current.length === 0`
76-
(i.e., no prior results exist). When prior results already exist
77-
they remain visible in the FlatList while the new request is
78-
in-flight, matching v2.5.6 production behavior. Applied to both
79-
`_request` and `_requestNearby`.
80-
81-
8. When `resultsRef.current.length === 0` and the loading spinner is
82-
enabled, `dataSource` may still contain predefined places (populated
83-
by `buildRowsFromResults([])` when the user cleared the input).
84-
Since `ListEmptyComponent` only renders when `data` is empty,
85-
the loading spinner was invisible even though `listLoaderDisplayed`
86-
was true. Fix: call `setDataSource([])` alongside
87-
`setListLoaderDisplayed(true)` to clear stale predefined places
88-
from the FlatList data, allowing `ListEmptyComponent` to render
89-
the loading spinner. Applied to both `_request` and
90-
`_requestNearby`.
91-
92-
9. v2.6.4 replaced `TouchableHighlight` with `Pressable` for suggestion
93-
rows AND added `onBlur={_onBlur}` to the Pressable. This causes two
94-
problems on iOS:
95-
(a) `Pressable` does not participate in the gesture responder system
96-
the same way `TouchableHighlight` does. When a parent `ScrollView`
97-
uses `keyboardShouldPersistTaps="handled"` (as `FormWrapper` does),
98-
`TouchableHighlight` counts as "handling" the tap so the keyboard
99-
stays up and `onPress` fires. `Pressable` does not, so the first
100-
tap dismisses the keyboard without firing `onPress`, making
101-
suggestion selection appear broken.
102-
(b) The `onBlur={_onBlur}` on the Pressable fires when the keyboard
103-
dismisses, calling `hideListView()` and `inputRef.current.blur()`,
104-
which cascades to AddressSearch collapsing the list to zero height.
105-
Fix: replace `Pressable` with `TouchableHighlight` (restoring v2.5.6
106-
behavior) and remove the `onBlur` handler. `TouchableHighlight` uses
107-
`underlayColor` for press feedback instead of function-style styles.
8+
This patch adds keyboard accessibility to autocomplete result rows.
9+
The row Pressable elements lacked tabIndex, making them unreachable
10+
via Tab key navigation. When tabbing from the text input, focus would
11+
leave the container, triggering onBlur which hid the list before any
12+
selection could occur. Adding tabIndex={0}, accessible,
13+
accessibilityRole="button", and a Space onKeyDown handler makes rows
14+
keyboard-focusable and selectable. The accessibilityRole="button" is
15+
critical: it causes useActiveElementRole to return "button" when a row
16+
is focused, which disables the form's pressOnEnter keyboard shortcut
17+
(via shouldDisableEnterShortcut in Button) so Enter reaches the row's
18+
own onPress handler instead of submitting the form. Space is handled
19+
via onKeyDown to also prevent page scroll.
10820
```
10921
110-
- Upstream PR/issue: 🛑, library is unmaintained (https://github.com/FaridSafi/react-native-google-places-autocomplete/issues/978)
111-
- E/App issue: https://github.com/Expensify/App/pull/82233
112-
- PR introducing patch: https://github.com/Expensify/App/pull/82233
22+
- E/App issue: https://github.com/Expensify/App/issues/79621
23+
24+
### [react-native-google-places-autocomplete+2.6.4+002+fix-tdz-crash-on-render.patch](react-native-google-places-autocomplete+2.6.4+002+fix-tdz-crash-on-render.patch)
25+
26+
- Reason:
27+
28+
```
29+
Upstream 2.6.4 crashes on the component's very first render with
30+
"ReferenceError: Cannot access '_request' before initialization".
31+
The 2.6.x rewrite introduced two temporal dead zone (TDZ) bugs where
32+
component-scope `const`s are read during render before they are
33+
declared:
34+
35+
1. `const requestRef = useRef(_request)` reads `_request`, which is
36+
declared ~300 lines later. Fixed by initializing the ref to `null`;
37+
the component already assigns `requestRef.current = _request` on
38+
every render, and the ref is only ever read from inside the
39+
debounced callback, which cannot fire before that assignment.
40+
2. `_disableRowLoaders` (a `useCallback`) appears in the dependency
41+
arrays of two earlier hooks. Dependency arrays are evaluated during
42+
render, so both reads hit the TDZ. Fixed by moving the
43+
`_disableRowLoaders` declaration above its first use; it only
44+
depends on `buildRowsFromResults`, which is declared earlier still,
45+
so the move is behavior-preserving.
46+
47+
Note: Jest cannot reproduce this crash because Babel transpiles
48+
`const` to `var` in the test environment, which erases TDZ semantics.
49+
It reproduces under Hermes and in browsers, where `const` is native.
50+
```
51+
52+
- Upstream issue: not yet reported at the time of writing (2.6.4 is the latest release).
53+
54+
### [react-native-google-places-autocomplete+2.6.4+003+restore-list-loading-state.patch](react-native-google-places-autocomplete+2.6.4+003+restore-list-loading-state.patch)
55+
56+
- Reason:
57+
58+
```
59+
Upstream 2.6.4 gates the whole result list on `dataSource.length > 0`, but the
60+
loader and the "no results" state are rendered through the FlatList's
61+
ListEmptyComponent, which by definition only renders when the list IS empty.
62+
Those two conditions are mutually exclusive, so both `listLoaderComponent` and
63+
`listEmptyComponent` became unreachable dead props in 2.6.x.
64+
65+
AddressSearch passes both, so upgrading from 2.5.6 silently dropped the address
66+
search spinner and the "no results found" message.
67+
68+
This patch restores the 2.5.6 behavior by also rendering the list when there is
69+
an empty state to show. The `stateText.length > minLength` guard mirrors 2.5.6's
70+
`stateText !== ''` gate; it matters because `_request` clears results without
71+
resetting `listLoaderDisplayed`, so without it an aborted in-flight request
72+
could leave a spinner on screen after the input is cleared.
73+
74+
Resulting behavior (matches 2.5.6 / production):
75+
- loader shows only while searching AND there are no previous results
76+
- previous results stay visible, with no loader, while the next search runs
77+
- the empty state shows when a search comes back with nothing
78+
79+
Covered by tests/unit/AddressSearchListTest.tsx.
80+
```
81+
82+
- Upstream issue: not yet reported at the time of writing (2.6.4 is the latest release).
83+
84+
### [react-native-google-places-autocomplete+2.6.4+004+restore-predefined-places-updates.patch](react-native-google-places-autocomplete+2.6.4+004+restore-predefined-places-updates.patch)
85+
86+
- Reason:
87+
88+
```
89+
In 2.5.6 the effect that rebuilds the list from `predefinedPlaces` depended on
90+
`props.predefinedPlaces`, so the list reacted to that prop changing. In 2.6.4 the
91+
same effect became mount-only (`}, []`), so predefined places that arrive or are
92+
filtered out after mount never reach the list.
93+
94+
AddressSearch passes `filteredPredefinedPlaces` (recent destinations, filtered by
95+
the search text and hidden once the user starts typing), so on 2.6.4 the recent
96+
destinations went stale: they never updated, and stale rows kept the list
97+
non-empty, which also suppressed the loader added in patch 003.
98+
99+
Restoring the dependency requires a second change. 2.5.6 held its defaults in one
100+
module-scope `defaultProps` object, so the default `predefinedPlaces` array had a
101+
stable identity. 2.6.4 moved defaults into destructuring (`= []`), which allocates
102+
a new array on every render — with the dependency restored that feeds an infinite
103+
render loop (effect -> setDataSource -> render -> new array -> effect). Hoisting
104+
the default to a module-level EMPTY_PREDEFINED_PLACES restores the stable identity
105+
2.5.6 had.
106+
107+
Covered by tests/unit/AddressSearchListTest.tsx.
108+
```
109+
110+
- Upstream issue: not yet reported at the time of writing (2.6.4 is the latest release).

0 commit comments

Comments
 (0)