From f63312c81135cd0bd53b750d5b7eabd0a59bd784 Mon Sep 17 00:00:00 2001
From: mabashian <mabashia@redhat.com>
Date: Thu, 29 Apr 2021 13:53:53 -0400
Subject: [PATCH 1/2] Prevent multi credential state updates from happening
 after unmount

---
 .../Lookup/MultiCredentialsLookup.jsx         | 24 ++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx
index ecc1a268c4..d64c397c7e 100644
--- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx
+++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx
@@ -1,5 +1,11 @@
 import 'styled-components/macro';
-import React, { Fragment, useState, useCallback, useEffect } from 'react';
+import React, {
+  Fragment,
+  useState,
+  useCallback,
+  useEffect,
+  useRef,
+} from 'react';
 import { withRouter } from 'react-router-dom';
 import PropTypes from 'prop-types';
 import { withI18n } from '@lingui/react';
@@ -26,6 +32,7 @@ async function loadCredentials(params, selectedCredentialTypeId) {
 }
 
 function MultiCredentialsLookup(props) {
+  const isMounted = useRef(null);
   const { value, onChange, onError, history, i18n } = props;
   const [selectedType, setSelectedType] = useState(null);
 
@@ -37,6 +44,9 @@ function MultiCredentialsLookup(props) {
   } = useRequest(
     useCallback(async () => {
       const types = await CredentialTypesAPI.loadAllTypes();
+      if (!isMounted.current) {
+        return;
+      }
       const match = types.find(type => type.kind === 'ssh') || types[0];
       setSelectedType(match);
       return types;
@@ -45,7 +55,11 @@ function MultiCredentialsLookup(props) {
   );
 
   useEffect(() => {
+    isMounted.current = true;
     fetchTypes();
+    return () => {
+      isMounted.current = false;
+    };
   }, [fetchTypes]);
 
   const {
@@ -72,6 +86,10 @@ function MultiCredentialsLookup(props) {
         CredentialsAPI.readOptions(),
       ]);
 
+      if (!isMounted.current) {
+        return;
+      }
+
       results.map(result => {
         if (result.kind === 'vault' && result.inputs?.vault_id) {
           result.label = `${result.name} | ${result.inputs.vault_id}`;
@@ -101,7 +119,11 @@ function MultiCredentialsLookup(props) {
   );
 
   useEffect(() => {
+    isMounted.current = true;
     fetchCredentials();
+    return () => {
+      isMounted.current = false;
+    };
   }, [fetchCredentials]);
 
   useEffect(() => {

From 5b716814944ead54185d960466640cbaaf486a86 Mon Sep 17 00:00:00 2001
From: mabashian <mabashia@redhat.com>
Date: Fri, 30 Apr 2021 15:31:36 -0400
Subject: [PATCH 2/2] Fixes test warnings where state updates were being
 triggered after component unmounts

---
 .../Lookup/MultiCredentialsLookup.jsx         | 31 +++++--------------
 .../MultiSelect/useSyncedSelectValue.js       | 11 ++++++-
 .../Inventory/InventoryEdit/InventoryEdit.jsx | 10 +++---
 .../InventorySourceDetail.jsx                 |  9 ++----
 .../src/screens/Job/JobOutput/JobOutput.jsx   |  5 ++-
 .../Template/shared/JobTemplateForm.jsx       |  9 ++++--
 .../screens/Template/shared/LabelSelect.jsx   | 14 +++++++--
 awx/ui_next/src/util/useIsMounted.js          | 12 +++++++
 awx/ui_next/src/util/useRequest.js            | 12 ++-----
 9 files changed, 59 insertions(+), 54 deletions(-)
 create mode 100644 awx/ui_next/src/util/useIsMounted.js

diff --git a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx
index d64c397c7e..d2b4e3fb1e 100644
--- a/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx
+++ b/awx/ui_next/src/components/Lookup/MultiCredentialsLookup.jsx
@@ -1,11 +1,5 @@
 import 'styled-components/macro';
-import React, {
-  Fragment,
-  useState,
-  useCallback,
-  useEffect,
-  useRef,
-} from 'react';
+import React, { Fragment, useState, useCallback, useEffect } from 'react';
 import { withRouter } from 'react-router-dom';
 import PropTypes from 'prop-types';
 import { withI18n } from '@lingui/react';
@@ -18,6 +12,7 @@ import OptionsList from '../OptionsList';
 import useRequest from '../../util/useRequest';
 import { getQSConfig, parseQueryString } from '../../util/qs';
 import Lookup from './Lookup';
+import useIsMounted from '../../util/useIsMounted';
 
 const QS_CONFIG = getQSConfig('credentials', {
   page: 1,
@@ -32,9 +27,9 @@ async function loadCredentials(params, selectedCredentialTypeId) {
 }
 
 function MultiCredentialsLookup(props) {
-  const isMounted = useRef(null);
   const { value, onChange, onError, history, i18n } = props;
   const [selectedType, setSelectedType] = useState(null);
+  const isMounted = useIsMounted();
 
   const {
     result: credentialTypes,
@@ -44,22 +39,18 @@ function MultiCredentialsLookup(props) {
   } = useRequest(
     useCallback(async () => {
       const types = await CredentialTypesAPI.loadAllTypes();
-      if (!isMounted.current) {
-        return;
-      }
       const match = types.find(type => type.kind === 'ssh') || types[0];
-      setSelectedType(match);
+      if (isMounted.current) {
+        setSelectedType(match);
+      }
       return types;
+      /* eslint-disable-next-line react-hooks/exhaustive-deps */
     }, []),
     []
   );
 
   useEffect(() => {
-    isMounted.current = true;
     fetchTypes();
-    return () => {
-      isMounted.current = false;
-    };
   }, [fetchTypes]);
 
   const {
@@ -86,10 +77,6 @@ function MultiCredentialsLookup(props) {
         CredentialsAPI.readOptions(),
       ]);
 
-      if (!isMounted.current) {
-        return;
-      }
-
       results.map(result => {
         if (result.kind === 'vault' && result.inputs?.vault_id) {
           result.label = `${result.name} | ${result.inputs.vault_id}`;
@@ -119,11 +106,7 @@ function MultiCredentialsLookup(props) {
   );
 
   useEffect(() => {
-    isMounted.current = true;
     fetchCredentials();
-    return () => {
-      isMounted.current = false;
-    };
   }, [fetchCredentials]);
 
   useEffect(() => {
diff --git a/awx/ui_next/src/components/MultiSelect/useSyncedSelectValue.js b/awx/ui_next/src/components/MultiSelect/useSyncedSelectValue.js
index 1a62a09ec2..1b4c46195d 100644
--- a/awx/ui_next/src/components/MultiSelect/useSyncedSelectValue.js
+++ b/awx/ui_next/src/components/MultiSelect/useSyncedSelectValue.js
@@ -1,4 +1,5 @@
 import { useState, useEffect } from 'react';
+import useIsMounted from '../../util/useIsMounted';
 
 /*
   Hook for using PatternFly's <Select> component when a pre-existing value
@@ -9,8 +10,12 @@ import { useState, useEffect } from 'react';
 export default function useSyncedSelectValue(value, onChange) {
   const [options, setOptions] = useState([]);
   const [selections, setSelections] = useState([]);
+  const isMounted = useIsMounted();
 
   useEffect(() => {
+    if (!isMounted.current) {
+      return;
+    }
     const newOptions = [];
     if (value !== selections && options.length) {
       const syncedValue = value.map(item => {
@@ -41,7 +46,11 @@ export default function useSyncedSelectValue(value, onChange) {
     selections: options.length ? addToStringToObjects(selections) : [],
     onSelect,
     options,
-    setOptions: newOpts => setOptions(addToStringToObjects(newOpts)),
+    setOptions: newOpts => {
+      if (isMounted.current) {
+        setOptions(addToStringToObjects(newOpts));
+      }
+    },
   };
 }
 
diff --git a/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx b/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx
index f038ebf6fa..abd0ee3f5f 100644
--- a/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventoryEdit/InventoryEdit.jsx
@@ -1,4 +1,4 @@
-import React, { useState, useEffect, useRef } from 'react';
+import React, { useState, useEffect } from 'react';
 import { useHistory } from 'react-router-dom';
 import { object } from 'prop-types';
 
@@ -7,6 +7,7 @@ import { InventoriesAPI, CredentialTypesAPI } from '../../../api';
 import ContentLoading from '../../../components/ContentLoading';
 import InventoryForm from '../shared/InventoryForm';
 import { getAddedAndRemoved } from '../../../util/lists';
+import useIsMounted from '../../../util/useIsMounted';
 
 function InventoryEdit({ inventory }) {
   const [error, setError] = useState(null);
@@ -14,10 +15,9 @@ function InventoryEdit({ inventory }) {
   const [contentLoading, setContentLoading] = useState(true);
   const [credentialTypeId, setCredentialTypeId] = useState(null);
   const history = useHistory();
-  const isMounted = useRef(null);
+  const isMounted = useIsMounted();
 
   useEffect(() => {
-    isMounted.current = true;
     const loadData = async () => {
       try {
         const [
@@ -47,9 +47,7 @@ function InventoryEdit({ inventory }) {
       }
     };
     loadData();
-    return () => {
-      isMounted.current = false;
-    };
+    /* eslint-disable-next-line react-hooks/exhaustive-deps */
   }, [inventory.id, contentLoading, inventory, credentialTypeId]);
 
   const handleCancel = () => {
diff --git a/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx b/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx
index 486e7f5cdf..e2e0712547 100644
--- a/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx
+++ b/awx/ui_next/src/screens/Inventory/InventorySourceDetail/InventorySourceDetail.jsx
@@ -1,4 +1,4 @@
-import React, { useCallback, useEffect, useRef, useState } from 'react';
+import React, { useCallback, useEffect, useState } from 'react';
 import { Link, useHistory } from 'react-router-dom';
 import { withI18n } from '@lingui/react';
 import { t } from '@lingui/macro';
@@ -23,6 +23,7 @@ import Popover from '../../../components/Popover';
 import useRequest from '../../../util/useRequest';
 import { InventorySourcesAPI } from '../../../api';
 import { relatedResourceDeleteRequests } from '../../../util/getRelatedResourceDeleteDetails';
+import useIsMounted from '../../../util/useIsMounted';
 
 function InventorySourceDetail({ inventorySource, i18n }) {
   const {
@@ -57,7 +58,7 @@ function InventorySourceDetail({ inventorySource, i18n }) {
   } = inventorySource;
   const [deletionError, setDeletionError] = useState(false);
   const history = useHistory();
-  const isMounted = useRef(null);
+  const isMounted = useIsMounted();
 
   const {
     result: sourceChoices,
@@ -75,11 +76,7 @@ function InventorySourceDetail({ inventorySource, i18n }) {
   );
 
   useEffect(() => {
-    isMounted.current = true;
     fetchSourceChoices();
-    return () => {
-      isMounted.current = false;
-    };
   }, [fetchSourceChoices]);
 
   const handleDelete = async () => {
diff --git a/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx b/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx
index af64e8afc5..9a58be1055 100644
--- a/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx
+++ b/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx
@@ -47,6 +47,7 @@ import {
   removeParams,
   getQSConfig,
 } from '../../../util/qs';
+import useIsMounted from '../../../util/useIsMounted';
 
 const QS_CONFIG = getQSConfig('job_output', {
   order_by: 'start_line',
@@ -275,10 +276,10 @@ const cache = new CellMeasurerCache({
 function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
   const location = useLocation();
   const listRef = useRef(null);
-  const isMounted = useRef(false);
   const previousWidth = useRef(0);
   const jobSocketCounter = useRef(0);
   const interval = useRef(null);
+  const isMounted = useIsMounted();
   const history = useHistory();
   const [contentError, setContentError] = useState(null);
   const [cssMap, setCssMap] = useState({});
@@ -292,7 +293,6 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
   const [results, setResults] = useState({});
 
   useEffect(() => {
-    isMounted.current = true;
     loadJobEvents();
 
     if (isJobRunning(job.status)) {
@@ -319,7 +319,6 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
         ws.close();
       }
       clearInterval(interval.current);
-      isMounted.current = false;
     };
   }, [location.search]); // eslint-disable-line react-hooks/exhaustive-deps
 
diff --git a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx
index 1706e5d6da..b831c02c61 100644
--- a/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx
+++ b/awx/ui_next/src/screens/Template/shared/JobTemplateForm.jsx
@@ -45,6 +45,7 @@ import { JobTemplatesAPI } from '../../../api';
 import LabelSelect from './LabelSelect';
 import PlaybookSelect from './PlaybookSelect';
 import WebhookSubForm from './WebhookSubForm';
+import useIsMounted from '../../../util/useIsMounted';
 
 const { origin } = document.location;
 
@@ -67,6 +68,7 @@ function JobTemplateForm({
   const [enableWebhooks, setEnableWebhooks] = useState(
     Boolean(template.webhook_service)
   );
+  const isMounted = useIsMounted();
 
   const [askInventoryOnLaunchField] = useField('ask_inventory_on_launch');
   const [jobTypeField, jobTypeMeta, jobTypeHelpers] = useField({
@@ -119,8 +121,11 @@ function JobTemplateForm({
         return;
       }
       const { data } = await JobTemplatesAPI.readInstanceGroups(template.id);
-      setFieldValue('initialInstanceGroups', data.results);
-      setFieldValue('instanceGroups', [...data.results]);
+      if (isMounted.current) {
+        setFieldValue('initialInstanceGroups', data.results);
+        setFieldValue('instanceGroups', [...data.results]);
+      }
+      // eslint-disable-next-line react-hooks/exhaustive-deps
     }, [setFieldValue, template])
   );
 
diff --git a/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx b/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx
index 74a48924eb..e6e4ee8e48 100644
--- a/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx
+++ b/awx/ui_next/src/screens/Template/shared/LabelSelect.jsx
@@ -4,8 +4,12 @@ import { Select, SelectOption, SelectVariant } from '@patternfly/react-core';
 import { t } from '@lingui/macro';
 import { LabelsAPI } from '../../../api';
 import { useSyncedSelectValue } from '../../../components/MultiSelect';
+import useIsMounted from '../../../util/useIsMounted';
 
-async function loadLabelOptions(setLabels, onError) {
+async function loadLabelOptions(setLabels, onError, isMounted) {
+  if (!isMounted.current) {
+    return;
+  }
   let labels;
   try {
     const { data } = await LabelsAPI.read({
@@ -32,11 +36,12 @@ async function loadLabelOptions(setLabels, onError) {
 
 function LabelSelect({ value, placeholder, onChange, onError, createText }) {
   const [isLoading, setIsLoading] = useState(true);
+  const [isExpanded, setIsExpanded] = useState(false);
+  const isMounted = useIsMounted();
   const { selections, onSelect, options, setOptions } = useSyncedSelectValue(
     value,
     onChange
   );
-  const [isExpanded, setIsExpanded] = useState(false);
 
   const toggleExpanded = toggleValue => {
     setIsExpanded(toggleValue);
@@ -44,7 +49,10 @@ function LabelSelect({ value, placeholder, onChange, onError, createText }) {
 
   useEffect(() => {
     (async () => {
-      await loadLabelOptions(setOptions, onError);
+      await loadLabelOptions(setOptions, onError, isMounted);
+      if (!isMounted.current) {
+        return;
+      }
       setIsLoading(false);
     })();
     /* eslint-disable-next-line react-hooks/exhaustive-deps */
diff --git a/awx/ui_next/src/util/useIsMounted.js b/awx/ui_next/src/util/useIsMounted.js
new file mode 100644
index 0000000000..9a9dea82cd
--- /dev/null
+++ b/awx/ui_next/src/util/useIsMounted.js
@@ -0,0 +1,12 @@
+import { useEffect, useRef } from 'react';
+
+export default function useIsMounted() {
+  const isMounted = useRef(null);
+  useEffect(() => {
+    isMounted.current = true;
+    return () => {
+      isMounted.current = false;
+    };
+  });
+  return isMounted;
+}
diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js
index 8dc7e9a282..c3328ce7c7 100644
--- a/awx/ui_next/src/util/useRequest.js
+++ b/awx/ui_next/src/util/useRequest.js
@@ -1,10 +1,11 @@
-import { useEffect, useState, useRef, useCallback } from 'react';
+import { useEffect, useState, useCallback } from 'react';
 import { useLocation, useHistory } from 'react-router-dom';
 import {
   parseQueryString,
   replaceParams,
   encodeNonDefaultQueryString,
 } from './qs';
+import useIsMounted from './useIsMounted';
 
 /*
  * The useRequest hook accepts a request function and returns an object with
@@ -22,14 +23,7 @@ export default function useRequest(makeRequest, initialValue) {
   const [result, setResult] = useState(initialValue);
   const [error, setError] = useState(null);
   const [isLoading, setIsLoading] = useState(false);
-  const isMounted = useRef(null);
-
-  useEffect(() => {
-    isMounted.current = true;
-    return () => {
-      isMounted.current = false;
-    };
-  }, []);
+  const isMounted = useIsMounted();
 
   return {
     result,