From 2d10b3aa2dca25972165e500815ac331fbcc9bca Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 21 Dec 2023 12:18:46 +0200 Subject: [PATCH 1/7] fix(clerk-js): Fetching custom role in OrganizationSwitcher In order to do this efficiently and to avoid requesting everytime the switcher opens, a simple cache inside useFetch was implemented. --- .../OrganizationProfile/RemoveDomainForm.tsx | 4 +- .../VerifiedDomainForm.tsx | 8 +- .../OrganizationProfile/VerifyDomainForm.tsx | 4 +- .../src/ui/elements/OrganizationPreview.tsx | 11 +- packages/clerk-js/src/ui/hooks/useFetch.ts | 105 +++++++++++++++--- .../clerk-js/src/ui/hooks/useFetchRoles.ts | 4 +- 6 files changed, 107 insertions(+), 29 deletions(-) diff --git a/packages/clerk-js/src/ui/components/OrganizationProfile/RemoveDomainForm.tsx b/packages/clerk-js/src/ui/components/OrganizationProfile/RemoveDomainForm.tsx index 9aabea7319b..16aefca0f54 100644 --- a/packages/clerk-js/src/ui/components/OrganizationProfile/RemoveDomainForm.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationProfile/RemoveDomainForm.tsx @@ -19,7 +19,7 @@ export const RemoveDomainForm = (props: RemoveDomainFormProps) => { const { domainId: id, onSuccess, onReset } = props; const ref = React.useRef(); - const { data: domain, status: domainStatus } = useFetch( + const { data: domain, isLoading: domainIsLoading } = useFetch( organization?.getDomain, { domainId: id, @@ -41,7 +41,7 @@ export const RemoveDomainForm = (props: RemoveDomainFormProps) => { return null; } - if (domainStatus.isLoading || !domain) { + if (domainIsLoading || !domain) { return ( diff --git a/packages/clerk-js/src/ui/components/OrganizationProfile/VerifyDomainForm.tsx b/packages/clerk-js/src/ui/components/OrganizationProfile/VerifyDomainForm.tsx index c8b4f9067b5..c71fbb340ea 100644 --- a/packages/clerk-js/src/ui/components/OrganizationProfile/VerifyDomainForm.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationProfile/VerifyDomainForm.tsx @@ -29,7 +29,7 @@ export const VerifyDomainForm = withCardStateProvider((props: VerifyDomainFormPr const { organizationSettings } = useEnvironment(); const { organization } = useOrganization(); - const { data: domain, status: domainStatus } = useFetch(organization?.getDomain, { + const { data: domain, isLoading: domainIsLoading } = useFetch(organization?.getDomain, { domainId: id, }); const title = localizationKeys('organizationProfile.verifyDomainPage.title'); @@ -108,7 +108,7 @@ export const VerifyDomainForm = withCardStateProvider((props: VerifyDomainFormPr }); }; - if (domainStatus.isLoading || !domain) { + if (domainIsLoading || !domain) { return ( , 'elementId'> & { @@ -34,7 +34,12 @@ export const OrganizationPreview = (props: OrganizationPreviewProps) => { elementId, ...rest } = props; - const role = user?.organizationMemberships.find(membership => membership.organization.id === organization.id)?.role; + + const { localizeCustomRole } = useLocalizeCustomRoles(); + const membership = user?.organizationMemberships.find(membership => membership.organization.id === organization.id); + + const { options } = useFetchRoles(); + const unlocalizedRoleLabel = options?.find(a => a.value === membership?.role)?.label; const mainTextSize = mainIdentifierVariant || ({ xs: 'subtitle', sm: 'caption', md: 'subtitle', lg: 'h1' } as const)[size]; @@ -84,7 +89,7 @@ export const OrganizationPreview = (props: OrganizationPreviewProps) => { diff --git a/packages/clerk-js/src/ui/hooks/useFetch.ts b/packages/clerk-js/src/ui/hooks/useFetch.ts index 87c81d43477..bf0fc3431e0 100644 --- a/packages/clerk-js/src/ui/hooks/useFetch.ts +++ b/packages/clerk-js/src/ui/hooks/useFetch.ts @@ -1,7 +1,52 @@ -import { useEffect, useRef } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; -import { useLoadingStatus } from './useLoadingStatus'; -import { useSafeState } from './useSafeState'; +export type State = { + data?: Data; + error?: Error; + isLoading?: boolean; + cachedAt?: number; +}; + +export interface Cache { + keys(): IterableIterator; + + get(key: string): State | undefined; + + set(key: string, value: State): void; + + delete(key: string): void; +} + +const map = new WeakMap(); +const subscribers = new Set<() => void>(); +const useCache = ( + param: any, +): { + get: () => State | undefined; + set: (state: State) => void; + subscribe: (callback: () => void) => () => void; +} => { + // const subscribers = useRef(new Set<() => void>()); + + const get = useCallback(() => map.get(param), [param]); + const set = useCallback( + (data: State) => { + map.set(param, data); + subscribers.forEach(callback => callback()); + }, + [param], + ); + const subscribe = useCallback((callback: () => void) => { + subscribers.add(callback); + return () => subscribers.delete(callback); + }, []); + + return { + get, + set, + subscribe, + }; +}; export const useFetch = ( fetcher: ((...args: any) => Promise) | undefined, @@ -10,35 +55,63 @@ export const useFetch = ( onSuccess?: (data: T) => void; }, ) => { - const [data, setData] = useSafeState(null); - const requestStatus = useLoadingStatus({ - status: 'loading', - }); + const cache = useCache(params); + const [state, setState] = useState(cache.get()); const fetcherRef = useRef(fetcher); + useEffect(() => { + const unsub = cache.subscribe(() => { + setState(cache.get()); + }); + return () => unsub(); + }, [cache.subscribe]); + useEffect(() => { if (!fetcherRef.current) { return; } - requestStatus.setLoading(); + + // Only fetch stale data + if (Date.now() - (cache.get()?.cachedAt || 0) < 20000) { + return; + } + + // No parallel requests for the same resource + if (cache.get()?.isLoading) { + return; + } + + cache.set({ + data: null, + isLoading: true, + error: null, + }); fetcherRef .current(params) .then(result => { - requestStatus.setIdle(); if (typeof result !== 'undefined') { - setData(typeof result === 'object' ? { ...result } : result); - callbacks?.onSuccess?.(typeof result === 'object' ? { ...result } : result); + const data = typeof result === 'object' ? { ...result } : result; + cache.set({ + data, + isLoading: false, + error: null, + cachedAt: Date.now(), + }); + callbacks?.onSuccess?.(data); } }) .catch(() => { - requestStatus.setError(); - setData(null); + cache.set({ + data: null, + isLoading: false, + error: true, + cachedAt: Date.now(), + }); }); - }, [JSON.stringify(params)]); + }, [JSON.stringify(params), cache.set, cache.get]); return { - status: requestStatus, - data, + ...state, }; }; diff --git a/packages/clerk-js/src/ui/hooks/useFetchRoles.ts b/packages/clerk-js/src/ui/hooks/useFetchRoles.ts index b685c4337d8..211c28262ff 100644 --- a/packages/clerk-js/src/ui/hooks/useFetchRoles.ts +++ b/packages/clerk-js/src/ui/hooks/useFetchRoles.ts @@ -13,10 +13,10 @@ const getRolesParams = { }; export const useFetchRoles = () => { const { organization } = useOrganization(); - const { data, status } = useFetch(organization?.getRoles, getRolesParams); + const { data, isLoading } = useFetch(organization?.getRoles, getRolesParams); return { - isLoading: status.isLoading, + isLoading, options: data?.data?.map(role => ({ value: role.key, label: role.name })), }; }; From 45f4999abd4b3edd5888071c7116dfa17cbf741e Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 21 Dec 2023 12:51:05 +0200 Subject: [PATCH 2/7] chore(clerk-js): Clear cache before each test --- .../__tests__/OrganizationMembers.test.tsx | 9 +++++++++ .../OrganizationSwitcherPopover.tsx | 2 ++ .../OrganizationSwitcherTrigger.tsx | 1 + .../__tests__/OrganizationSwitcher.test.tsx | 1 + .../src/ui/elements/OrganizationPreview.tsx | 4 +++- packages/clerk-js/src/ui/hooks/useFetch.ts | 18 +++++++++++++----- .../clerk-js/src/ui/hooks/useFetchRoles.ts | 4 ++-- 7 files changed, 31 insertions(+), 8 deletions(-) diff --git a/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx b/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx index c0d03ce549c..2b91a639cba 100644 --- a/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx @@ -4,6 +4,7 @@ import { screen, waitFor, waitForElementToBeRemoved } from '@testing-library/rea import userEvent from '@testing-library/user-event'; import { render } from '../../../../testUtils'; +import { clearFetchCache } from '../../../hooks/useFetch'; import { bindCreateFixtures } from '../../../utils/test/createFixtures'; import { OrganizationMembers } from '../OrganizationMembers'; import { createFakeMember, createFakeOrganizationInvitation, createFakeOrganizationMembershipRequest } from './utils'; @@ -15,6 +16,13 @@ async function waitForLoadingCompleted(container: HTMLElement) { } describe('OrganizationMembers', () => { + /** + * `` internally uses useFetch which caches the results, be sure to clear the cache before each test + */ + beforeEach(() => { + clearFetchCache(); + }); + it('renders the Organization Members page', async () => { const { wrapper, fixtures } = await createFixtures(f => { f.withOrganizations(); @@ -174,6 +182,7 @@ describe('OrganizationMembers', () => { }), ); + // fixtures.clerk.organization?.getRoles.mockRejectedValue(null); fixtures.clerk.organization?.getRoles.mockResolvedValue({ total_count: 2, data: [ diff --git a/packages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsx b/packages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsx index f8074feecef..7059868d528 100644 --- a/packages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherPopover.tsx @@ -146,6 +146,7 @@ export const OrganizationSwitcherPopover = React.forwardRef ({ padding: `${t.space.$4} ${t.space.$5}`, @@ -178,6 +179,7 @@ export const OrganizationSwitcherPopover = React.forwardRef ({ padding: `${t.space.$4} ${t.space.$5}`, diff --git a/packages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherTrigger.tsx b/packages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherTrigger.tsx index f4d85a8e967..d15a4946df7 100644 --- a/packages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherTrigger.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationSwitcher/OrganizationSwitcherTrigger.tsx @@ -43,6 +43,7 @@ export const OrganizationSwitcherTrigger = withAvatarShimmer( elementId={'organizationSwitcherTrigger'} gap={3} size='xs' + fetchRoles organization={organization} sx={t => ({ maxWidth: '30ch', color: t.colors.$blackAlpha600 })} /> diff --git a/packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx b/packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx index d85f2195b6f..0cf7cfa5ffc 100644 --- a/packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationSwitcher/__tests__/OrganizationSwitcher.test.tsx @@ -56,6 +56,7 @@ describe('OrganizationSwitcher', () => { }); }); + fixtures.clerk.organization?.getRoles.mockRejectedValue(null); fixtures.clerk.user?.getOrganizationInvitations.mockReturnValueOnce( Promise.resolve({ data: [], diff --git a/packages/clerk-js/src/ui/elements/OrganizationPreview.tsx b/packages/clerk-js/src/ui/elements/OrganizationPreview.tsx index deb5a20550d..074c762e5ac 100644 --- a/packages/clerk-js/src/ui/elements/OrganizationPreview.tsx +++ b/packages/clerk-js/src/ui/elements/OrganizationPreview.tsx @@ -17,6 +17,7 @@ export type OrganizationPreviewProps = Omit, 'elem badge?: React.ReactNode; rounded?: boolean; elementId?: OrganizationPreviewId; + fetchRoles?: boolean; }; export const OrganizationPreview = (props: OrganizationPreviewProps) => { @@ -25,6 +26,7 @@ export const OrganizationPreview = (props: OrganizationPreviewProps) => { size = 'md', icon, rounded = false, + fetchRoles = false, badge, sx, user, @@ -38,7 +40,7 @@ export const OrganizationPreview = (props: OrganizationPreviewProps) => { const { localizeCustomRole } = useLocalizeCustomRoles(); const membership = user?.organizationMemberships.find(membership => membership.organization.id === organization.id); - const { options } = useFetchRoles(); + const { options } = useFetchRoles(fetchRoles); const unlocalizedRoleLabel = options?.find(a => a.value === membership?.role)?.label; const mainTextSize = diff --git a/packages/clerk-js/src/ui/hooks/useFetch.ts b/packages/clerk-js/src/ui/hooks/useFetch.ts index bf0fc3431e0..522b407d3aa 100644 --- a/packages/clerk-js/src/ui/hooks/useFetch.ts +++ b/packages/clerk-js/src/ui/hooks/useFetch.ts @@ -15,10 +15,20 @@ export interface Cache { set(key: string, value: State): void; delete(key: string): void; + + clear(): void; } -const map = new WeakMap(); +let requestCache = new WeakMap(); const subscribers = new Set<() => void>(); + +/** + * This utility should only be used in tests to clear previously fetched data + */ +export const clearFetchCache = () => { + requestCache = new WeakMap(); +}; + const useCache = ( param: any, ): { @@ -26,12 +36,10 @@ const useCache = ( set: (state: State) => void; subscribe: (callback: () => void) => () => void; } => { - // const subscribers = useRef(new Set<() => void>()); - - const get = useCallback(() => map.get(param), [param]); + const get = useCallback(() => requestCache.get(param), [param]); const set = useCallback( (data: State) => { - map.set(param, data); + requestCache.set(param, data); subscribers.forEach(callback => callback()); }, [param], diff --git a/packages/clerk-js/src/ui/hooks/useFetchRoles.ts b/packages/clerk-js/src/ui/hooks/useFetchRoles.ts index 211c28262ff..4ec4830e379 100644 --- a/packages/clerk-js/src/ui/hooks/useFetchRoles.ts +++ b/packages/clerk-js/src/ui/hooks/useFetchRoles.ts @@ -11,9 +11,9 @@ const getRolesParams = { */ pageSize: 20, }; -export const useFetchRoles = () => { +export const useFetchRoles = (enabled = true) => { const { organization } = useOrganization(); - const { data, isLoading } = useFetch(organization?.getRoles, getRolesParams); + const { data, isLoading } = useFetch(enabled ? organization?.getRoles : undefined, getRolesParams); return { isLoading, From 6559ce826eef724654dfad82e9b402b18f5e9967 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 21 Dec 2023 13:01:48 +0200 Subject: [PATCH 3/7] chore(clerk-js): Add changeset --- .changeset/two-crews-talk.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/two-crews-talk.md diff --git a/.changeset/two-crews-talk.md b/.changeset/two-crews-talk.md new file mode 100644 index 00000000000..c2181e0dda4 --- /dev/null +++ b/.changeset/two-crews-talk.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': patch +--- + +Bug fix: fetch custom roles in OrganizationSwitcher From 033fba30adac2e6ded484d2fc8cd4f5657ceb30a Mon Sep 17 00:00:00 2001 From: panteliselef Date: Thu, 21 Dec 2023 13:19:58 +0200 Subject: [PATCH 4/7] chore(clerk-js): Clean up --- .../__tests__/OrganizationMembers.test.tsx | 1 - .../src/ui/elements/OrganizationPreview.tsx | 4 +-- packages/clerk-js/src/ui/hooks/useFetch.ts | 26 +++++++++---------- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx b/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx index 2b91a639cba..f6c8a7a030a 100644 --- a/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationProfile/__tests__/OrganizationMembers.test.tsx @@ -182,7 +182,6 @@ describe('OrganizationMembers', () => { }), ); - // fixtures.clerk.organization?.getRoles.mockRejectedValue(null); fixtures.clerk.organization?.getRoles.mockResolvedValue({ total_count: 2, data: [ diff --git a/packages/clerk-js/src/ui/elements/OrganizationPreview.tsx b/packages/clerk-js/src/ui/elements/OrganizationPreview.tsx index 074c762e5ac..9d91fa0f782 100644 --- a/packages/clerk-js/src/ui/elements/OrganizationPreview.tsx +++ b/packages/clerk-js/src/ui/elements/OrganizationPreview.tsx @@ -38,9 +38,9 @@ export const OrganizationPreview = (props: OrganizationPreviewProps) => { } = props; const { localizeCustomRole } = useLocalizeCustomRoles(); - const membership = user?.organizationMemberships.find(membership => membership.organization.id === organization.id); - const { options } = useFetchRoles(fetchRoles); + + const membership = user?.organizationMemberships.find(membership => membership.organization.id === organization.id); const unlocalizedRoleLabel = options?.find(a => a.value === membership?.role)?.label; const mainTextSize = diff --git a/packages/clerk-js/src/ui/hooks/useFetch.ts b/packages/clerk-js/src/ui/hooks/useFetch.ts index 522b407d3aa..23578d887a9 100644 --- a/packages/clerk-js/src/ui/hooks/useFetch.ts +++ b/packages/clerk-js/src/ui/hooks/useFetch.ts @@ -8,8 +8,6 @@ export type State = { }; export interface Cache { - keys(): IterableIterator; - get(key: string): State | undefined; set(key: string, value: State): void; @@ -19,7 +17,14 @@ export interface Cache { clear(): void; } +/** + * Global cache for storing status of fetched resources + */ let requestCache = new WeakMap(); + +/** + * A set to store subscribers in order to notify when the value of a key of `requestCache` changes + */ const subscribers = new Set<() => void>(); /** @@ -76,17 +81,11 @@ export const useFetch = ( }, [cache.subscribe]); useEffect(() => { - if (!fetcherRef.current) { - return; - } - - // Only fetch stale data - if (Date.now() - (cache.get()?.cachedAt || 0) < 20000) { - return; - } + const fetcherMissing = !fetcherRef.current; + const isCacheStale = Date.now() - (cache.get()?.cachedAt || 0) < 20000; + const isRequestOnGoing = cache.get()?.isLoading; - // No parallel requests for the same resource - if (cache.get()?.isLoading) { + if (fetcherMissing || isCacheStale || isRequestOnGoing) { return; } @@ -95,8 +94,7 @@ export const useFetch = ( isLoading: true, error: null, }); - fetcherRef - .current(params) + fetcherRef.current!(params) .then(result => { if (typeof result !== 'undefined') { const data = typeof result === 'object' ? { ...result } : result; From f315922b06b3ecc76691794295d3108295d551bd Mon Sep 17 00:00:00 2001 From: panteliselef Date: Fri, 22 Dec 2023 21:52:42 +0200 Subject: [PATCH 5/7] fix(clerk-js): Change WeakMap to Map and store a serialized key --- packages/clerk-js/src/ui/hooks/useFetch.ts | 65 ++++++++++------------ 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/packages/clerk-js/src/ui/hooks/useFetch.ts b/packages/clerk-js/src/ui/hooks/useFetch.ts index 23578d887a9..c4adb4ecb96 100644 --- a/packages/clerk-js/src/ui/hooks/useFetch.ts +++ b/packages/clerk-js/src/ui/hooks/useFetch.ts @@ -7,20 +7,10 @@ export type State = { cachedAt?: number; }; -export interface Cache { - get(key: string): State | undefined; - - set(key: string, value: State): void; - - delete(key: string): void; - - clear(): void; -} - /** * Global cache for storing status of fetched resources */ -let requestCache = new WeakMap(); +let requestCache = new Map(); /** * A set to store subscribers in order to notify when the value of a key of `requestCache` changes @@ -31,23 +21,26 @@ const subscribers = new Set<() => void>(); * This utility should only be used in tests to clear previously fetched data */ export const clearFetchCache = () => { - requestCache = new WeakMap(); + requestCache = new Map(); }; -const useCache = ( - param: any, +const serialize = (obj: unknown) => JSON.stringify(obj); + +const useCache = ( + key: K, ): { - get: () => State | undefined; - set: (state: State) => void; - subscribe: (callback: () => void) => () => void; + getCache: () => State | undefined; + setCache: (state: State) => void; + subscribeCache: (callback: () => void) => () => void; } => { - const get = useCallback(() => requestCache.get(param), [param]); + const serializedKey = serialize(key); + const get = useCallback(() => requestCache.get(serializedKey), [serializedKey]); const set = useCallback( (data: State) => { - requestCache.set(param, data); + requestCache.set(serializedKey, data); subscribers.forEach(callback => callback()); }, - [param], + [serializedKey], ); const subscribe = useCallback((callback: () => void) => { subscribers.add(callback); @@ -55,41 +48,41 @@ const useCache = ( }, []); return { - get, - set, - subscribe, + getCache: get, + setCache: set, + subscribeCache: subscribe, }; }; -export const useFetch = ( +export const useFetch = ( fetcher: ((...args: any) => Promise) | undefined, - params: any, + params: K, callbacks?: { onSuccess?: (data: T) => void; }, ) => { - const cache = useCache(params); + const { subscribeCache, getCache, setCache } = useCache(params); - const [state, setState] = useState(cache.get()); + const [state, setState] = useState(getCache()); const fetcherRef = useRef(fetcher); useEffect(() => { - const unsub = cache.subscribe(() => { - setState(cache.get()); + const unsub = subscribeCache(() => { + setState(getCache()); }); return () => unsub(); - }, [cache.subscribe]); + }, [getCache, subscribeCache]); useEffect(() => { const fetcherMissing = !fetcherRef.current; - const isCacheStale = Date.now() - (cache.get()?.cachedAt || 0) < 20000; - const isRequestOnGoing = cache.get()?.isLoading; + const isCacheStale = Date.now() - (getCache()?.cachedAt || 0) < 20000; + const isRequestOnGoing = getCache()?.isLoading; if (fetcherMissing || isCacheStale || isRequestOnGoing) { return; } - cache.set({ + setCache({ data: null, isLoading: true, error: null, @@ -98,7 +91,7 @@ export const useFetch = ( .then(result => { if (typeof result !== 'undefined') { const data = typeof result === 'object' ? { ...result } : result; - cache.set({ + setCache({ data, isLoading: false, error: null, @@ -108,14 +101,14 @@ export const useFetch = ( } }) .catch(() => { - cache.set({ + setCache({ data: null, isLoading: false, error: true, cachedAt: Date.now(), }); }); - }, [JSON.stringify(params), cache.set, cache.get]); + }, [serialize(params), setCache, getCache]); return { ...state, From 8132f4a8f3e57796924710fc3f51bbe1c9658782 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Fri, 22 Dec 2023 22:05:40 +0200 Subject: [PATCH 6/7] chore(clerk-js): Add `isValidating` status --- packages/clerk-js/src/ui/hooks/useFetch.ts | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/packages/clerk-js/src/ui/hooks/useFetch.ts b/packages/clerk-js/src/ui/hooks/useFetch.ts index c4adb4ecb96..bd8a48f7504 100644 --- a/packages/clerk-js/src/ui/hooks/useFetch.ts +++ b/packages/clerk-js/src/ui/hooks/useFetch.ts @@ -1,9 +1,16 @@ import { useCallback, useEffect, useRef, useState } from 'react'; export type State = { - data?: Data; - error?: Error; - isLoading?: boolean; + data: Data | null; + error: Error | null; + /** + * if there's an ongoing request and no "loaded data" + */ + isLoading: boolean; + /** + * if there's a request or revalidation loading + */ + isValidating: boolean; cachedAt?: number; }; @@ -75,8 +82,8 @@ export const useFetch = ( useEffect(() => { const fetcherMissing = !fetcherRef.current; - const isCacheStale = Date.now() - (getCache()?.cachedAt || 0) < 20000; - const isRequestOnGoing = getCache()?.isLoading; + const isCacheStale = Date.now() - (getCache()?.cachedAt || 0) < 3000; //20000; + const isRequestOnGoing = getCache()?.isValidating; if (fetcherMissing || isCacheStale || isRequestOnGoing) { return; @@ -84,7 +91,8 @@ export const useFetch = ( setCache({ data: null, - isLoading: true, + isLoading: !getCache(), + isValidating: true, error: null, }); fetcherRef.current!(params) @@ -94,6 +102,7 @@ export const useFetch = ( setCache({ data, isLoading: false, + isValidating: false, error: null, cachedAt: Date.now(), }); @@ -104,6 +113,7 @@ export const useFetch = ( setCache({ data: null, isLoading: false, + isValidating: false, error: true, cachedAt: Date.now(), }); From 67c450cd5764c8667cde399742c0a80777333228 Mon Sep 17 00:00:00 2001 From: panteliselef Date: Fri, 22 Dec 2023 22:11:46 +0200 Subject: [PATCH 7/7] chore(clerk-js): Use `useSyncExternalStore` --- packages/clerk-js/src/ui/hooks/useFetch.ts | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/packages/clerk-js/src/ui/hooks/useFetch.ts b/packages/clerk-js/src/ui/hooks/useFetch.ts index bd8a48f7504..99e3b23028b 100644 --- a/packages/clerk-js/src/ui/hooks/useFetch.ts +++ b/packages/clerk-js/src/ui/hooks/useFetch.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useRef, useState } from 'react'; +import { useCallback, useEffect, useRef, useSyncExternalStore } from 'react'; export type State = { data: Data | null; @@ -69,20 +69,13 @@ export const useFetch = ( }, ) => { const { subscribeCache, getCache, setCache } = useCache(params); - - const [state, setState] = useState(getCache()); const fetcherRef = useRef(fetcher); - useEffect(() => { - const unsub = subscribeCache(() => { - setState(getCache()); - }); - return () => unsub(); - }, [getCache, subscribeCache]); + const cached = useSyncExternalStore(subscribeCache, getCache); useEffect(() => { const fetcherMissing = !fetcherRef.current; - const isCacheStale = Date.now() - (getCache()?.cachedAt || 0) < 3000; //20000; + const isCacheStale = Date.now() - (getCache()?.cachedAt || 0) < 1000 * 60 * 2; //cache for 2 minutes; const isRequestOnGoing = getCache()?.isValidating; if (fetcherMissing || isCacheStale || isRequestOnGoing) { @@ -121,6 +114,6 @@ export const useFetch = ( }, [serialize(params), setCache, getCache]); return { - ...state, + ...cached, }; };