Skip to content

Commit

Permalink
fix(clerk-js): Fetching custom role in OrganizationSwitcher with cache (
Browse files Browse the repository at this point in the history
  • Loading branch information
panteliselef committed Dec 22, 2023
1 parent 3ec07c1 commit 741546b
Show file tree
Hide file tree
Showing 11 changed files with 134 additions and 36 deletions.
5 changes: 5 additions & 0 deletions .changeset/two-crews-talk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Bug fix: fetch custom roles in OrganizationSwitcher
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const RemoveDomainPage = () => {
const { params } = useRouter();

const ref = React.useRef<OrganizationDomainResource>();
const { data: domain, status: domainStatus } = useFetch(
const { data: domain, isLoading: domainIsLoading } = useFetch(
organization?.getDomain,
{
domainId: params.id,
Expand All @@ -37,7 +37,7 @@ export const RemoveDomainPage = () => {
return null;
}

if (domainStatus.isLoading || !domain) {
if (domainIsLoading || !domain) {
return (
<Flex
direction={'row'}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { LinkButtonWithDescription } from '../UserProfile/LinkButtonWithDescript
import { OrganizationProfileBreadcrumbs } from './OrganizationProfileNavbar';

const useCalloutLabel = (
domain: OrganizationDomainResource | null,
domain: OrganizationDomainResource | undefined | null,
{
infoLabel: infoLabelKey,
}: {
Expand Down Expand Up @@ -120,7 +120,7 @@ export const VerifiedDomainPage = withCardStateProvider(() => {
type: 'checkbox',
});

const { data: domain, status: domainStatus } = useFetch(
const { data: domain, isLoading: domainIsLoading } = useFetch(
organization?.getDomain,
{
domainId: params.id,
Expand Down Expand Up @@ -168,7 +168,7 @@ export const VerifiedDomainPage = withCardStateProvider(() => {
return null;
}

if (domainStatus.isLoading || !domain) {
if (domainIsLoading || !domain) {
return (
<Flex
direction={'row'}
Expand Down Expand Up @@ -264,7 +264,7 @@ export const VerifiedDomainPage = withCardStateProvider(() => {
localizationKey={localizationKeys(
'organizationProfile.verifiedDomainPage.enrollmentTab.formButton__save',
)}
isDisabled={domainStatus.isLoading || !domain || !isFormDirty}
isDisabled={domainIsLoading || !domain || !isFormDirty}
/>
</Form.Root>
</TabPanel>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const VerifyDomainPage = withCardStateProvider(() => {

const [success, setSuccess] = React.useState(false);

const { data: domain, status: domainStatus } = useFetch(organization?.getDomain, {
const { data: domain, isLoading: domainIsLoading } = useFetch(organization?.getDomain, {
domainId: params.id,
});
const title = localizationKeys('organizationProfile.verifyDomainPage.title');
Expand Down Expand Up @@ -122,7 +122,7 @@ export const VerifyDomainPage = withCardStateProvider(() => {
});
};

if (domainStatus.isLoading || !domain) {
if (domainIsLoading || !domain) {
return (
<Flex
direction={'row'}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { act, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event';

import { render } from '../../../../testUtils';
import { clearFetchCache } from '../../../hooks/useFetch';
import { bindCreateFixtures } from '../../../utils/test/createFixtures';
import { runFakeTimers } from '../../../utils/test/runFakeTimers';
import { OrganizationMembers } from '../OrganizationMembers';
Expand All @@ -12,6 +13,13 @@ import { createFakeMember, createFakeOrganizationInvitation, createFakeOrganizat
const { createFixtures } = bindCreateFixtures('OrganizationProfile');

describe('OrganizationMembers', () => {
/**
* `<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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ export const OrganizationSwitcherPopover = React.forwardRef<HTMLDivElement, Orga
gap={4}
organization={currentOrg}
user={user}
fetchRoles
sx={theme => t => ({ padding: `0 ${theme.space.$6}`, marginBottom: t.space.$2 })}
/>
<Actions role='menu'>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export const OrganizationSwitcherTrigger = withAvatarShimmer(
elementId={'organizationSwitcher'}
gap={3}
size={'sm'}
fetchRoles
organization={organization}
sx={{ maxWidth: '30ch' }}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ describe('OrganizationSwitcher', () => {
});
});

fixtures.clerk.organization?.getRoles.mockRejectedValue(null);
fixtures.clerk.user?.getOrganizationInvitations.mockReturnValueOnce(
Promise.resolve({
data: [],
Expand Down
13 changes: 10 additions & 3 deletions packages/clerk-js/src/ui/elements/OrganizationPreview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import type { OrganizationPreviewId, UserOrganizationInvitationResource, UserRes
import React from 'react';

import { descriptors, Flex, Text } from '../customizables';
import { useFetchRoles, useLocalizeCustomRoles } from '../hooks/useFetchRoles';
import type { PropsOfComponent, ThemableCssProp } from '../styledSystem';
import { roleLocalizationKey } from '../utils';
import { OrganizationAvatar } from './OrganizationAvatar';

export type OrganizationPreviewProps = Omit<PropsOfComponent<typeof Flex>, 'elementId'> & {
Expand All @@ -16,6 +16,7 @@ export type OrganizationPreviewProps = Omit<PropsOfComponent<typeof Flex>, 'elem
badge?: React.ReactNode;
rounded?: boolean;
elementId?: OrganizationPreviewId;
fetchRoles?: boolean;
};

export const OrganizationPreview = (props: OrganizationPreviewProps) => {
Expand All @@ -24,6 +25,7 @@ export const OrganizationPreview = (props: OrganizationPreviewProps) => {
size = 'md',
icon,
rounded = false,
fetchRoles = false,
badge,
sx,
user,
Expand All @@ -32,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 { options } = useFetchRoles(fetchRoles);

const membership = user?.organizationMemberships.find(membership => membership.organization.id === organization.id);
const unlocalizedRoleLabel = options?.find(a => a.value === membership?.role)?.label;

return (
<Flex
Expand Down Expand Up @@ -80,7 +87,7 @@ export const OrganizationPreview = (props: OrganizationPreviewProps) => {
<Text
elementDescriptor={descriptors.organizationPreviewSecondaryIdentifier}
elementId={descriptors.organizationPreviewSecondaryIdentifier.setId(elementId)}
localizationKey={role && roleLocalizationKey(role)}
localizationKey={localizeCustomRole(membership?.role) || unlocalizedRoleLabel}
variant='smallRegular'
colorScheme='neutral'
truncate
Expand Down
119 changes: 97 additions & 22 deletions packages/clerk-js/src/ui/hooks/useFetch.ts
Original file line number Diff line number Diff line change
@@ -1,44 +1,119 @@
import { useEffect, useRef } from 'react';
import { useCallback, useEffect, useRef, useSyncExternalStore } from 'react';

import { useLoadingStatus } from './useLoadingStatus';
import { useSafeState } from './useSafeState';
export type State<Data = any, Error = any> = {
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;
};

/**
* Global cache for storing status of fetched resources
*/
let requestCache = new Map<string, State>();

/**
* A set to store subscribers in order to notify when the value of a key of `requestCache` changes
*/
const subscribers = new Set<() => void>();

/**
* This utility should only be used in tests to clear previously fetched data
*/
export const clearFetchCache = () => {
requestCache = new Map<string, State>();
};

const serialize = (obj: unknown) => JSON.stringify(obj);

export const useFetch = <T>(
const useCache = <K = any, V = any>(
key: K,
): {
getCache: () => State<V> | undefined;
setCache: (state: State) => void;
subscribeCache: (callback: () => void) => () => void;
} => {
const serializedKey = serialize(key);
const get = useCallback(() => requestCache.get(serializedKey), [serializedKey]);
const set = useCallback(
(data: State) => {
requestCache.set(serializedKey, data);
subscribers.forEach(callback => callback());
},
[serializedKey],
);
const subscribe = useCallback((callback: () => void) => {
subscribers.add(callback);
return () => subscribers.delete(callback);
}, []);

return {
getCache: get,
setCache: set,
subscribeCache: subscribe,
};
};

export const useFetch = <K, T>(
fetcher: ((...args: any) => Promise<T>) | undefined,
params: any,
params: K,
callbacks?: {
onSuccess?: (data: T) => void;
},
) => {
const [data, setData] = useSafeState<T | null>(null);
const requestStatus = useLoadingStatus({
status: 'loading',
});

const { subscribeCache, getCache, setCache } = useCache<K, T>(params);
const fetcherRef = useRef(fetcher);

const cached = useSyncExternalStore(subscribeCache, getCache);

useEffect(() => {
if (!fetcherRef.current) {
const fetcherMissing = !fetcherRef.current;
const isCacheStale = Date.now() - (getCache()?.cachedAt || 0) < 1000 * 60 * 2; //cache for 2 minutes;
const isRequestOnGoing = getCache()?.isValidating;

if (fetcherMissing || isCacheStale || isRequestOnGoing) {
return;
}
requestStatus.setLoading();
fetcherRef
.current(params)

setCache({
data: null,
isLoading: !getCache(),
isValidating: 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;
setCache({
data,
isLoading: false,
isValidating: false,
error: null,
cachedAt: Date.now(),
});
callbacks?.onSuccess?.(data);
}
})
.catch(() => {
requestStatus.setError();
setData(null);
setCache({
data: null,
isLoading: false,
isValidating: false,
error: true,
cachedAt: Date.now(),
});
});
}, [JSON.stringify(params)]);
}, [serialize(params), setCache, getCache]);

return {
status: requestStatus,
data,
...cached,
};
};
6 changes: 3 additions & 3 deletions packages/clerk-js/src/ui/hooks/useFetchRoles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ const getRolesParams = {
*/
pageSize: 20,
};
export const useFetchRoles = () => {
export const useFetchRoles = (enabled = true) => {
const { organization } = useCoreOrganization();
const { data, status } = useFetch(organization?.getRoles, getRolesParams);
const { data, isLoading } = useFetch(enabled ? organization?.getRoles : undefined, getRolesParams);

return {
isLoading: status.isLoading,
isLoading,
options: data?.data?.map(role => ({ value: role.key, label: role.name })),
};
};
Expand Down

0 comments on commit 741546b

Please sign in to comment.