Skip to content

Commit 54095c5

Browse files
CollinBeczakCollinBeczak
authored
fix component unmounting errors (#2778)
* fix component unmounting errors * run formatting
1 parent 710ed08 commit 54095c5

File tree

11 files changed

+120
-60
lines changed

11 files changed

+120
-60
lines changed

‎src/components/AdminPane/Manage/ProjectPickerModal/ProjectPickerModal.jsx‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { get as levenshtein } from "fast-levenshtein";
22
import _compact from "lodash/compact";
33
import _isEmpty from "lodash/isEmpty";
44
import _map from "lodash/map";
5-
import { useMemo, useState } from "react";
5+
import { useEffect, useMemo, useRef, useState } from "react";
66
import { FormattedMessage } from "react-intl";
77
import BusySpinner from "../../../BusySpinner/BusySpinner";
88
import WithPagedProjects from "../../../HOCs/WithPagedProjects/WithPagedProjects";
@@ -14,12 +14,20 @@ import messages from "./Messages";
1414

1515
export function ProjectPickerModal(props) {
1616
const [isSearching, setIsSearching] = useState(false);
17+
const mountedRef = useRef(true);
18+
useEffect(
19+
() => () => {
20+
mountedRef.current = false;
21+
},
22+
[],
23+
);
1724

1825
const executeSearch = (queryCriteria) => {
1926
if (!queryCriteria.query) {
20-
return; // nothing to do
27+
return;
2128
}
2229

30+
if (!mountedRef.current) return;
2331
setIsSearching(true);
2432
props
2533
.searchProjects(
@@ -31,7 +39,7 @@ export function ProjectPickerModal(props) {
3139
queryCriteria?.page?.resultsPerPage,
3240
)
3341
.finally(() => {
34-
setIsSearching(false);
42+
if (mountedRef.current) setIsSearching(false);
3543
});
3644
};
3745

‎src/components/ChallengeDetail/ChallengeDetail.jsx‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ export class ChallengeDetail extends Component {
112112
resetSelectedClusters = () => this.setState({ selectedClusters: [] });
113113

114114
componentDidMount() {
115+
this._isMounted = true;
115116
window.scrollTo(0, 0);
116117

117118
const { url, params } = this.props.match;
@@ -121,6 +122,10 @@ export class ChallengeDetail extends Component {
121122
}
122123
}
123124

125+
componentWillUnmount() {
126+
this._isMounted = false;
127+
}
128+
124129
componentDidUpdate() {
125130
if (!_isObject(this.props.user) && this.state.detailTab === DETAIL_TABS.COMMENTS) {
126131
this.setState({ detailTab: DETAIL_TABS.OVERVIEW });
@@ -150,6 +155,7 @@ export class ChallengeDetail extends Component {
150155

151156
if (response.ok) {
152157
const body = await response.json();
158+
if (!this._isMounted) return;
153159
if (body?.total_count) {
154160
this.setState({ issue: body.items[0] });
155161
}
@@ -263,9 +269,8 @@ export class ChallengeDetail extends Component {
263269
disabled
264270
className="mr-bg-black-15 mr-w-full mr-p-2 mr-text-sm"
265271
style={{ height: 500 }}
266-
>
267-
{challenge.overpassQL}
268-
</textarea>
272+
defaultValue={challenge.overpassQL}
273+
/>
269274
);
270275
case DETAIL_TABS.COMMENTS:
271276
return (

‎src/components/Dropdown/Dropdown.jsx‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,18 @@ const Dropdown = ({
2727
const [visible, setVisible] = useState(false);
2828
const referenceRef = useRef();
2929
const popperRef = useRef();
30+
const visibleTimeoutRef = useRef();
31+
32+
useEffect(() => {
33+
return () => clearTimeout(visibleTimeoutRef.current);
34+
}, []);
3035

3136
const toggle = useCallback(
3237
(bool) => {
3338
setActive(bool);
3439
toggleVisible();
35-
setTimeout(() => setVisible(bool), 1);
40+
clearTimeout(visibleTimeoutRef.current);
41+
visibleTimeoutRef.current = setTimeout(() => setVisible(bool), 1);
3642
},
3743
[toggleVisible],
3844
);

‎src/components/HOCs/WithChallengeTaskClusters/WithChallengeTaskClusters.jsx‎

Lines changed: 39 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -157,62 +157,61 @@ export const WithChallengeTaskClusters = function (
157157
ignoreLocked,
158158
)
159159
.then((results) => {
160-
if (currentFetchId >= this.state.fetchId) {
161-
const totalCount = results.length;
162-
// If we retrieved 1001 tasks then there might be more tasks and
163-
// they should be clustered. So fetch as clusters
164-
// (unless we are zoomed all the way in already)
165-
if (totalCount > UNCLUSTER_THRESHOLD && (this.props.criteria?.zoom ?? 0) < MAX_ZOOM) {
166-
this.props
167-
.fetchTaskClusters(challengeId, searchCriteria, 25, overrideDisable)
168-
.then((results) => {
169-
const clusters = results.clusters;
170-
if (currentFetchId >= this.state.fetchId) {
171-
const taskCount = _sum(_map(clusters, (c) => c.numberOfPoints));
172-
this.setState({
173-
clusters,
174-
loading: false,
175-
taskCount: taskCount,
176-
showAsClusters: true,
177-
});
178-
}
160+
if (!this._isMounted || currentFetchId < this.state.fetchId) return;
161+
const totalCount = results.length;
162+
// If we retrieved 1001 tasks then there might be more tasks and
163+
// they should be clustered. So fetch as clusters
164+
// (unless we are zoomed all the way in already)
165+
if (totalCount > UNCLUSTER_THRESHOLD && (this.props.criteria?.zoom ?? 0) < MAX_ZOOM) {
166+
this.props
167+
.fetchTaskClusters(challengeId, searchCriteria, 25, overrideDisable)
168+
.then((results) => {
169+
if (!this._isMounted || currentFetchId < this.state.fetchId) return;
170+
const clusters = results.clusters;
171+
const taskCount = _sum(_map(clusters, (c) => c.numberOfPoints));
172+
this.setState({
173+
clusters,
174+
loading: false,
175+
taskCount: taskCount,
176+
showAsClusters: true,
179177
});
180-
} else {
181-
this.setState({
182-
clusters: results,
183-
loading: false,
184-
taskCount: totalCount,
185178
});
186-
}
179+
} else {
180+
this.setState({
181+
clusters: results,
182+
loading: false,
183+
taskCount: totalCount,
184+
});
187185
}
188186
})
189187
.catch((error) => {
190188
console.log(error);
191-
this.setState({ clusters: {}, loading: false, taskCount: 0 });
189+
if (this._isMounted) this.setState({ clusters: {}, loading: false, taskCount: 0 });
192190
});
193191
} else {
194192
this.props
195193
.fetchTaskClusters(challengeId, searchCriteria, 25, overrideDisable)
196194
.then((results) => {
195+
if (!this._isMounted || currentFetchId < this.state.fetchId) return;
197196
const clusters = results.clusters;
198-
if (currentFetchId >= this.state.fetchId) {
199-
const taskCount = _sum(_map(clusters, (c) => c.numberOfPoints));
197+
const taskCount = _sum(_map(clusters, (c) => c.numberOfPoints));
198+
this.setState({
199+
clusters,
200+
loading: false,
201+
taskCount: taskCount,
202+
showAsClusters: true,
203+
});
204+
})
205+
.catch((error) => {
206+
console.log(error);
207+
if (this._isMounted) {
200208
this.setState({
201-
clusters,
209+
clusters: {},
202210
loading: false,
203-
taskCount: taskCount,
211+
taskCount: 0,
204212
showAsClusters: true,
205213
});
206214
}
207-
})
208-
.catch((error) => {
209-
console.log(error);
210-
this.setState({
211-
clusters: {},
212-
loading: false,
213-
taskCount: 0,
214-
showAsClusters: true,
215-
});
216215
});
217216
}
218217
}
@@ -278,6 +277,7 @@ export const WithChallengeTaskClusters = function (
278277
};
279278

280279
updateTaskInClusters = async (updatedTasks) => {
280+
if (!this._isMounted) return;
281281
if (!Array.isArray(updatedTasks)) {
282282
updatedTasks = [updatedTasks];
283283
}

‎src/components/HOCs/WithFeatured/WithFeatured.jsx‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,11 @@ const WithFeatured = (WrappedComponent, options = {}) => {
2222
};
2323

2424
componentDidMount() {
25+
this._isMounted = true;
2526
// Fetch featured challenges
2627
if (!options.excludeChallenges) {
2728
this.props.fetchFeaturedChallenges().then((normalizedResults) => {
29+
if (!this._isMounted) return;
2830
if (normalizedResults && !_isEmpty(normalizedResults.entities)) {
2931
this.setState({
3032
featuredChallenges: _filter(
@@ -39,6 +41,7 @@ const WithFeatured = (WrappedComponent, options = {}) => {
3941
// Fetch featured projects
4042
if (!options.excludeProjects) {
4143
this.props.fetchFeaturedProjects().then((normalizedResults) => {
44+
if (!this._isMounted) return;
4245
if (normalizedResults && !_isEmpty(normalizedResults.entities)) {
4346
this.setState({
4447
featuredProjects: _values(normalizedResults.entities.projects),
@@ -48,6 +51,10 @@ const WithFeatured = (WrappedComponent, options = {}) => {
4851
}
4952
}
5053

54+
componentWillUnmount() {
55+
this._isMounted = false;
56+
}
57+
5158
render() {
5259
return (
5360
<WrappedComponent

‎src/components/HOCs/WithLeaderboard/WithLeaderboard.jsx‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,11 @@ const WithLeaderboard = function (WrappedComponent, initialMonthsPast = 1, initi
114114
startDate,
115115
endDate,
116116
).then((leaderboard) => {
117+
if (!this._isMounted) return;
117118
if (currentFetch >= this.state.fetchId) {
118119
this.setState({ leaderboard });
119120

120121
const userId = this.props.user?.id;
121-
// The reason for using _get is that the structure of the props may vary
122-
// depending on where this component is used, and accessing the user's score
123-
// directly through `this.props.user.score` may result in runtime errors if
124-
// the `user` object or the `score` property is not available in certain contexts.
125-
// By using `_get`, we safely handle cases where the expected property may be missing
126-
// or nested within a deeper structure.
127122
const userScore = this.props.user?.score;
128123
if (userScore && userId && !options.ignoreUser && userType !== USER_TYPE_REVIEWER) {
129124
this.props
@@ -135,6 +130,7 @@ const WithLeaderboard = function (WrappedComponent, initialMonthsPast = 1, initi
135130
endDate,
136131
)
137132
.then((userLeaderboard) => {
133+
if (!this._isMounted) return;
138134
this.mergeInUserLeaderboard(userLeaderboard);
139135
this.setState({ leaderboardLoading: false });
140136
});
@@ -228,6 +224,7 @@ const WithLeaderboard = function (WrappedComponent, initialMonthsPast = 1, initi
228224
};
229225

230226
componentDidMount() {
227+
this._isMounted = true;
231228
if (!initialOptions.isWidget) {
232229
this.updateLeaderboard(
233230
this.monthsPast(),
@@ -239,6 +236,10 @@ const WithLeaderboard = function (WrappedComponent, initialMonthsPast = 1, initi
239236
}
240237
}
241238

239+
componentWillUnmount() {
240+
this._isMounted = false;
241+
}
242+
242243
componentDidUpdate(prevProps) {
243244
// A change to state will also fetch leaderboard data, so we only need to
244245
// worry about fetching if we're controlled and props change.

‎src/components/HOCs/WithNearbyTasks/WithNearbyTasks.jsx‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ export const WithNearbyTasks = function (WrappedComponent) {
8686
: MAX_NEARBY_TASK_LIMIT,
8787
);
8888

89+
if (!this._isMounted) return;
8990
const tasksLength = nearbyTasks.tasks.length;
9091
this.setState({
9192
nearbyTasks: {
@@ -100,7 +101,7 @@ export const WithNearbyTasks = function (WrappedComponent) {
100101
});
101102
} catch (error) {
102103
console.error("Error fetching nearby tasks:", error);
103-
this.setState({ loading: false });
104+
if (this._isMounted) this.setState({ loading: false });
104105
}
105106
}
106107
};
@@ -141,6 +142,7 @@ export const WithNearbyTasks = function (WrappedComponent) {
141142
boundingBox,
142143
MAX_NEARBY_TASK_LIMIT,
143144
);
145+
if (!this._isMounted) return;
144146
const tasksLength = nearbyTasks.tasks?.length;
145147

146148
if (tasksLength > 0) {
@@ -161,9 +163,14 @@ export const WithNearbyTasks = function (WrappedComponent) {
161163
};
162164

163165
componentDidMount() {
166+
this._isMounted = true;
164167
this.updateNearbyTasks();
165168
}
166169

170+
componentWillUnmount() {
171+
this._isMounted = false;
172+
}
173+
167174
componentDidUpdate(prevProps) {
168175
if (this.props.task && this.props.task?.id !== prevProps.task?.id) {
169176
this.setState({

‎src/components/HOCs/WithProgress/WithProgress.jsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,18 @@ const WithProgress = function (WrappedComponent, operationName) {
1414
stepsCompleted: 0,
1515
};
1616

17+
componentDidMount() {
18+
this._isMounted = true;
19+
}
20+
21+
componentWillUnmount() {
22+
this._isMounted = false;
23+
}
24+
1725
updateProgress = (inProgress, stepsCompleted) => {
18-
this.setState({ inProgress, stepsCompleted });
26+
if (this._isMounted) {
27+
this.setState({ inProgress, stepsCompleted });
28+
}
1929
};
2030

2131
render() {

‎src/components/HOCs/WithSystemNotices/WithSystemNotices.jsx‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,18 @@ export const WithSystemNotices = function (WrappedComponent) {
1818
};
1919

2020
async componentDidMount() {
21+
this._isMounted = true;
2122
if (!this.state.systemNotices) {
2223
const activeNotices = await fetchActiveSystemNotices();
23-
24+
if (!this._isMounted) return;
2425
this.setState({ systemNotices: activeNotices });
2526
}
2627
}
2728

29+
componentWillUnmount() {
30+
this._isMounted = false;
31+
}
32+
2833
/**
2934
* Retrieves all acknowledged notices from the user's app settings
3035
*

‎src/components/TaskPane/ActiveTaskDetails/ActiveTaskControls/ActiveTaskControls.jsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,10 +178,10 @@ export class ActiveTaskControls extends Component {
178178
}
179179
}
180180
} catch (error) {
181-
this.setState({ completingTask: false });
181+
if (this._isMounted) this.setState({ completingTask: false });
182182
throw error;
183183
} finally {
184-
this.setState({ completingTask: false });
184+
if (this._isMounted) this.setState({ completingTask: false });
185185
}
186186
};
187187

@@ -372,7 +372,12 @@ export class ActiveTaskControls extends Component {
372372
}
373373
}
374374

375+
componentDidMount() {
376+
this._isMounted = true;
377+
}
378+
375379
componentWillUnmount() {
380+
this._isMounted = false;
376381
if (!_isEmpty(this.props.activeKeyboardShortcuts?.[hiddenShortcutGroup])) {
377382
for (const shortcut of hiddenShortcuts) {
378383
this.props.deactivateKeyboardShortcut(

0 commit comments

Comments
 (0)