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 before
    userInvitations: {
      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 lists
      keepPreviousData: 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

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

function cacheKey(...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 785189e Compare July 25, 2023 11:23

jit-ci Bot reviewed Jul 25, 2023

View reviewed changes

panteliselef force-pushed the ORG-59 branch 3 times, most recently from a5c0c58 to 6f059cf Compare 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 9a215b6 Compare July 26, 2023 13:48

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

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

clerk-cookie removed the types label Aug 3, 2023

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

clerk-cookie added the types label Aug 3, 2023

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

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

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

panteliselef changed the title ~~ORG-83 UserOrganizationInvitation & useOrganizationList~~ ORG-83 userInvitations in useOrganizationList Aug 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 747a624 Compare 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 useOrganizationList Aug 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 34484b6 Compare August 9, 2023 12:34

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

panteliselef merged commit 4ea30e8 into main Aug 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

	function cacheKey(type: 'userInvitations', user: UserResource, pagination: GetUserOrganizationInvitations) {
	function cacheKey(...keys: Array<string \| number \| undefined>) {

Uh oh!

Conversation

panteliselef commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Packages affected

Description

This PR

Exposed API

Uh oh!

changeset-bot Bot commented Jul 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

jit-ci Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

panteliselef Aug 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clerk-cookie commented Aug 9, 2024

Uh oh!

Reviewers

Assignees

panteliselef commented Jul 25, 2023 •

edited

Loading

changeset-bot Bot commented Jul 25, 2023 •

edited

Loading

panteliselef Aug 8, 2023 •

edited

Loading