From e510cb628db66d92cb18e2b8db3a66e5773f8e1d Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Fri, 13 Dec 2019 16:44:47 +0100 Subject: [PATCH 1/4] Add useIsMountedRef hook --- packages/app-accounts/src/translate.ts | 8 +-- packages/app-staking/src/index.tsx | 40 ++++++------ packages/app-staking/src/translate.ts | 6 +- packages/react-api/src/Api.tsx | 7 +-- .../react-components/src/Status/Context.ts | 8 +-- packages/react-components/src/types.ts | 1 + packages/react-hooks/src/index.ts | 1 + packages/react-hooks/src/useAccounts.tsx | 11 +++- packages/react-hooks/src/useAddresses.tsx | 11 +++- packages/react-hooks/src/useCall.tsx | 62 +++++++++---------- packages/react-hooks/src/useDebounce.tsx | 5 +- packages/react-hooks/src/useIsMountedRef.tsx | 23 +++++++ 12 files changed, 107 insertions(+), 76 deletions(-) create mode 100644 packages/react-hooks/src/useIsMountedRef.tsx diff --git a/packages/app-accounts/src/translate.ts b/packages/app-accounts/src/translate.ts index 6e0e4b756b36..d434e77edff4 100644 --- a/packages/app-accounts/src/translate.ts +++ b/packages/app-accounts/src/translate.ts @@ -2,14 +2,10 @@ // This software may be modified and distributed under the terms // of the Apache-2.0 license. See the LICENSE file for details. -import { - withTranslation, - useTranslation as _useTranslation, - UseTranslationResponse -} from 'react-i18next'; +import { useTranslation as useTranslationBase, UseTranslationResponse, withTranslation } from 'react-i18next'; export function useTranslation (): UseTranslationResponse { - return _useTranslation('app-accounts'); + return useTranslationBase('app-accounts'); } export default withTranslation(['app-accounts']); diff --git a/packages/app-staking/src/index.tsx b/packages/app-staking/src/index.tsx index 53e16ee6a97d..79de9fffdcdd 100644 --- a/packages/app-staking/src/index.tsx +++ b/packages/app-staking/src/index.tsx @@ -3,7 +3,7 @@ // of the Apache-2.0 license. See the LICENSE file for details. import { DerivedHeartbeats, DerivedStakingOverview } from '@polkadot/api-derive/types'; -import { AppProps, I18nProps } from '@polkadot/react-components/types'; +import { AppProps as Props } from '@polkadot/react-components/types'; import { AccountId } from '@polkadot/types/interfaces'; import React, { useEffect, useState } from 'react'; @@ -22,12 +22,9 @@ import Summary from './Overview/Summary'; import Query from './Query'; import Targets from './Targets'; import { MAX_SESSIONS } from './constants'; -import translate from './translate'; +import { useTranslation } from './translate'; import useSessionRewards from './useSessionRewards'; -interface Props extends AppProps, I18nProps { -} - const EMPY_ACCOUNTS: string[] = []; const EMPTY_ALL: [string[], string[]] = [EMPY_ACCOUNTS, EMPY_ACCOUNTS]; @@ -40,7 +37,8 @@ function transformStakingControllers ([stashes, controllers]: [AccountId[], Opti ]; } -function StakingApp ({ basePath, className, t }: Props): React.ReactElement { +function StakingApp ({ basePath, className }: Props): React.ReactElement { + const { t } = useTranslation(); const { api, isSubstrateV2 } = useApi(); const { hasAccounts } = useAccounts(); const { pathname } = useLocation(); @@ -135,22 +133,20 @@ function StakingApp ({ basePath, className, t }: Props): React.ReactElement = { txqueue: [] as QueueTx[] }; -const Context: React.Context = React.createContext(defaultState as QueueProps); -const QueueConsumer: React.Consumer = Context.Consumer; -const QueueProvider: React.Provider = Context.Provider; +const StatusContext: React.Context = React.createContext(defaultState as QueueProps); +const QueueConsumer: React.Consumer = StatusContext.Consumer; +const QueueProvider: React.Provider = StatusContext.Provider; -export default Context; +export default StatusContext; export { QueueConsumer, diff --git a/packages/react-components/src/types.ts b/packages/react-components/src/types.ts index 26e8b7615705..f61eebd2d12b 100644 --- a/packages/react-components/src/types.ts +++ b/packages/react-components/src/types.ts @@ -21,6 +21,7 @@ export interface BareProps { export interface AppProps { basePath: string; + className?: string; onStatusChange: (status: ActionStatus) => void; } diff --git a/packages/react-hooks/src/index.ts b/packages/react-hooks/src/index.ts index d7a8dc652e92..f1d5804d695d 100644 --- a/packages/react-hooks/src/index.ts +++ b/packages/react-hooks/src/index.ts @@ -10,6 +10,7 @@ export { default as useCall } from './useCall'; export { default as useForm } from './useForm'; export { default as useDebounce } from './useDebounce'; export { default as useFavorites } from './useFavorites'; +export { default as useIsMountedRef } from './useIsMountedRef'; export { default as usePassword } from './usePassword'; export { default as useToggle } from './useToggle'; export { default as useTx } from './useTx'; diff --git a/packages/react-hooks/src/useAccounts.tsx b/packages/react-hooks/src/useAccounts.tsx index 87181ddb1bb0..6a0f834b2887 100644 --- a/packages/react-hooks/src/useAccounts.tsx +++ b/packages/react-hooks/src/useAccounts.tsx @@ -5,20 +5,25 @@ import { useEffect, useState } from 'react'; import accountObservable from '@polkadot/ui-keyring/observable/accounts'; +import useIsMountedRef from './useIsMountedRef'; + interface UseAccounts { allAccounts: string[]; hasAccounts: boolean; } export default function useAccounts (): UseAccounts { + const mounted = useIsMountedRef(); const [state, setState] = useState({ allAccounts: [], hasAccounts: false }); useEffect((): () => void => { const subscription = accountObservable.subject.subscribe((accounts): void => { - const allAccounts = accounts ? Object.keys(accounts) : []; - const hasAccounts = allAccounts.length !== 0; + if (mounted.current) { + const allAccounts = accounts ? Object.keys(accounts) : []; + const hasAccounts = allAccounts.length !== 0; - setState({ allAccounts, hasAccounts }); + setState({ allAccounts, hasAccounts }); + } }); return (): void => { diff --git a/packages/react-hooks/src/useAddresses.tsx b/packages/react-hooks/src/useAddresses.tsx index 8b8a89ffdcf8..299dbbfb8fa3 100644 --- a/packages/react-hooks/src/useAddresses.tsx +++ b/packages/react-hooks/src/useAddresses.tsx @@ -5,20 +5,25 @@ import { useEffect, useState } from 'react'; import addressObservable from '@polkadot/ui-keyring/observable/addresses'; +import useIsMountedRef from './useIsMountedRef'; + interface UseAccounts { allAddresses: string[]; hasAddresses: boolean; } export default function useAccounts (): UseAccounts { + const mounted = useIsMountedRef(); const [state, setState] = useState({ allAddresses: [], hasAddresses: false }); useEffect((): () => void => { const subscription = addressObservable.subject.subscribe((addresses): void => { - const allAddresses = addresses ? Object.keys(addresses) : []; - const hasAddresses = allAddresses.length !== 0; + if (mounted.current) { + const allAddresses = addresses ? Object.keys(addresses) : []; + const hasAddresses = allAddresses.length !== 0; - setState({ allAddresses, hasAddresses }); + setState({ allAddresses, hasAddresses }); + } }); return (): void => { diff --git a/packages/react-hooks/src/useCall.tsx b/packages/react-hooks/src/useCall.tsx index ac74008edaf0..fafd07a2805b 100644 --- a/packages/react-hooks/src/useCall.tsx +++ b/packages/react-hooks/src/useCall.tsx @@ -8,6 +8,8 @@ import { CallOptions, CallParam, CallParams } from './types'; import { useEffect, useRef, useState } from 'react'; import { isNull, isUndefined } from '@polkadot/util'; +import useIsMountedRef, { MountedRef } from './useIsMountedRef'; + interface TrackFnCallback { (value: Codec): void; } @@ -29,7 +31,6 @@ interface TrackFn { interface Tracker { isActive: boolean; count: number; - paramCount: number; serialized: string | null; subscriber: TrackFnResult | null; } @@ -64,29 +65,31 @@ function unsubscribe (tracker: TrackerRef): void { } // subscribe, tyring to play nice with the browser threads -function subscribe (tracker: TrackerRef, fn: TrackFn | undefined, params: CallParams, setValue: (value: T) => void, { isSingle, transform = transformIdentity }: CallOptions): void { +function subscribe (mounted: MountedRef, tracker: TrackerRef, fn: TrackFn | undefined, params: CallParams, setValue: (value: T) => void, { isSingle, transform = transformIdentity }: CallOptions): void { const validParams = params.filter((p): boolean => !isUndefined(p)); unsubscribe(tracker); setTimeout((): void => { - if (fn && (!fn.meta || !fn.meta.type?.isDoubleMap || validParams.length === 2)) { - // swap to acive mode and reset our count - tracker.current.isActive = true; - tracker.current.count = 0; - - // eslint-disable-next-line @typescript-eslint/ban-ts-ignore - // @ts-ignore We tried to get the typings right, close but no cigar... - tracker.current.subscriber = fn(...params, (value: any): void => { - // when we don't have an active sub, or single-shot, ignore (we use the isActive flag here - // since .subscriber may not be set on immeditae callback) - if (tracker.current.isActive && (!isSingle || !tracker.current.count)) { - tracker.current.count++; - setValue(transform(value)); - } - }); - } else { - tracker.current.subscriber = null; + if (mounted.current) { + if (fn && (!fn.meta || !fn.meta.type?.isDoubleMap || validParams.length === 2)) { + // swap to acive mode and reset our count + tracker.current.isActive = true; + tracker.current.count = 0; + + // eslint-disable-next-line @typescript-eslint/ban-ts-ignore + // @ts-ignore We tried to get the typings right, close but no cigar... + tracker.current.subscriber = fn(...params, (value: any): void => { + // when we don't have an active sub, or single-shot, ignore (we use the isActive flag here + // since .subscriber may not be set on immeditae callback) + if (mounted.current && tracker.current.isActive && (!isSingle || !tracker.current.count)) { + tracker.current.count++; + setValue(transform(value)); + } + }); + } else { + tracker.current.subscriber = null; + } } }, 0); } @@ -96,8 +99,9 @@ function subscribe (tracker: TrackerRef, fn: TrackFn | undefined, params: Ca // - has a callback to set the value // FIXME The typings here need some serious TLC export default function useCall (fn: TrackFn | undefined, params: CallParams, options: CallOptions = {}): T | undefined { + const mounted = useIsMountedRef(); + const tracker = useRef({ isActive: false, count: 0, serialized: null, subscriber: null }); const [value, setValue] = useState(options.defaultValue); - const tracker = useRef({ isActive: false, count: 0, paramCount: -1, serialized: null, subscriber: null }); // initial effect, we need an unsubscription useEffect((): () => void => { @@ -108,20 +112,14 @@ export default function useCall (fn: TrackFn | undefined, params: CallParams // on changes, re-subscribe useEffect((): void => { - // check if we have a function - if (fn) { - const validParams = params.filter((param): boolean => !isUndefined(param)); - - // in the case on unmounting, the params may go empty, cater for this, don't trigger - if (validParams.length >= tracker.current.paramCount) { - const [serialized, mappedParams] = extractParams(fn, params, options.paramMap || transformIdentity); + // check if we have a function & that we are mounted + if (mounted.current && fn) { + const [serialized, mappedParams] = extractParams(fn, params, options.paramMap || transformIdentity); - if (mappedParams && serialized !== tracker.current.serialized) { - tracker.current.paramCount = validParams.length; - tracker.current.serialized = serialized; + if (mappedParams && serialized !== tracker.current.serialized) { + tracker.current.serialized = serialized; - subscribe(tracker, fn, mappedParams, setValue, options); - } + subscribe(mounted, tracker, fn, mappedParams, setValue, options); } } }, [fn, params]); diff --git a/packages/react-hooks/src/useDebounce.tsx b/packages/react-hooks/src/useDebounce.tsx index ba5904f37d48..da8ef48b5679 100644 --- a/packages/react-hooks/src/useDebounce.tsx +++ b/packages/react-hooks/src/useDebounce.tsx @@ -4,13 +4,16 @@ import { useState, useEffect } from 'react'; +import useIsMountedRef from './useIsMountedRef'; + // Debounces inputs export default function useDebounce (value: T, delay = 250): T { + const mounted = useIsMountedRef(); const [debouncedValue, setDebouncedValue] = useState(value); useEffect((): () => void => { const handler = setTimeout(() => { - setDebouncedValue(value); + mounted.current && setDebouncedValue(value); }, delay); // each time it renders, it clears diff --git a/packages/react-hooks/src/useIsMountedRef.tsx b/packages/react-hooks/src/useIsMountedRef.tsx new file mode 100644 index 000000000000..b4ba58dfb6f8 --- /dev/null +++ b/packages/react-hooks/src/useIsMountedRef.tsx @@ -0,0 +1,23 @@ +// Copyright 2017-2019 @polkadot/react-hooks authors & contributors +// This software may be modified and distributed under the terms +// of the Apache-2.0 license. See the LICENSE file for details. + +import { useEffect, useRef } from 'react'; + +export interface MountedRef { + current: boolean; +} + +export default function useIsMountedRef (): MountedRef { + const isMounted = useRef(false); + + useEffect((): () => void => { + isMounted.current = true; + + return (): void => { + isMounted.current = false; + }; + }, []); + + return isMounted; +} From 934b8b2e387c8a72cf38c0b10bd0be4a8a395bfe Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Fri, 13 Dec 2019 16:52:19 +0100 Subject: [PATCH 2/4] Use mounted ref on async operations (staking) --- packages/app-staking/src/useBlockCounts.tsx | 9 ++++++--- packages/app-staking/src/useSessionRewards.tsx | 11 +++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/app-staking/src/useBlockCounts.tsx b/packages/app-staking/src/useBlockCounts.tsx index 273ff2d7b4cb..b7700ca03efe 100644 --- a/packages/app-staking/src/useBlockCounts.tsx +++ b/packages/app-staking/src/useBlockCounts.tsx @@ -6,18 +6,19 @@ import { SessionIndex } from '@polkadot/types/interfaces'; import { SessionRewards } from './types'; import { useEffect, useState } from 'react'; -import { useApi, useCall } from '@polkadot/react-hooks'; +import { useApi, useCall, useIsMountedRef } from '@polkadot/react-hooks'; import { u32 } from '@polkadot/types'; export default function useBlockCounts (accountId: string, sessionRewards: SessionRewards[]): u32[] { const { api } = useApi(); + const mounted = useIsMountedRef(); const [counts, setCounts] = useState([]); const [historic, setHistoric] = useState([]); const sessionIndex = useCall(api.query.session.currentIndex, []); const current = useCall(api.query.imOnline?.authoredBlocks, [sessionIndex, accountId]); useEffect((): void => { - if (api.query.imOnline?.authoredBlocks && sessionRewards && sessionRewards.length) { + if (api.query.imOnline?.authoredBlocks && sessionRewards?.length) { const filtered = sessionRewards.filter(({ sessionIndex }): boolean => sessionIndex.gtn(0)); if (filtered.length) { @@ -25,7 +26,9 @@ export default function useBlockCounts (accountId: string, sessionRewards: Sessi .all(filtered.map(({ parentHash, sessionIndex }): Promise => api.query.imOnline.authoredBlocks.at(parentHash, sessionIndex.subn(1), accountId) as Promise )) - .then(setHistoric); + .then((historic): void => { + mounted.current && setHistoric(historic); + }); } } }, [accountId, sessionRewards]); diff --git a/packages/app-staking/src/useSessionRewards.tsx b/packages/app-staking/src/useSessionRewards.tsx index f2ef3f753673..4709317bafc5 100644 --- a/packages/app-staking/src/useSessionRewards.tsx +++ b/packages/app-staking/src/useSessionRewards.tsx @@ -9,7 +9,7 @@ import BN from 'bn.js'; import { useEffect, useState } from 'react'; import { ApiPromise } from '@polkadot/api'; import { registry } from '@polkadot/react-api'; -import { useApi, useCacheKey } from '@polkadot/react-hooks'; +import { useApi, useCacheKey, useIsMountedRef } from '@polkadot/react-hooks'; import { createType } from '@polkadot/types'; import { bnMax, u8aToU8a } from '@polkadot/util'; @@ -141,6 +141,7 @@ async function loadSome (api: ApiPromise, fromHash: Hash, toHash: Hash): Promise export default function useSessionRewards (maxSessions: number): SessionRewards[] { const { api } = useApi(); + const mounted = useIsMountedRef(); const [getCache, setCache] = useCacheKey('hooks:sessionSlashes'); const [filtered, setFiltered] = useState([]); @@ -169,12 +170,14 @@ export default function useSessionRewards (maxSessions: number): SessionRewards[ toNumber = fromNumber; fromNumber = bnMax(toNumber.subn(MAX_BLOCKS), new BN(1)); - setCache(toJSON(workQueue, maxSessionsStore)); - setFiltered(workQueue.slice(-maxSessions)); + if (mounted.current) { + setCache(toJSON(workQueue, maxSessionsStore)); + setFiltered(workQueue.slice(-maxSessions)); + } const lastNumber = workQueue[workQueue.length - 1]?.blockNumber; - if (!lastNumber || fromNumber.eqn(1) || ((workQueue.length >= maxSessionsStore) && fromNumber.lt(savedNumber || lastNumber))) { + if (!mounted.current || !lastNumber || fromNumber.eqn(1) || ((workQueue.length >= maxSessionsStore) && fromNumber.lt(savedNumber || lastNumber))) { break; } } From 46becdf6815f87d1f2c442d4f3ee05b263e66828 Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Fri, 13 Dec 2019 17:11:45 +0100 Subject: [PATCH 3/4] Rework historic caching --- .../app-staking/src/useSessionRewards.tsx | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/app-staking/src/useSessionRewards.tsx b/packages/app-staking/src/useSessionRewards.tsx index 4709317bafc5..bcba7320df66 100644 --- a/packages/app-staking/src/useSessionRewards.tsx +++ b/packages/app-staking/src/useSessionRewards.tsx @@ -12,6 +12,7 @@ import { registry } from '@polkadot/react-api'; import { useApi, useCacheKey, useIsMountedRef } from '@polkadot/react-hooks'; import { createType } from '@polkadot/types'; import { bnMax, u8aToU8a } from '@polkadot/util'; +import { recover } from 'secp256k1/elliptic'; interface SerializedSlash { accountId: string; @@ -32,8 +33,8 @@ interface Serialized { const MAX_BLOCKS = 2500; function fromJSON (sessions: Serialized[]): SessionRewards[] { - let hasSome = false; - let keepAll = true; + let hasData = false; + let keepAll = false; return sessions .map(({ blockHash, blockNumber, isEventsEmpty, parentHash, reward, sessionIndex, slashes, treasury }): SessionRewards => ({ @@ -50,17 +51,22 @@ function fromJSON (sessions: Serialized[]): SessionRewards[] { treasury: createType(registry, 'Balance', treasury) })) .filter(({ parentHash }): boolean => !parentHash.isEmpty) - .filter(({ isEventsEmpty }): boolean => { - if (!isEventsEmpty) { - // we first see if we have some data up to this point (we may not, i.e. non-archive) - hasSome = true; - } else if (hasSome) { - // if data is followed by empty, drop everything from here on - keepAll = false; + .reverse() + // we drop everything before the second non-empty + .filter(({ isEventsEmpty }, index): boolean => { + if (index !== 0) { + if (!isEventsEmpty) { + // we first see if we have some data up to this point (we may not, i.e. non-archive) + hasData = true; + } else if (hasData) { + // if data is followed by empty, drop everything from here on + keepAll = true; + } } return keepAll; - }); + }) + .reverse(); } function toJSON (sessions: SessionRewards[], maxSessions: number): Serialized[] { From 43acaec54dd12c56cdae819ca043a0eb611a09ea Mon Sep 17 00:00:00 2001 From: Jaco Greeff Date: Fri, 13 Dec 2019 17:13:20 +0100 Subject: [PATCH 4/4] linting --- packages/app-staking/src/useSessionRewards.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/app-staking/src/useSessionRewards.tsx b/packages/app-staking/src/useSessionRewards.tsx index bcba7320df66..4e798b8b6857 100644 --- a/packages/app-staking/src/useSessionRewards.tsx +++ b/packages/app-staking/src/useSessionRewards.tsx @@ -12,7 +12,6 @@ import { registry } from '@polkadot/react-api'; import { useApi, useCacheKey, useIsMountedRef } from '@polkadot/react-hooks'; import { createType } from '@polkadot/types'; import { bnMax, u8aToU8a } from '@polkadot/util'; -import { recover } from 'secp256k1/elliptic'; interface SerializedSlash { accountId: string;