ORG-83 userInvitations in useOrganizationList by panteliselef · Pull Request #1520 · clerk/javascript

panteliselef · 2023-07-25T11:18:37Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

This PR

Creates UserOrganizationInvitation and UserOrganizationInvitationResource
Updates useOrganization to return userInvitations
Adds the ability to aggregate the userInvitations when fetched from useOrganization (Very useful from infinite scrolling)

You might be familiar with OrganizationInvitation that already exists in our codebase.

Both UserOrganizationInvitation and OrganizationInvitation describe the same entity. The former is from the perspective of a user where the later from the perspective of an organization.

Exposed API

const{
isLoaded,
organizationList,// Same as before
createOrganization,// Same as before
setActive,// Same as beforeuserInvitations: {
data,
count,
isFetching,
isLoading,
isError,
page,
pageCount,
fetchPage,
fetchNext,
fetchPrevious,
hasNextPage,
hasPreviousPage,},}=useOrganizationList({userInvitations: {page: 1,pageSize: 10,infinite: true,// Aggregate the data or not. Ideal for infinite listskeepPreviousData: true,},});

changeset-bot · 2023-07-25T11:18:40Z

🦋 Changeset detected

Latest commit: 34484b6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@clerk/clerk-js	Minor
@clerk/shared	Minor
@clerk/clerk-react	Minor
@clerk/types	Minor
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
@clerk/remix	Patch
gatsby-plugin-clerk	Patch
@clerk/nextjs	Patch
@clerk/backend	Patch
@clerk/fastify	Patch
@clerk/localizations	Patch
@clerk/clerk-sdk-node	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jit-ci

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

nikosdouvlis · 2023-07-26T07:05:24Z

 type CoreClerkContextWrapperProps = {
 clerk: Clerk;
 children: React.ReactNode;
+ swrConfig?: any;


Could you please explain why we need this?

This is passed from MockClerkProvider down to CoreOrganizationProvider in order to clear swr cache between tests

nikosdouvlis · 2023-07-26T07:11:02Z

 <ClientContext.Provider value={clientCtx}>
 <SessionContext.Provider value={sessionCtx}>
- <OrganizationContext.Provider value={organizationCtx}>
+ <OrganizationProvider {...organizationCtx.value}>


I guess we're treating this differently because of swrConfig... Not sure how I feel about this small discrepancy to be honest - if this is a testing-only issue, we might be able to find a different solution that works for the tests. What was the original issue?

We need a way to clear cache between tests. The recommended way from the SWR docs is this
Wrapping our CoreClerkContextWrapper with will not work. I verified this by logging the results of useSWRConfig inside our useOrganization while tests where running.
My solution was to expose a OrganizationProvider instead of the "low-level" context and include the <SWRConfig/> provider inside of the OrganizationProvider.
☝️ This is now working as expecting.

I see that the cache provider follows the following interface:
interface Cache<Data> { get(key: string): Data | undefined set(key: string, value: Data): void delete(key: string): void keys(): IterableIterator<string> }
Did you try wrapping the test contexts with a SWRConfig that uses a "noop" cache provider, eg:
{ get: () => undefined, set: noop, delete: noop, keys: () => [] }
so that we don't need to reset it between the tests?

I don't see why this would help. It seemed like the useSWRConfig within useOrganization would return the default values, as it couldn't reach the correct provider of SWRConfig.
For example is I was passing <SWRConfig value={{ dedupingInterval: 0 }}/> the useSWRConfig would return {dedupingInterval: 2000} which is the default value

Does this mean that a SWRConfig provider is found deeper in the tree so you cannot override the defaults in the tests?

nikosdouvlis · 2023-07-26T07:34:50Z

 }));
 }
+
+function cacheKey(type: 'userInvitations', user: UserResource, pagination: GetUserOrganizationInvitations) {


Do we care about typing these objects here? Do you think this would be enough? If it's generic enough in can be used in other similar places as well - other hooks using swr for example.
Suggested change
functioncacheKey(type: 'userInvitations',user: UserResource,pagination: GetUserOrganizationInvitations){
functioncacheKey(...keys: Array<string|number|undefined>){

I think you are correct. Although swr@2 seems to automatically parse objects as keys, which will make this function obsolete

We need to make sure to check whether swr uses referential equality checks or if it somehow builds a scalar value by paring the object's values - otherwise we might risk invalidating the cache on every render.

Does this answer your question ?

SokratisVidros · 2023-08-07T10:01:56Z

+ const [paginatedPage, setPaginatedPage] = useState(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);
+
+ // Cache initialPage and initialPageSize until unmount
+ const initialPageRef = useRef(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);


❓ Do we need the last default fallback? Can't we ensure that the initialPage attribute always has a value?

Follow-up question? Do we need a ref for this?

initialPage does not always have a value as someone can simply pass as params the following
useOrganizationList({infinite:true})
Here we want to not use the defaults in general but use the default value of initialPage

SokratisVidros · 2023-08-07T10:14:53Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


❓ What does isomorphic mean in this context? It feels that it's a bit confusing.

I needed a way to introduce parity between the returned values of useSWR and useSWRInfinite.
based on the params received in the hook return the appropriate values

The naming still feels a bit off. I'd just remove the isomorphic prefix.

Sure, i'll update this shortly in a following PR

SokratisVidros · 2023-08-07T10:15:40Z

+ setSize,
+ mutate: userInvitationsInfiniteMutate,
+ } = useSWRInfinite(getInfiniteKey, ({ initialPage, initialPageSize, status }) => {
+ return !clerk.loaded || !user


❓ I believe this hook should trigger the fetch only if triggerInfinite is set to true

It does, you can check getInfiniteKey. The triggering is decided based on the key not the fetcher function. I updated that part and removed the conditional from this place to avoid confusions

SokratisVidros · 2023-08-09T12:11:38Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


The naming still feels a bit off. I'd just remove the isomorphic prefix.

…seOrganizationList` - Improves DX of useOrganizationList

… introduce OrganizationProvider We needed to wrap <OrganizationContext.Provider/> with SWRConfig provider so that useSWR hooks can read from the correct context in tests.

…d pageSize

…nList + expose isLoading and isFetching

clerk-cookie · 2024-08-09T00:01:29Z

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

panteliselef · 2023-07-25T11:18:37Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

This PR

Creates UserOrganizationInvitation and UserOrganizationInvitationResource
Updates useOrganization to return userInvitations
Adds the ability to aggregate the userInvitations when fetched from useOrganization (Very useful from infinite scrolling)

You might be familiar with OrganizationInvitation that already exists in our codebase.

Both UserOrganizationInvitation and OrganizationInvitation describe the same entity. The former is from the perspective of a user where the later from the perspective of an organization.

Exposed API

const{
isLoaded,
organizationList,// Same as before
createOrganization,// Same as before
setActive,// Same as beforeuserInvitations: {
data,
count,
isFetching,
isLoading,
isError,
page,
pageCount,
fetchPage,
fetchNext,
fetchPrevious,
hasNextPage,
hasPreviousPage,},}=useOrganizationList({userInvitations: {page: 1,pageSize: 10,infinite: true,// Aggregate the data or not. Ideal for infinite listskeepPreviousData: true,},});

changeset-bot · 2023-07-25T11:18:40Z

🦋 Changeset detected

Latest commit: 34484b6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@clerk/clerk-js	Minor
@clerk/shared	Minor
@clerk/clerk-react	Minor
@clerk/types	Minor
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
@clerk/remix	Patch
gatsby-plugin-clerk	Patch
@clerk/nextjs	Patch
@clerk/backend	Patch
@clerk/fastify	Patch
@clerk/localizations	Patch
@clerk/clerk-sdk-node	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jit-ci

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

nikosdouvlis · 2023-07-26T07:05:24Z

 type CoreClerkContextWrapperProps = {
 clerk: Clerk;
 children: React.ReactNode;
+ swrConfig?: any;


Could you please explain why we need this?

This is passed from MockClerkProvider down to CoreOrganizationProvider in order to clear swr cache between tests

nikosdouvlis · 2023-07-26T07:11:02Z

 <ClientContext.Provider value={clientCtx}>
 <SessionContext.Provider value={sessionCtx}>
- <OrganizationContext.Provider value={organizationCtx}>
+ <OrganizationProvider {...organizationCtx.value}>


I guess we're treating this differently because of swrConfig... Not sure how I feel about this small discrepancy to be honest - if this is a testing-only issue, we might be able to find a different solution that works for the tests. What was the original issue?

We need a way to clear cache between tests. The recommended way from the SWR docs is this
Wrapping our CoreClerkContextWrapper with will not work. I verified this by logging the results of useSWRConfig inside our useOrganization while tests where running.
My solution was to expose a OrganizationProvider instead of the "low-level" context and include the <SWRConfig/> provider inside of the OrganizationProvider.
☝️ This is now working as expecting.

I see that the cache provider follows the following interface:
interface Cache<Data> { get(key: string): Data | undefined set(key: string, value: Data): void delete(key: string): void keys(): IterableIterator<string> }
Did you try wrapping the test contexts with a SWRConfig that uses a "noop" cache provider, eg:
{ get: () => undefined, set: noop, delete: noop, keys: () => [] }
so that we don't need to reset it between the tests?

I don't see why this would help. It seemed like the useSWRConfig within useOrganization would return the default values, as it couldn't reach the correct provider of SWRConfig.
For example is I was passing <SWRConfig value={{ dedupingInterval: 0 }}/> the useSWRConfig would return {dedupingInterval: 2000} which is the default value

Does this mean that a SWRConfig provider is found deeper in the tree so you cannot override the defaults in the tests?

nikosdouvlis · 2023-07-26T07:34:50Z

 }));
 }
+
+function cacheKey(type: 'userInvitations', user: UserResource, pagination: GetUserOrganizationInvitations) {


Do we care about typing these objects here? Do you think this would be enough? If it's generic enough in can be used in other similar places as well - other hooks using swr for example.
Suggested change
functioncacheKey(type: 'userInvitations',user: UserResource,pagination: GetUserOrganizationInvitations){
functioncacheKey(...keys: Array<string|number|undefined>){

I think you are correct. Although swr@2 seems to automatically parse objects as keys, which will make this function obsolete

We need to make sure to check whether swr uses referential equality checks or if it somehow builds a scalar value by paring the object's values - otherwise we might risk invalidating the cache on every render.

Does this answer your question ?

SokratisVidros · 2023-08-07T10:01:56Z

+ const [paginatedPage, setPaginatedPage] = useState(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);
+
+ // Cache initialPage and initialPageSize until unmount
+ const initialPageRef = useRef(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);


❓ Do we need the last default fallback? Can't we ensure that the initialPage attribute always has a value?

Follow-up question? Do we need a ref for this?

initialPage does not always have a value as someone can simply pass as params the following
useOrganizationList({infinite:true})
Here we want to not use the defaults in general but use the default value of initialPage

SokratisVidros · 2023-08-07T10:14:53Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


❓ What does isomorphic mean in this context? It feels that it's a bit confusing.

I needed a way to introduce parity between the returned values of useSWR and useSWRInfinite.
based on the params received in the hook return the appropriate values

The naming still feels a bit off. I'd just remove the isomorphic prefix.

Sure, i'll update this shortly in a following PR

SokratisVidros · 2023-08-07T10:15:40Z

+ setSize,
+ mutate: userInvitationsInfiniteMutate,
+ } = useSWRInfinite(getInfiniteKey, ({ initialPage, initialPageSize, status }) => {
+ return !clerk.loaded || !user


❓ I believe this hook should trigger the fetch only if triggerInfinite is set to true

It does, you can check getInfiniteKey. The triggering is decided based on the key not the fetcher function. I updated that part and removed the conditional from this place to avoid confusions

SokratisVidros · 2023-08-09T12:11:38Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


The naming still feels a bit off. I'd just remove the isomorphic prefix.

…seOrganizationList` - Improves DX of useOrganizationList

… introduce OrganizationProvider We needed to wrap <OrganizationContext.Provider/> with SWRConfig provider so that useSWR hooks can read from the correct context in tests.

…d pageSize

…nList + expose isLoading and isFetching

clerk-cookie · 2024-08-09T00:01:29Z

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

panteliselef · 2023-07-25T11:18:37Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

This PR

Creates UserOrganizationInvitation and UserOrganizationInvitationResource
Updates useOrganization to return userInvitations
Adds the ability to aggregate the userInvitations when fetched from useOrganization (Very useful from infinite scrolling)

You might be familiar with OrganizationInvitation that already exists in our codebase.

Both UserOrganizationInvitation and OrganizationInvitation describe the same entity. The former is from the perspective of a user where the later from the perspective of an organization.

Exposed API

const{
isLoaded,
organizationList,// Same as before
createOrganization,// Same as before
setActive,// Same as beforeuserInvitations: {
data,
count,
isFetching,
isLoading,
isError,
page,
pageCount,
fetchPage,
fetchNext,
fetchPrevious,
hasNextPage,
hasPreviousPage,},}=useOrganizationList({userInvitations: {page: 1,pageSize: 10,infinite: true,// Aggregate the data or not. Ideal for infinite listskeepPreviousData: true,},});

changeset-bot · 2023-07-25T11:18:40Z

🦋 Changeset detected

Latest commit: 34484b6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@clerk/clerk-js	Minor
@clerk/shared	Minor
@clerk/clerk-react	Minor
@clerk/types	Minor
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
@clerk/remix	Patch
gatsby-plugin-clerk	Patch
@clerk/nextjs	Patch
@clerk/backend	Patch
@clerk/fastify	Patch
@clerk/localizations	Patch
@clerk/clerk-sdk-node	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jit-ci

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

nikosdouvlis · 2023-07-26T07:05:24Z

 type CoreClerkContextWrapperProps = {
 clerk: Clerk;
 children: React.ReactNode;
+ swrConfig?: any;


Could you please explain why we need this?

This is passed from MockClerkProvider down to CoreOrganizationProvider in order to clear swr cache between tests

nikosdouvlis · 2023-07-26T07:11:02Z

 <ClientContext.Provider value={clientCtx}>
 <SessionContext.Provider value={sessionCtx}>
- <OrganizationContext.Provider value={organizationCtx}>
+ <OrganizationProvider {...organizationCtx.value}>


I guess we're treating this differently because of swrConfig... Not sure how I feel about this small discrepancy to be honest - if this is a testing-only issue, we might be able to find a different solution that works for the tests. What was the original issue?

We need a way to clear cache between tests. The recommended way from the SWR docs is this
Wrapping our CoreClerkContextWrapper with will not work. I verified this by logging the results of useSWRConfig inside our useOrganization while tests where running.
My solution was to expose a OrganizationProvider instead of the "low-level" context and include the <SWRConfig/> provider inside of the OrganizationProvider.
☝️ This is now working as expecting.

I see that the cache provider follows the following interface:
interface Cache<Data> { get(key: string): Data | undefined set(key: string, value: Data): void delete(key: string): void keys(): IterableIterator<string> }
Did you try wrapping the test contexts with a SWRConfig that uses a "noop" cache provider, eg:
{ get: () => undefined, set: noop, delete: noop, keys: () => [] }
so that we don't need to reset it between the tests?

I don't see why this would help. It seemed like the useSWRConfig within useOrganization would return the default values, as it couldn't reach the correct provider of SWRConfig.
For example is I was passing <SWRConfig value={{ dedupingInterval: 0 }}/> the useSWRConfig would return {dedupingInterval: 2000} which is the default value

Does this mean that a SWRConfig provider is found deeper in the tree so you cannot override the defaults in the tests?

nikosdouvlis · 2023-07-26T07:34:50Z

 }));
 }
+
+function cacheKey(type: 'userInvitations', user: UserResource, pagination: GetUserOrganizationInvitations) {


Do we care about typing these objects here? Do you think this would be enough? If it's generic enough in can be used in other similar places as well - other hooks using swr for example.
Suggested change
functioncacheKey(type: 'userInvitations',user: UserResource,pagination: GetUserOrganizationInvitations){
functioncacheKey(...keys: Array<string|number|undefined>){

I think you are correct. Although swr@2 seems to automatically parse objects as keys, which will make this function obsolete

We need to make sure to check whether swr uses referential equality checks or if it somehow builds a scalar value by paring the object's values - otherwise we might risk invalidating the cache on every render.

Does this answer your question ?

SokratisVidros · 2023-08-07T10:01:56Z

+ const [paginatedPage, setPaginatedPage] = useState(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);
+
+ // Cache initialPage and initialPageSize until unmount
+ const initialPageRef = useRef(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);


❓ Do we need the last default fallback? Can't we ensure that the initialPage attribute always has a value?

Follow-up question? Do we need a ref for this?

initialPage does not always have a value as someone can simply pass as params the following
useOrganizationList({infinite:true})
Here we want to not use the defaults in general but use the default value of initialPage

SokratisVidros · 2023-08-07T10:14:53Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


❓ What does isomorphic mean in this context? It feels that it's a bit confusing.

I needed a way to introduce parity between the returned values of useSWR and useSWRInfinite.
based on the params received in the hook return the appropriate values

The naming still feels a bit off. I'd just remove the isomorphic prefix.

Sure, i'll update this shortly in a following PR

SokratisVidros · 2023-08-07T10:15:40Z

+ setSize,
+ mutate: userInvitationsInfiniteMutate,
+ } = useSWRInfinite(getInfiniteKey, ({ initialPage, initialPageSize, status }) => {
+ return !clerk.loaded || !user


❓ I believe this hook should trigger the fetch only if triggerInfinite is set to true

It does, you can check getInfiniteKey. The triggering is decided based on the key not the fetcher function. I updated that part and removed the conditional from this place to avoid confusions

SokratisVidros · 2023-08-09T12:11:38Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


The naming still feels a bit off. I'd just remove the isomorphic prefix.

…seOrganizationList` - Improves DX of useOrganizationList

… introduce OrganizationProvider We needed to wrap <OrganizationContext.Provider/> with SWRConfig provider so that useSWR hooks can read from the correct context in tests.

…d pageSize

…nList + expose isLoading and isFetching

clerk-cookie · 2024-08-09T00:01:29Z

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

panteliselef · 2023-07-25T11:18:37Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

This PR

Creates UserOrganizationInvitation and UserOrganizationInvitationResource
Updates useOrganization to return userInvitations
Adds the ability to aggregate the userInvitations when fetched from useOrganization (Very useful from infinite scrolling)

You might be familiar with OrganizationInvitation that already exists in our codebase.

Both UserOrganizationInvitation and OrganizationInvitation describe the same entity. The former is from the perspective of a user where the later from the perspective of an organization.

Exposed API

const{
isLoaded,
organizationList,// Same as before
createOrganization,// Same as before
setActive,// Same as beforeuserInvitations: {
data,
count,
isFetching,
isLoading,
isError,
page,
pageCount,
fetchPage,
fetchNext,
fetchPrevious,
hasNextPage,
hasPreviousPage,},}=useOrganizationList({userInvitations: {page: 1,pageSize: 10,infinite: true,// Aggregate the data or not. Ideal for infinite listskeepPreviousData: true,},});

changeset-bot · 2023-07-25T11:18:40Z

🦋 Changeset detected

Latest commit: 34484b6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@clerk/clerk-js	Minor
@clerk/shared	Minor
@clerk/clerk-react	Minor
@clerk/types	Minor
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
@clerk/remix	Patch
gatsby-plugin-clerk	Patch
@clerk/nextjs	Patch
@clerk/backend	Patch
@clerk/fastify	Patch
@clerk/localizations	Patch
@clerk/clerk-sdk-node	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jit-ci

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

nikosdouvlis · 2023-07-26T07:05:24Z

 type CoreClerkContextWrapperProps = {
 clerk: Clerk;
 children: React.ReactNode;
+ swrConfig?: any;


Could you please explain why we need this?

This is passed from MockClerkProvider down to CoreOrganizationProvider in order to clear swr cache between tests

nikosdouvlis · 2023-07-26T07:11:02Z

 <ClientContext.Provider value={clientCtx}>
 <SessionContext.Provider value={sessionCtx}>
- <OrganizationContext.Provider value={organizationCtx}>
+ <OrganizationProvider {...organizationCtx.value}>


I guess we're treating this differently because of swrConfig... Not sure how I feel about this small discrepancy to be honest - if this is a testing-only issue, we might be able to find a different solution that works for the tests. What was the original issue?

We need a way to clear cache between tests. The recommended way from the SWR docs is this
Wrapping our CoreClerkContextWrapper with will not work. I verified this by logging the results of useSWRConfig inside our useOrganization while tests where running.
My solution was to expose a OrganizationProvider instead of the "low-level" context and include the <SWRConfig/> provider inside of the OrganizationProvider.
☝️ This is now working as expecting.

I see that the cache provider follows the following interface:
interface Cache<Data> { get(key: string): Data | undefined set(key: string, value: Data): void delete(key: string): void keys(): IterableIterator<string> }
Did you try wrapping the test contexts with a SWRConfig that uses a "noop" cache provider, eg:
{ get: () => undefined, set: noop, delete: noop, keys: () => [] }
so that we don't need to reset it between the tests?

I don't see why this would help. It seemed like the useSWRConfig within useOrganization would return the default values, as it couldn't reach the correct provider of SWRConfig.
For example is I was passing <SWRConfig value={{ dedupingInterval: 0 }}/> the useSWRConfig would return {dedupingInterval: 2000} which is the default value

Does this mean that a SWRConfig provider is found deeper in the tree so you cannot override the defaults in the tests?

nikosdouvlis · 2023-07-26T07:34:50Z

 }));
 }
+
+function cacheKey(type: 'userInvitations', user: UserResource, pagination: GetUserOrganizationInvitations) {


Do we care about typing these objects here? Do you think this would be enough? If it's generic enough in can be used in other similar places as well - other hooks using swr for example.
Suggested change
functioncacheKey(type: 'userInvitations',user: UserResource,pagination: GetUserOrganizationInvitations){
functioncacheKey(...keys: Array<string|number|undefined>){

I think you are correct. Although swr@2 seems to automatically parse objects as keys, which will make this function obsolete

We need to make sure to check whether swr uses referential equality checks or if it somehow builds a scalar value by paring the object's values - otherwise we might risk invalidating the cache on every render.

Does this answer your question ?

SokratisVidros · 2023-08-07T10:01:56Z

+ const [paginatedPage, setPaginatedPage] = useState(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);
+
+ // Cache initialPage and initialPageSize until unmount
+ const initialPageRef = useRef(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);


❓ Do we need the last default fallback? Can't we ensure that the initialPage attribute always has a value?

Follow-up question? Do we need a ref for this?

initialPage does not always have a value as someone can simply pass as params the following
useOrganizationList({infinite:true})
Here we want to not use the defaults in general but use the default value of initialPage

SokratisVidros · 2023-08-07T10:14:53Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


❓ What does isomorphic mean in this context? It feels that it's a bit confusing.

I needed a way to introduce parity between the returned values of useSWR and useSWRInfinite.
based on the params received in the hook return the appropriate values

The naming still feels a bit off. I'd just remove the isomorphic prefix.

Sure, i'll update this shortly in a following PR

SokratisVidros · 2023-08-07T10:15:40Z

+ setSize,
+ mutate: userInvitationsInfiniteMutate,
+ } = useSWRInfinite(getInfiniteKey, ({ initialPage, initialPageSize, status }) => {
+ return !clerk.loaded || !user


❓ I believe this hook should trigger the fetch only if triggerInfinite is set to true

It does, you can check getInfiniteKey. The triggering is decided based on the key not the fetcher function. I updated that part and removed the conditional from this place to avoid confusions

SokratisVidros · 2023-08-09T12:11:38Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


The naming still feels a bit off. I'd just remove the isomorphic prefix.

…seOrganizationList` - Improves DX of useOrganizationList

… introduce OrganizationProvider We needed to wrap <OrganizationContext.Provider/> with SWRConfig provider so that useSWR hooks can read from the correct context in tests.

…d pageSize

…nList + expose isLoading and isFetching

clerk-cookie · 2024-08-09T00:01:29Z

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

panteliselef · 2023-07-25T11:18:37Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

This PR

Creates UserOrganizationInvitation and UserOrganizationInvitationResource
Updates useOrganization to return userInvitations
Adds the ability to aggregate the userInvitations when fetched from useOrganization (Very useful from infinite scrolling)

You might be familiar with OrganizationInvitation that already exists in our codebase.

Both UserOrganizationInvitation and OrganizationInvitation describe the same entity. The former is from the perspective of a user where the later from the perspective of an organization.

Exposed API

const{
isLoaded,
organizationList,// Same as before
createOrganization,// Same as before
setActive,// Same as beforeuserInvitations: {
data,
count,
isFetching,
isLoading,
isError,
page,
pageCount,
fetchPage,
fetchNext,
fetchPrevious,
hasNextPage,
hasPreviousPage,},}=useOrganizationList({userInvitations: {page: 1,pageSize: 10,infinite: true,// Aggregate the data or not. Ideal for infinite listskeepPreviousData: true,},});

changeset-bot · 2023-07-25T11:18:40Z

🦋 Changeset detected

Latest commit: 34484b6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@clerk/clerk-js	Minor
@clerk/shared	Minor
@clerk/clerk-react	Minor
@clerk/types	Minor
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
@clerk/remix	Patch
gatsby-plugin-clerk	Patch
@clerk/nextjs	Patch
@clerk/backend	Patch
@clerk/fastify	Patch
@clerk/localizations	Patch
@clerk/clerk-sdk-node	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jit-ci

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

nikosdouvlis · 2023-07-26T07:05:24Z

 type CoreClerkContextWrapperProps = {
 clerk: Clerk;
 children: React.ReactNode;
+ swrConfig?: any;


Could you please explain why we need this?

This is passed from MockClerkProvider down to CoreOrganizationProvider in order to clear swr cache between tests

nikosdouvlis · 2023-07-26T07:11:02Z

 <ClientContext.Provider value={clientCtx}>
 <SessionContext.Provider value={sessionCtx}>
- <OrganizationContext.Provider value={organizationCtx}>
+ <OrganizationProvider {...organizationCtx.value}>


I guess we're treating this differently because of swrConfig... Not sure how I feel about this small discrepancy to be honest - if this is a testing-only issue, we might be able to find a different solution that works for the tests. What was the original issue?

We need a way to clear cache between tests. The recommended way from the SWR docs is this
Wrapping our CoreClerkContextWrapper with will not work. I verified this by logging the results of useSWRConfig inside our useOrganization while tests where running.
My solution was to expose a OrganizationProvider instead of the "low-level" context and include the <SWRConfig/> provider inside of the OrganizationProvider.
☝️ This is now working as expecting.

I see that the cache provider follows the following interface:
interface Cache<Data> { get(key: string): Data | undefined set(key: string, value: Data): void delete(key: string): void keys(): IterableIterator<string> }
Did you try wrapping the test contexts with a SWRConfig that uses a "noop" cache provider, eg:
{ get: () => undefined, set: noop, delete: noop, keys: () => [] }
so that we don't need to reset it between the tests?

I don't see why this would help. It seemed like the useSWRConfig within useOrganization would return the default values, as it couldn't reach the correct provider of SWRConfig.
For example is I was passing <SWRConfig value={{ dedupingInterval: 0 }}/> the useSWRConfig would return {dedupingInterval: 2000} which is the default value

Does this mean that a SWRConfig provider is found deeper in the tree so you cannot override the defaults in the tests?

nikosdouvlis · 2023-07-26T07:34:50Z

 }));
 }
+
+function cacheKey(type: 'userInvitations', user: UserResource, pagination: GetUserOrganizationInvitations) {


Do we care about typing these objects here? Do you think this would be enough? If it's generic enough in can be used in other similar places as well - other hooks using swr for example.
Suggested change
functioncacheKey(type: 'userInvitations',user: UserResource,pagination: GetUserOrganizationInvitations){
functioncacheKey(...keys: Array<string|number|undefined>){

I think you are correct. Although swr@2 seems to automatically parse objects as keys, which will make this function obsolete

We need to make sure to check whether swr uses referential equality checks or if it somehow builds a scalar value by paring the object's values - otherwise we might risk invalidating the cache on every render.

Does this answer your question ?

SokratisVidros · 2023-08-07T10:01:56Z

+ const [paginatedPage, setPaginatedPage] = useState(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);
+
+ // Cache initialPage and initialPageSize until unmount
+ const initialPageRef = useRef(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);


❓ Do we need the last default fallback? Can't we ensure that the initialPage attribute always has a value?

Follow-up question? Do we need a ref for this?

initialPage does not always have a value as someone can simply pass as params the following
useOrganizationList({infinite:true})
Here we want to not use the defaults in general but use the default value of initialPage

SokratisVidros · 2023-08-07T10:14:53Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


❓ What does isomorphic mean in this context? It feels that it's a bit confusing.

I needed a way to introduce parity between the returned values of useSWR and useSWRInfinite.
based on the params received in the hook return the appropriate values

The naming still feels a bit off. I'd just remove the isomorphic prefix.

Sure, i'll update this shortly in a following PR

SokratisVidros · 2023-08-07T10:15:40Z

+ setSize,
+ mutate: userInvitationsInfiniteMutate,
+ } = useSWRInfinite(getInfiniteKey, ({ initialPage, initialPageSize, status }) => {
+ return !clerk.loaded || !user


❓ I believe this hook should trigger the fetch only if triggerInfinite is set to true

It does, you can check getInfiniteKey. The triggering is decided based on the key not the fetcher function. I updated that part and removed the conditional from this place to avoid confusions

SokratisVidros · 2023-08-09T12:11:38Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


The naming still feels a bit off. I'd just remove the isomorphic prefix.

…seOrganizationList` - Improves DX of useOrganizationList

… introduce OrganizationProvider We needed to wrap <OrganizationContext.Provider/> with SWRConfig provider so that useSWR hooks can read from the correct context in tests.

…d pageSize

…nList + expose isLoading and isFetching

clerk-cookie · 2024-08-09T00:01:29Z

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

panteliselef · 2023-07-25T11:18:37Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

This PR

Creates UserOrganizationInvitation and UserOrganizationInvitationResource
Updates useOrganization to return userInvitations
Adds the ability to aggregate the userInvitations when fetched from useOrganization (Very useful from infinite scrolling)

You might be familiar with OrganizationInvitation that already exists in our codebase.

Both UserOrganizationInvitation and OrganizationInvitation describe the same entity. The former is from the perspective of a user where the later from the perspective of an organization.

Exposed API

const{
isLoaded,
organizationList,// Same as before
createOrganization,// Same as before
setActive,// Same as beforeuserInvitations: {
data,
count,
isFetching,
isLoading,
isError,
page,
pageCount,
fetchPage,
fetchNext,
fetchPrevious,
hasNextPage,
hasPreviousPage,},}=useOrganizationList({userInvitations: {page: 1,pageSize: 10,infinite: true,// Aggregate the data or not. Ideal for infinite listskeepPreviousData: true,},});

changeset-bot · 2023-07-25T11:18:40Z

🦋 Changeset detected

Latest commit: 34484b6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@clerk/clerk-js	Minor
@clerk/shared	Minor
@clerk/clerk-react	Minor
@clerk/types	Minor
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
@clerk/remix	Patch
gatsby-plugin-clerk	Patch
@clerk/nextjs	Patch
@clerk/backend	Patch
@clerk/fastify	Patch
@clerk/localizations	Patch
@clerk/clerk-sdk-node	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jit-ci

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

nikosdouvlis · 2023-07-26T07:05:24Z

 type CoreClerkContextWrapperProps = {
 clerk: Clerk;
 children: React.ReactNode;
+ swrConfig?: any;


Could you please explain why we need this?

This is passed from MockClerkProvider down to CoreOrganizationProvider in order to clear swr cache between tests

nikosdouvlis · 2023-07-26T07:11:02Z

 <ClientContext.Provider value={clientCtx}>
 <SessionContext.Provider value={sessionCtx}>
- <OrganizationContext.Provider value={organizationCtx}>
+ <OrganizationProvider {...organizationCtx.value}>


I guess we're treating this differently because of swrConfig... Not sure how I feel about this small discrepancy to be honest - if this is a testing-only issue, we might be able to find a different solution that works for the tests. What was the original issue?

We need a way to clear cache between tests. The recommended way from the SWR docs is this
Wrapping our CoreClerkContextWrapper with will not work. I verified this by logging the results of useSWRConfig inside our useOrganization while tests where running.
My solution was to expose a OrganizationProvider instead of the "low-level" context and include the <SWRConfig/> provider inside of the OrganizationProvider.
☝️ This is now working as expecting.

I see that the cache provider follows the following interface:
interface Cache<Data> { get(key: string): Data | undefined set(key: string, value: Data): void delete(key: string): void keys(): IterableIterator<string> }
Did you try wrapping the test contexts with a SWRConfig that uses a "noop" cache provider, eg:
{ get: () => undefined, set: noop, delete: noop, keys: () => [] }
so that we don't need to reset it between the tests?

I don't see why this would help. It seemed like the useSWRConfig within useOrganization would return the default values, as it couldn't reach the correct provider of SWRConfig.
For example is I was passing <SWRConfig value={{ dedupingInterval: 0 }}/> the useSWRConfig would return {dedupingInterval: 2000} which is the default value

Does this mean that a SWRConfig provider is found deeper in the tree so you cannot override the defaults in the tests?

nikosdouvlis · 2023-07-26T07:34:50Z

 }));
 }
+
+function cacheKey(type: 'userInvitations', user: UserResource, pagination: GetUserOrganizationInvitations) {


Do we care about typing these objects here? Do you think this would be enough? If it's generic enough in can be used in other similar places as well - other hooks using swr for example.
Suggested change
functioncacheKey(type: 'userInvitations',user: UserResource,pagination: GetUserOrganizationInvitations){
functioncacheKey(...keys: Array<string|number|undefined>){

I think you are correct. Although swr@2 seems to automatically parse objects as keys, which will make this function obsolete

We need to make sure to check whether swr uses referential equality checks or if it somehow builds a scalar value by paring the object's values - otherwise we might risk invalidating the cache on every render.

Does this answer your question ?

SokratisVidros · 2023-08-07T10:01:56Z

+ const [paginatedPage, setPaginatedPage] = useState(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);
+
+ // Cache initialPage and initialPageSize until unmount
+ const initialPageRef = useRef(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);


❓ Do we need the last default fallback? Can't we ensure that the initialPage attribute always has a value?

Follow-up question? Do we need a ref for this?

initialPage does not always have a value as someone can simply pass as params the following
useOrganizationList({infinite:true})
Here we want to not use the defaults in general but use the default value of initialPage

SokratisVidros · 2023-08-07T10:14:53Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


❓ What does isomorphic mean in this context? It feels that it's a bit confusing.

I needed a way to introduce parity between the returned values of useSWR and useSWRInfinite.
based on the params received in the hook return the appropriate values

The naming still feels a bit off. I'd just remove the isomorphic prefix.

Sure, i'll update this shortly in a following PR

SokratisVidros · 2023-08-07T10:15:40Z

+ setSize,
+ mutate: userInvitationsInfiniteMutate,
+ } = useSWRInfinite(getInfiniteKey, ({ initialPage, initialPageSize, status }) => {
+ return !clerk.loaded || !user


❓ I believe this hook should trigger the fetch only if triggerInfinite is set to true

It does, you can check getInfiniteKey. The triggering is decided based on the key not the fetcher function. I updated that part and removed the conditional from this place to avoid confusions

SokratisVidros · 2023-08-09T12:11:38Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


The naming still feels a bit off. I'd just remove the isomorphic prefix.

…seOrganizationList` - Improves DX of useOrganizationList

… introduce OrganizationProvider We needed to wrap <OrganizationContext.Provider/> with SWRConfig provider so that useSWR hooks can read from the correct context in tests.

…d pageSize

…nList + expose isLoading and isFetching

clerk-cookie · 2024-08-09T00:01:29Z

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

panteliselef · 2023-07-25T11:18:37Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

This PR

Creates UserOrganizationInvitation and UserOrganizationInvitationResource
Updates useOrganization to return userInvitations
Adds the ability to aggregate the userInvitations when fetched from useOrganization (Very useful from infinite scrolling)

You might be familiar with OrganizationInvitation that already exists in our codebase.

Both UserOrganizationInvitation and OrganizationInvitation describe the same entity. The former is from the perspective of a user where the later from the perspective of an organization.

Exposed API

const{
isLoaded,
organizationList,// Same as before
createOrganization,// Same as before
setActive,// Same as beforeuserInvitations: {
data,
count,
isFetching,
isLoading,
isError,
page,
pageCount,
fetchPage,
fetchNext,
fetchPrevious,
hasNextPage,
hasPreviousPage,},}=useOrganizationList({userInvitations: {page: 1,pageSize: 10,infinite: true,// Aggregate the data or not. Ideal for infinite listskeepPreviousData: true,},});

changeset-bot · 2023-07-25T11:18:40Z

🦋 Changeset detected

Latest commit: 34484b6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@clerk/clerk-js	Minor
@clerk/shared	Minor
@clerk/clerk-react	Minor
@clerk/types	Minor
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
@clerk/remix	Patch
gatsby-plugin-clerk	Patch
@clerk/nextjs	Patch
@clerk/backend	Patch
@clerk/fastify	Patch
@clerk/localizations	Patch
@clerk/clerk-sdk-node	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jit-ci

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

nikosdouvlis · 2023-07-26T07:05:24Z

 type CoreClerkContextWrapperProps = {
 clerk: Clerk;
 children: React.ReactNode;
+ swrConfig?: any;


Could you please explain why we need this?

This is passed from MockClerkProvider down to CoreOrganizationProvider in order to clear swr cache between tests

nikosdouvlis · 2023-07-26T07:11:02Z

 <ClientContext.Provider value={clientCtx}>
 <SessionContext.Provider value={sessionCtx}>
- <OrganizationContext.Provider value={organizationCtx}>
+ <OrganizationProvider {...organizationCtx.value}>


I guess we're treating this differently because of swrConfig... Not sure how I feel about this small discrepancy to be honest - if this is a testing-only issue, we might be able to find a different solution that works for the tests. What was the original issue?

We need a way to clear cache between tests. The recommended way from the SWR docs is this
Wrapping our CoreClerkContextWrapper with will not work. I verified this by logging the results of useSWRConfig inside our useOrganization while tests where running.
My solution was to expose a OrganizationProvider instead of the "low-level" context and include the <SWRConfig/> provider inside of the OrganizationProvider.
☝️ This is now working as expecting.

I see that the cache provider follows the following interface:
interface Cache<Data> { get(key: string): Data | undefined set(key: string, value: Data): void delete(key: string): void keys(): IterableIterator<string> }
Did you try wrapping the test contexts with a SWRConfig that uses a "noop" cache provider, eg:
{ get: () => undefined, set: noop, delete: noop, keys: () => [] }
so that we don't need to reset it between the tests?

I don't see why this would help. It seemed like the useSWRConfig within useOrganization would return the default values, as it couldn't reach the correct provider of SWRConfig.
For example is I was passing <SWRConfig value={{ dedupingInterval: 0 }}/> the useSWRConfig would return {dedupingInterval: 2000} which is the default value

Does this mean that a SWRConfig provider is found deeper in the tree so you cannot override the defaults in the tests?

nikosdouvlis · 2023-07-26T07:34:50Z

 }));
 }
+
+function cacheKey(type: 'userInvitations', user: UserResource, pagination: GetUserOrganizationInvitations) {


Do we care about typing these objects here? Do you think this would be enough? If it's generic enough in can be used in other similar places as well - other hooks using swr for example.
Suggested change
functioncacheKey(type: 'userInvitations',user: UserResource,pagination: GetUserOrganizationInvitations){
functioncacheKey(...keys: Array<string|number|undefined>){

I think you are correct. Although swr@2 seems to automatically parse objects as keys, which will make this function obsolete

We need to make sure to check whether swr uses referential equality checks or if it somehow builds a scalar value by paring the object's values - otherwise we might risk invalidating the cache on every render.

Does this answer your question ?

SokratisVidros · 2023-08-07T10:01:56Z

+ const [paginatedPage, setPaginatedPage] = useState(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);
+
+ // Cache initialPage and initialPageSize until unmount
+ const initialPageRef = useRef(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);


❓ Do we need the last default fallback? Can't we ensure that the initialPage attribute always has a value?

Follow-up question? Do we need a ref for this?

initialPage does not always have a value as someone can simply pass as params the following
useOrganizationList({infinite:true})
Here we want to not use the defaults in general but use the default value of initialPage

SokratisVidros · 2023-08-07T10:14:53Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


❓ What does isomorphic mean in this context? It feels that it's a bit confusing.

I needed a way to introduce parity between the returned values of useSWR and useSWRInfinite.
based on the params received in the hook return the appropriate values

The naming still feels a bit off. I'd just remove the isomorphic prefix.

Sure, i'll update this shortly in a following PR

SokratisVidros · 2023-08-07T10:15:40Z

+ setSize,
+ mutate: userInvitationsInfiniteMutate,
+ } = useSWRInfinite(getInfiniteKey, ({ initialPage, initialPageSize, status }) => {
+ return !clerk.loaded || !user


❓ I believe this hook should trigger the fetch only if triggerInfinite is set to true

It does, you can check getInfiniteKey. The triggering is decided based on the key not the fetcher function. I updated that part and removed the conditional from this place to avoid confusions

SokratisVidros · 2023-08-09T12:11:38Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


The naming still feels a bit off. I'd just remove the isomorphic prefix.

…seOrganizationList` - Improves DX of useOrganizationList

… introduce OrganizationProvider We needed to wrap <OrganizationContext.Provider/> with SWRConfig provider so that useSWR hooks can read from the correct context in tests.

…d pageSize

…nList + expose isLoading and isFetching

clerk-cookie · 2024-08-09T00:01:29Z

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

panteliselef · 2023-07-25T11:18:37Z

Type of change

Packages affected

Description

npm test runs as expected.
npm run build runs as expected.

This PR

Creates UserOrganizationInvitation and UserOrganizationInvitationResource
Updates useOrganization to return userInvitations
Adds the ability to aggregate the userInvitations when fetched from useOrganization (Very useful from infinite scrolling)

You might be familiar with OrganizationInvitation that already exists in our codebase.

Both UserOrganizationInvitation and OrganizationInvitation describe the same entity. The former is from the perspective of a user where the later from the perspective of an organization.

Exposed API

const{
isLoaded,
organizationList,// Same as before
createOrganization,// Same as before
setActive,// Same as beforeuserInvitations: {
data,
count,
isFetching,
isLoading,
isError,
page,
pageCount,
fetchPage,
fetchNext,
fetchPrevious,
hasNextPage,
hasPreviousPage,},}=useOrganizationList({userInvitations: {page: 1,pageSize: 10,infinite: true,// Aggregate the data or not. Ideal for infinite listskeepPreviousData: true,},});

changeset-bot · 2023-07-25T11:18:40Z

🦋 Changeset detected

Latest commit: 34484b6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages

Name	Type
@clerk/clerk-js	Minor
@clerk/shared	Minor
@clerk/clerk-react	Minor
@clerk/types	Minor
@clerk/chrome-extension	Patch
@clerk/clerk-expo	Patch
@clerk/remix	Patch
gatsby-plugin-clerk	Patch
@clerk/nextjs	Patch
@clerk/backend	Patch
@clerk/fastify	Patch
@clerk/localizations	Patch
@clerk/clerk-sdk-node	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

jit-ci

✅ Great news! Jit hasn't found any security issues in your PR. Good Job! 🏆

nikosdouvlis · 2023-07-26T07:05:24Z

 type CoreClerkContextWrapperProps = {
 clerk: Clerk;
 children: React.ReactNode;
+ swrConfig?: any;


Could you please explain why we need this?

This is passed from MockClerkProvider down to CoreOrganizationProvider in order to clear swr cache between tests

nikosdouvlis · 2023-07-26T07:11:02Z

 <ClientContext.Provider value={clientCtx}>
 <SessionContext.Provider value={sessionCtx}>
- <OrganizationContext.Provider value={organizationCtx}>
+ <OrganizationProvider {...organizationCtx.value}>


I guess we're treating this differently because of swrConfig... Not sure how I feel about this small discrepancy to be honest - if this is a testing-only issue, we might be able to find a different solution that works for the tests. What was the original issue?

We need a way to clear cache between tests. The recommended way from the SWR docs is this
Wrapping our CoreClerkContextWrapper with will not work. I verified this by logging the results of useSWRConfig inside our useOrganization while tests where running.
My solution was to expose a OrganizationProvider instead of the "low-level" context and include the <SWRConfig/> provider inside of the OrganizationProvider.
☝️ This is now working as expecting.

I see that the cache provider follows the following interface:
interface Cache<Data> { get(key: string): Data | undefined set(key: string, value: Data): void delete(key: string): void keys(): IterableIterator<string> }
Did you try wrapping the test contexts with a SWRConfig that uses a "noop" cache provider, eg:
{ get: () => undefined, set: noop, delete: noop, keys: () => [] }
so that we don't need to reset it between the tests?

I don't see why this would help. It seemed like the useSWRConfig within useOrganization would return the default values, as it couldn't reach the correct provider of SWRConfig.
For example is I was passing <SWRConfig value={{ dedupingInterval: 0 }}/> the useSWRConfig would return {dedupingInterval: 2000} which is the default value

Does this mean that a SWRConfig provider is found deeper in the tree so you cannot override the defaults in the tests?

nikosdouvlis · 2023-07-26T07:34:50Z

 }));
 }
+
+function cacheKey(type: 'userInvitations', user: UserResource, pagination: GetUserOrganizationInvitations) {


Do we care about typing these objects here? Do you think this would be enough? If it's generic enough in can be used in other similar places as well - other hooks using swr for example.
Suggested change
functioncacheKey(type: 'userInvitations',user: UserResource,pagination: GetUserOrganizationInvitations){
functioncacheKey(...keys: Array<string|number|undefined>){

I think you are correct. Although swr@2 seems to automatically parse objects as keys, which will make this function obsolete

We need to make sure to check whether swr uses referential equality checks or if it somehow builds a scalar value by paring the object's values - otherwise we might risk invalidating the cache on every render.

Does this answer your question ?

SokratisVidros · 2023-08-07T10:01:56Z

+ const [paginatedPage, setPaginatedPage] = useState(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);
+
+ // Cache initialPage and initialPageSize until unmount
+ const initialPageRef = useRef(shouldUseDefaults ? 1 : userInvitations?.initialPage ?? 1);


❓ Do we need the last default fallback? Can't we ensure that the initialPage attribute always has a value?

Follow-up question? Do we need a ref for this?

initialPage does not always have a value as someone can simply pass as params the following
useOrganizationList({infinite:true})
Here we want to not use the defaults in general but use the default value of initialPage

SokratisVidros · 2023-08-07T10:14:53Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


❓ What does isomorphic mean in this context? It feels that it's a bit confusing.

I needed a way to introduce parity between the returned values of useSWR and useSWRInfinite.
based on the params received in the hook return the appropriate values

The naming still feels a bit off. I'd just remove the isomorphic prefix.

Sure, i'll update this shortly in a following PR

SokratisVidros · 2023-08-07T10:15:40Z

+ setSize,
+ mutate: userInvitationsInfiniteMutate,
+ } = useSWRInfinite(getInfiniteKey, ({ initialPage, initialPageSize, status }) => {
+ return !clerk.loaded || !user


❓ I believe this hook should trigger the fetch only if triggerInfinite is set to true

It does, you can check getInfiniteKey. The triggering is decided based on the key not the fetcher function. I updated that part and removed the conditional from this place to avoid confusions

SokratisVidros · 2023-08-09T12:11:38Z

+ return userInvitationsData?.total_count ?? 0;
+ }, [triggerInfinite, userInvitationsDataInfinite, userInvitationsData]);
+
+ const isomorphicIsLoading = triggerInfinite ? userInvitationsLoadingInfinite : userInvitationsLoading;


The naming still feels a bit off. I'd just remove the isomorphic prefix.

…seOrganizationList` - Improves DX of useOrganizationList

… introduce OrganizationProvider We needed to wrap <OrganizationContext.Provider/> with SWRConfig provider so that useSWR hooks can read from the correct context in tests.

…d pageSize

…nList + expose isLoading and isFetching

clerk-cookie · 2024-08-09T00:01:29Z

This PR has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

panteliselef self-assigned this Jul 25, 2023

panteliselef requested review from SokratisVidros and nikosdouvlis as code owners July 25, 2023 11:18

clerk-cookie added types clerk-js labels Jul 25, 2023

panteliselef force-pushed the ORG-59 branch from 47d01f5 to 785189eCompare July 25, 2023 11:23

jit-ciBot reviewed Jul 25, 2023
View reviewed changes

panteliselef force-pushed the ORG-59 branch 3 times, most recently from a5c0c58 to 6f059cfCompare July 25, 2023 13:00

clerk-cookie added the react label Jul 25, 2023

nikosdouvlis reviewed Jul 26, 2023
View reviewed changes

panteliselef force-pushed the ORG-59 branch 2 times, most recently from bf065dd to 9a215b6Compare July 26, 2023 13:48

panteliselef changed the title ~~Introduce UserOrganizationInvitation~~[WIP] UserOrganizationInvitation & useOrganizationListJul 26, 2023

panteliselef force-pushed the ORG-59 branch from f853e99 to 28ea166Compare August 3, 2023 07:39

clerk-cookie removed the types label Aug 3, 2023

panteliselef force-pushed the ORG-59 branch from 28ea166 to 3dde4ccCompare August 3, 2023 11:19

clerk-cookie added the types label Aug 3, 2023

panteliselef force-pushed the ORG-59 branch from 3dde4cc to aebc06cCompare August 3, 2023 11:39

panteliselef changed the title ~~[WIP] UserOrganizationInvitation & useOrganizationList~~ORG-83 UserOrganizationInvitation & useOrganizationListAug 3, 2023

panteliselef force-pushed the ORG-59 branch 3 times, most recently from 8d2af63 to 7ce7885Compare August 4, 2023 18:32

panteliselef changed the title ~~ORG-83 UserOrganizationInvitation & useOrganizationList~~ORG-83 userInvitations in useOrganizationListAug 4, 2023

panteliselef requested review from chanioxaris, desiprisg and mzhong9723 August 4, 2023 18:46

SokratisVidros suggested changes Aug 7, 2023
View reviewed changes

panteliselef force-pushed the ORG-59 branch from a3ffbde to 747a624Compare August 8, 2023 10:01

panteliselef requested review from SokratisVidros and nikosdouvlis August 8, 2023 10:02

panteliselef changed the title ~~ORG-83 userInvitations in useOrganizationList~~🚨1️⃣ORG-83 userInvitations in useOrganizationListAug 9, 2023

SokratisVidros approved these changes Aug 9, 2023
View reviewed changes

panteliselef added 10 commits August 9, 2023 15:32

feat(shared): Support infinite fetching for userInvitations from `u…
1ec0360
…seOrganizationList` - Improves DX of useOrganizationList

chore(clerk-js,shared,clerk-react): Deprecate OrganizationContext and…
8b84a57
… introduce OrganizationProvider We needed to wrap <OrganizationContext.Provider/> with SWRConfig provider so that useSWR hooks can read from the correct context in tests.

feat(shared,types,clerk-js): Abstract limit and offset behind page an…
ee1b11b
…d pageSize

feat(shared): Add unstable_mutate, status as params to useOrganizatio…
8dcef3b
…nList + expose isLoading and isFetching

chore(repo): Add changeset
02e9d73

test(clerk-js): Introduce tests for useCoreOrganizationList
0d43a2d

chore(repo): Update lock file
e107cc4

fix(shared): Fixes after review
affc027

test(clerk-js): Fixes after review
29dc7ef

chore(clerk-js): Move convertPageToOffset to utils
34484b6

panteliselef force-pushed the ORG-59 branch from 747a624 to 34484b6Compare August 9, 2023 12:34

panteliselef changed the title ~~🚨1️⃣ORG-83 userInvitations in useOrganizationList~~ORG-83 userInvitations in useOrganizationListAug 9, 2023

panteliselef merged commit 4ea30e8 into mainAug 9, 2023

panteliselef deleted the ORG-59 branch August 9, 2023 12:45

clerk-cookie mentioned this pull request Aug 9, 2023
Version Packages #1546
Merged

clerk locked as resolved and limited conversation to collaborators Aug 9, 2024