Fixes test warnings where state updates were being triggered after component unmounts

This commit is contained in:
mabashian
2021-04-30 15:31:36 -04:00
parent f63312c811
commit 5b71681494
9 changed files with 59 additions and 54 deletions

View File

@@ -1,11 +1,5 @@
import 'styled-components/macro'; import 'styled-components/macro';
import React, { import React, { Fragment, useState, useCallback, useEffect } from 'react';
Fragment,
useState,
useCallback,
useEffect,
useRef,
} from 'react';
import { withRouter } from 'react-router-dom'; import { withRouter } from 'react-router-dom';
import PropTypes from 'prop-types'; import PropTypes from 'prop-types';
import { withI18n } from '@lingui/react'; import { withI18n } from '@lingui/react';
@@ -18,6 +12,7 @@ import OptionsList from '../OptionsList';
import useRequest from '../../util/useRequest'; import useRequest from '../../util/useRequest';
import { getQSConfig, parseQueryString } from '../../util/qs'; import { getQSConfig, parseQueryString } from '../../util/qs';
import Lookup from './Lookup'; import Lookup from './Lookup';
import useIsMounted from '../../util/useIsMounted';
const QS_CONFIG = getQSConfig('credentials', { const QS_CONFIG = getQSConfig('credentials', {
page: 1, page: 1,
@@ -32,9 +27,9 @@ async function loadCredentials(params, selectedCredentialTypeId) {
} }
function MultiCredentialsLookup(props) { function MultiCredentialsLookup(props) {
const isMounted = useRef(null);
const { value, onChange, onError, history, i18n } = props; const { value, onChange, onError, history, i18n } = props;
const [selectedType, setSelectedType] = useState(null); const [selectedType, setSelectedType] = useState(null);
const isMounted = useIsMounted();
const { const {
result: credentialTypes, result: credentialTypes,
@@ -44,22 +39,18 @@ function MultiCredentialsLookup(props) {
} = useRequest( } = useRequest(
useCallback(async () => { useCallback(async () => {
const types = await CredentialTypesAPI.loadAllTypes(); const types = await CredentialTypesAPI.loadAllTypes();
if (!isMounted.current) {
return;
}
const match = types.find(type => type.kind === 'ssh') || types[0]; const match = types.find(type => type.kind === 'ssh') || types[0];
if (isMounted.current) {
setSelectedType(match); setSelectedType(match);
}
return types; return types;
/* eslint-disable-next-line react-hooks/exhaustive-deps */
}, []), }, []),
[] []
); );
useEffect(() => { useEffect(() => {
isMounted.current = true;
fetchTypes(); fetchTypes();
return () => {
isMounted.current = false;
};
}, [fetchTypes]); }, [fetchTypes]);
const { const {
@@ -86,10 +77,6 @@ function MultiCredentialsLookup(props) {
CredentialsAPI.readOptions(), CredentialsAPI.readOptions(),
]); ]);
if (!isMounted.current) {
return;
}
results.map(result => { results.map(result => {
if (result.kind === 'vault' && result.inputs?.vault_id) { if (result.kind === 'vault' && result.inputs?.vault_id) {
result.label = `${result.name} | ${result.inputs.vault_id}`; result.label = `${result.name} | ${result.inputs.vault_id}`;
@@ -119,11 +106,7 @@ function MultiCredentialsLookup(props) {
); );
useEffect(() => { useEffect(() => {
isMounted.current = true;
fetchCredentials(); fetchCredentials();
return () => {
isMounted.current = false;
};
}, [fetchCredentials]); }, [fetchCredentials]);
useEffect(() => { useEffect(() => {

View File

@@ -1,4 +1,5 @@
import { useState, useEffect } from 'react'; import { useState, useEffect } from 'react';
import useIsMounted from '../../util/useIsMounted';
/* /*
Hook for using PatternFly's <Select> component when a pre-existing value 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) { export default function useSyncedSelectValue(value, onChange) {
const [options, setOptions] = useState([]); const [options, setOptions] = useState([]);
const [selections, setSelections] = useState([]); const [selections, setSelections] = useState([]);
const isMounted = useIsMounted();
useEffect(() => { useEffect(() => {
if (!isMounted.current) {
return;
}
const newOptions = []; const newOptions = [];
if (value !== selections && options.length) { if (value !== selections && options.length) {
const syncedValue = value.map(item => { const syncedValue = value.map(item => {
@@ -41,7 +46,11 @@ export default function useSyncedSelectValue(value, onChange) {
selections: options.length ? addToStringToObjects(selections) : [], selections: options.length ? addToStringToObjects(selections) : [],
onSelect, onSelect,
options, options,
setOptions: newOpts => setOptions(addToStringToObjects(newOpts)), setOptions: newOpts => {
if (isMounted.current) {
setOptions(addToStringToObjects(newOpts));
}
},
}; };
} }

View File

@@ -1,4 +1,4 @@
import React, { useState, useEffect, useRef } from 'react'; import React, { useState, useEffect } from 'react';
import { useHistory } from 'react-router-dom'; import { useHistory } from 'react-router-dom';
import { object } from 'prop-types'; import { object } from 'prop-types';
@@ -7,6 +7,7 @@ import { InventoriesAPI, CredentialTypesAPI } from '../../../api';
import ContentLoading from '../../../components/ContentLoading'; import ContentLoading from '../../../components/ContentLoading';
import InventoryForm from '../shared/InventoryForm'; import InventoryForm from '../shared/InventoryForm';
import { getAddedAndRemoved } from '../../../util/lists'; import { getAddedAndRemoved } from '../../../util/lists';
import useIsMounted from '../../../util/useIsMounted';
function InventoryEdit({ inventory }) { function InventoryEdit({ inventory }) {
const [error, setError] = useState(null); const [error, setError] = useState(null);
@@ -14,10 +15,9 @@ function InventoryEdit({ inventory }) {
const [contentLoading, setContentLoading] = useState(true); const [contentLoading, setContentLoading] = useState(true);
const [credentialTypeId, setCredentialTypeId] = useState(null); const [credentialTypeId, setCredentialTypeId] = useState(null);
const history = useHistory(); const history = useHistory();
const isMounted = useRef(null); const isMounted = useIsMounted();
useEffect(() => { useEffect(() => {
isMounted.current = true;
const loadData = async () => { const loadData = async () => {
try { try {
const [ const [
@@ -47,9 +47,7 @@ function InventoryEdit({ inventory }) {
} }
}; };
loadData(); loadData();
return () => { /* eslint-disable-next-line react-hooks/exhaustive-deps */
isMounted.current = false;
};
}, [inventory.id, contentLoading, inventory, credentialTypeId]); }, [inventory.id, contentLoading, inventory, credentialTypeId]);
const handleCancel = () => { const handleCancel = () => {

View File

@@ -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 { Link, useHistory } from 'react-router-dom';
import { withI18n } from '@lingui/react'; import { withI18n } from '@lingui/react';
import { t } from '@lingui/macro'; import { t } from '@lingui/macro';
@@ -23,6 +23,7 @@ import Popover from '../../../components/Popover';
import useRequest from '../../../util/useRequest'; import useRequest from '../../../util/useRequest';
import { InventorySourcesAPI } from '../../../api'; import { InventorySourcesAPI } from '../../../api';
import { relatedResourceDeleteRequests } from '../../../util/getRelatedResourceDeleteDetails'; import { relatedResourceDeleteRequests } from '../../../util/getRelatedResourceDeleteDetails';
import useIsMounted from '../../../util/useIsMounted';
function InventorySourceDetail({ inventorySource, i18n }) { function InventorySourceDetail({ inventorySource, i18n }) {
const { const {
@@ -57,7 +58,7 @@ function InventorySourceDetail({ inventorySource, i18n }) {
} = inventorySource; } = inventorySource;
const [deletionError, setDeletionError] = useState(false); const [deletionError, setDeletionError] = useState(false);
const history = useHistory(); const history = useHistory();
const isMounted = useRef(null); const isMounted = useIsMounted();
const { const {
result: sourceChoices, result: sourceChoices,
@@ -75,11 +76,7 @@ function InventorySourceDetail({ inventorySource, i18n }) {
); );
useEffect(() => { useEffect(() => {
isMounted.current = true;
fetchSourceChoices(); fetchSourceChoices();
return () => {
isMounted.current = false;
};
}, [fetchSourceChoices]); }, [fetchSourceChoices]);
const handleDelete = async () => { const handleDelete = async () => {

View File

@@ -47,6 +47,7 @@ import {
removeParams, removeParams,
getQSConfig, getQSConfig,
} from '../../../util/qs'; } from '../../../util/qs';
import useIsMounted from '../../../util/useIsMounted';
const QS_CONFIG = getQSConfig('job_output', { const QS_CONFIG = getQSConfig('job_output', {
order_by: 'start_line', order_by: 'start_line',
@@ -275,10 +276,10 @@ const cache = new CellMeasurerCache({
function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) { function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
const location = useLocation(); const location = useLocation();
const listRef = useRef(null); const listRef = useRef(null);
const isMounted = useRef(false);
const previousWidth = useRef(0); const previousWidth = useRef(0);
const jobSocketCounter = useRef(0); const jobSocketCounter = useRef(0);
const interval = useRef(null); const interval = useRef(null);
const isMounted = useIsMounted();
const history = useHistory(); const history = useHistory();
const [contentError, setContentError] = useState(null); const [contentError, setContentError] = useState(null);
const [cssMap, setCssMap] = useState({}); const [cssMap, setCssMap] = useState({});
@@ -292,7 +293,6 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
const [results, setResults] = useState({}); const [results, setResults] = useState({});
useEffect(() => { useEffect(() => {
isMounted.current = true;
loadJobEvents(); loadJobEvents();
if (isJobRunning(job.status)) { if (isJobRunning(job.status)) {
@@ -319,7 +319,6 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) {
ws.close(); ws.close();
} }
clearInterval(interval.current); clearInterval(interval.current);
isMounted.current = false;
}; };
}, [location.search]); // eslint-disable-line react-hooks/exhaustive-deps }, [location.search]); // eslint-disable-line react-hooks/exhaustive-deps

View File

@@ -45,6 +45,7 @@ import { JobTemplatesAPI } from '../../../api';
import LabelSelect from './LabelSelect'; import LabelSelect from './LabelSelect';
import PlaybookSelect from './PlaybookSelect'; import PlaybookSelect from './PlaybookSelect';
import WebhookSubForm from './WebhookSubForm'; import WebhookSubForm from './WebhookSubForm';
import useIsMounted from '../../../util/useIsMounted';
const { origin } = document.location; const { origin } = document.location;
@@ -67,6 +68,7 @@ function JobTemplateForm({
const [enableWebhooks, setEnableWebhooks] = useState( const [enableWebhooks, setEnableWebhooks] = useState(
Boolean(template.webhook_service) Boolean(template.webhook_service)
); );
const isMounted = useIsMounted();
const [askInventoryOnLaunchField] = useField('ask_inventory_on_launch'); const [askInventoryOnLaunchField] = useField('ask_inventory_on_launch');
const [jobTypeField, jobTypeMeta, jobTypeHelpers] = useField({ const [jobTypeField, jobTypeMeta, jobTypeHelpers] = useField({
@@ -119,8 +121,11 @@ function JobTemplateForm({
return; return;
} }
const { data } = await JobTemplatesAPI.readInstanceGroups(template.id); const { data } = await JobTemplatesAPI.readInstanceGroups(template.id);
if (isMounted.current) {
setFieldValue('initialInstanceGroups', data.results); setFieldValue('initialInstanceGroups', data.results);
setFieldValue('instanceGroups', [...data.results]); setFieldValue('instanceGroups', [...data.results]);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [setFieldValue, template]) }, [setFieldValue, template])
); );

View File

@@ -4,8 +4,12 @@ import { Select, SelectOption, SelectVariant } from '@patternfly/react-core';
import { t } from '@lingui/macro'; import { t } from '@lingui/macro';
import { LabelsAPI } from '../../../api'; import { LabelsAPI } from '../../../api';
import { useSyncedSelectValue } from '../../../components/MultiSelect'; 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; let labels;
try { try {
const { data } = await LabelsAPI.read({ const { data } = await LabelsAPI.read({
@@ -32,11 +36,12 @@ async function loadLabelOptions(setLabels, onError) {
function LabelSelect({ value, placeholder, onChange, onError, createText }) { function LabelSelect({ value, placeholder, onChange, onError, createText }) {
const [isLoading, setIsLoading] = useState(true); const [isLoading, setIsLoading] = useState(true);
const [isExpanded, setIsExpanded] = useState(false);
const isMounted = useIsMounted();
const { selections, onSelect, options, setOptions } = useSyncedSelectValue( const { selections, onSelect, options, setOptions } = useSyncedSelectValue(
value, value,
onChange onChange
); );
const [isExpanded, setIsExpanded] = useState(false);
const toggleExpanded = toggleValue => { const toggleExpanded = toggleValue => {
setIsExpanded(toggleValue); setIsExpanded(toggleValue);
@@ -44,7 +49,10 @@ function LabelSelect({ value, placeholder, onChange, onError, createText }) {
useEffect(() => { useEffect(() => {
(async () => { (async () => {
await loadLabelOptions(setOptions, onError); await loadLabelOptions(setOptions, onError, isMounted);
if (!isMounted.current) {
return;
}
setIsLoading(false); setIsLoading(false);
})(); })();
/* eslint-disable-next-line react-hooks/exhaustive-deps */ /* eslint-disable-next-line react-hooks/exhaustive-deps */

View File

@@ -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;
}

View File

@@ -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 { useLocation, useHistory } from 'react-router-dom';
import { import {
parseQueryString, parseQueryString,
replaceParams, replaceParams,
encodeNonDefaultQueryString, encodeNonDefaultQueryString,
} from './qs'; } from './qs';
import useIsMounted from './useIsMounted';
/* /*
* The useRequest hook accepts a request function and returns an object with * 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 [result, setResult] = useState(initialValue);
const [error, setError] = useState(null); const [error, setError] = useState(null);
const [isLoading, setIsLoading] = useState(false); const [isLoading, setIsLoading] = useState(false);
const isMounted = useRef(null); const isMounted = useIsMounted();
useEffect(() => {
isMounted.current = true;
return () => {
isMounted.current = false;
};
}, []);
return { return {
result, result,