feat(shared): Organization hooks with `setData` and `revalidate` by panteliselef · Pull Request #1745 · clerk/javascript

panteliselef · 2023-09-19T12:07:53Z

Description

Issue:
So far our organization hooks did not officially support mutations. Developers did not have a way to alternate the cache or simply revalidate on demand.

Background:
For invitationList (from useOrganization) we didn't encounter such issues, because we are forcing an update under the hood when lastOrganizationMember changes. In the case of deleted resource, the hook would refetch and the deleted items would disappear from the cache. In the new flows we introduce with we need to update the cache in place and avoid refetching, in order to avoid layout shifts.

We want developers to explicitly call either revalidate or setData after a resource is updated and let them decide which method best suits their use-case.

This PR exposes the mutate function for both useSWR and useSWR infinite based on the parameters a developer provided.

✅ Wrapper for swr mutate, allows us to have a consistent API without relying on swr not to break our customers.
✅ Wrapper for swr mutate, allows us to move to other internal implementation with ease.
✅ Can alternate local cache with setData
✅ Revalidate on demand with revalidate. (for example after creating an organization or accepting an invite)
❌ mutate is what swr describes as Bound Mutate and does not use useSWRMutation under the hood.

As part of the PR, we replaced all usages of unstable__mutate from our update useOrganization and useOrganizationList hooks within ClerkJS Components

setData will not trigger a revalidation, it will simply replace the passed data with data already in cache
revalidate does not accept any parameter and will perform a revalidation causing a request to fire.
- For infinite pagination it will refetch all the cached pages.

Note: I attempted to optimize the revalidation of all pages with infinite by traversing the cache that is only affected by the updated resource.
There is also the case where this approach would result in duplicate keys when rendering lists.
This seems to be quite complicated as swr keeps n+1 records for infinite. That means the if 2 pages are fetched, 3 cache are created. 2 keys directly match the pages data and the 3rd is a "parent" record with its own key and it contains copies of the data of the 2 pages. Manually updating any of them, means we need to update the other as well.

Checklist

npm test runs as expected.
npm run build runs as expected.
(If applicable) JSDoc comments have been added or updated for any package exports
(If applicable) Documentation has been updated
This will be merged after the PR goes to production [WIP] update: Mention mutate in useOrganizationList clerk-docs#310

Type of change

Packages affected

changeset-bot · 2023-09-19T12:07:56Z

🦋 Changeset detected

Latest commit: d760fbc

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

This PR includes changesets to release 11 packages

Name	Type
@clerk/shared	Minor
@clerk/backend	Patch
@clerk/clerk-js	Patch
@clerk/clerk-expo	Patch
@clerk/fastify	Patch
@clerk/nextjs	Patch
@clerk/clerk-react	Patch
@clerk/remix	Patch
@clerk/clerk-sdk-node	Patch
gatsby-plugin-clerk	Patch
@clerk/chrome-extension	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

panteliselef · 2023-09-20T08:33:33Z

!snapshot

clerk-cookie · 2023-09-20T08:36:28Z

Hey @panteliselef - the snapshot version command generated the following package versions:

Package	Version
@clerk/backend	0.29.1-snapshot.3ef2f60
@clerk/chrome-extension	0.4.0-snapshot.3ef2f60
@clerk/clerk-js	4.58.0-snapshot.3ef2f60
@clerk/clerk-expo	0.19.2-snapshot.3ef2f60
@clerk/fastify	0.6.7-snapshot.3ef2f60
gatsby-plugin-clerk	4.4.8-snapshot.3ef2f60
@clerk/localizations	1.26.0-snapshot.3ef2f60
@clerk/nextjs	4.24.0-snapshot.3ef2f60
@clerk/clerk-react	4.25.0-snapshot.3ef2f60
@clerk/remix	2.10.3-snapshot.3ef2f60
@clerk/clerk-sdk-node	4.12.6-snapshot.3ef2f60
@clerk/shared	0.22.1-snapshot.3ef2f60
@clerk/types	3.52.0-snapshot.3ef2f60

Tip: use the snippet copy button below to quickly install the required packages.

# @clerk/backend
npm i @clerk/backend@0.29.1-snapshot.3ef2f60

# @clerk/chrome-extension
npm i @clerk/chrome-extension@0.4.0-snapshot.3ef2f60

# @clerk/clerk-js
npm i @clerk/clerk-js@4.58.0-snapshot.3ef2f60

# @clerk/clerk-expo
npm i @clerk/clerk-expo@0.19.2-snapshot.3ef2f60

# @clerk/fastify
npm i @clerk/fastify@0.6.7-snapshot.3ef2f60

# gatsby-plugin-clerk
npm i gatsby-plugin-clerk@4.4.8-snapshot.3ef2f60

# @clerk/localizations
npm i @clerk/localizations@1.26.0-snapshot.3ef2f60

# @clerk/nextjs
npm i @clerk/nextjs@4.24.0-snapshot.3ef2f60

# @clerk/clerk-react
npm i @clerk/clerk-react@4.25.0-snapshot.3ef2f60

# @clerk/remix
npm i @clerk/remix@2.10.3-snapshot.3ef2f60

# @clerk/clerk-sdk-node
npm i @clerk/clerk-sdk-node@4.12.6-snapshot.3ef2f60

# @clerk/shared
npm i @clerk/shared@0.22.1-snapshot.3ef2f60

# @clerk/types
npm i @clerk/types@3.52.0-snapshot.3ef2f60

SokratisVidros · 2023-09-22T09:46:22Z

@panteliselef can you please elaborate on this? So far we have been using SWR internally for our own convenience. Did customers ask for this? In Clerk the preferred way to do mutations is via the Clerk models that hold all the mutating methods.

panteliselef · 2023-09-22T10:18:15Z

@panteliselef can you please elaborate on this? So far we have been using SWR internally for our own convenience. Did customers ask for this? In Clerk the preferred way to do mutations is via the Clerk models that hold all the mutating methods.

Yes, there were customer requests, I'll share more internally.
I see SWR mutate as a way to force a revalidation or manually populate the cache for the collection of data.
This does not change the way someone will call the mutation methods that update a resource. On the contrary those 2 mechanism will work together, let me give an example:

A developer lists the suggestions of a user and has an accept button for each of those. I'd image the onClick function would contain something like this.

const updatedSuggestion = await suggestion.accept()

userSuggestions.mutate.(oldCache => replaceUpdatedItem(updatedSuggestion, oldCache), {
  // Since `accept` gives back the updated information,
  // we don't need to revalidate here.
  revalidate: false,
})

SokratisVidros · 2023-09-22T13:25:36Z

I see. Let's take it offline. We should be able to support this use case using the Clerk hooks API surface.

SokratisVidros · 2023-09-28T13:43:54Z

+'@clerk/shared': patch
+---
+
+Expose mutate for paginated lists of data in organization hooks.


❓ Is this description up to date?

I just updated this one

SokratisVidros · 2023-10-09T13:57:57Z

+  }
+
  return itemsInfinitePages.map(item => {
+    if (typeof item === 'undefined') {


Suggested change

if (typeof item === 'undefined') {

if (!item) { return }

Gentle reminder ☝️

SokratisVidros · 2023-10-09T14:05:22Z

+          revalidate: false,
+        });
+
+  const revalidate = triggerInfinite ? () => swrInfiniteMutate() : () => swrMutate();


As described in the PR, the revalidate will refetch all loaded pages when triggerInfinite is true. This behavior can be disabled.

What's the use case where we need to reload all pages?

regarding refetchAll, this only works when swr auto revalidates, unfortunately when mutate is called explicitly then all pages will be revalidated sequentially

@SokratisVidros As promised I checked also the implementation of react-query. Seems like it is also following the same pattern, more about it here.

Not a fan at all. Do we have a use case for this already are can we defer it?

SokratisVidros · 2023-10-09T14:09:01Z

+  data?: CData | ((currentData?: CData) => Promise<undefined | CData> | undefined | CData),
+) => Promise<CData | undefined>;
+
+export type PaginatedResources<T = unknown, Infinite = false, ArrayOrPaginated = unknown> = {


❓ Regardless of infinite pagination or not, the resource data should be stored in a single format in the JS struct. They should always be stored in a paginated struct. Is there a reason we need to support plain arrays? If not, this decision will drastically simplify the following hook declarations.

I think this is a great catch. Probably I will ditching ArrayOrPaginated completely. Trying to remember why did what i did, but it's been a while so my only theory is that i got destructed by the types return from the resource methods like organization?.getMemberships which can return both an array or the Paginated Object.

Our hooks should only consume those methods and their resolved values to always be Paginated Objects so i will give this another look and probably drop this.

SokratisVidros · 2023-10-31T15:47:23Z

+  }
+
  return itemsInfinitePages.map(item => {
+    if (typeof item === 'undefined') {


Gentle reminder ☝️

dimkl · 2023-10-31T17:39:09Z

@panteliselef We have changed the process of the v4 hotfixing. Please rebase this PR with main as the base branch and after it's closed, use the backport script to open a hotfix from main-v4.
Also if something that it's not related a fixing an issue in v4 and it can wait until v5, merge it in main and don't backport it.

dimkl · 2023-11-02T08:49:03Z

+  revalidate: () => Promise<void>;
+  setData: Infinite extends true
+    ? // Array of pages of data
+      CacheSetter<(ClerkPaginatedResponse<T> | undefined)[]>


❓ @panteliselef shouldn't this be CacheSetter<(ClerkPaginatedResponse<T>[] | undefined)> ?

@dimkl a "page" in swr can be either undefined or has a value of type ClerkPaginatedResponse

* feat(shared): Expose stable mutate * chore(clerk-js): OrganizationList and switcher with stable mutate * chore(clerk-js): OrganizationProfile with stable mutate * fix(shared): Replace mutate with setCache and revalidate * fix(clerk-js): Replace mutate with setCache and revalidate * chore(clerk-js,shared): Replace setCache with setData * fix(shared): Remove unnecessary complexity in types * chore(clerk-js): Update after rebase * chore(shared): Add comments with context * chore(shared): Update changeset (cherry picked from commit 743c4d2)

…) (#2013) * feat(shared): Expose stable mutate * chore(clerk-js): OrganizationList and switcher with stable mutate * chore(clerk-js): OrganizationProfile with stable mutate * fix(shared): Replace mutate with setCache and revalidate * fix(clerk-js): Replace mutate with setCache and revalidate * chore(clerk-js,shared): Replace setCache with setData * fix(shared): Remove unnecessary complexity in types * chore(clerk-js): Update after rebase * chore(shared): Add comments with context * chore(shared): Update changeset (cherry picked from commit 743c4d2) Co-authored-by: panteliselef <panteliselef@outlook.com>

panteliselef requested a review from a team as a code owner September 19, 2023 12:07

panteliselef self-assigned this Sep 19, 2023

clerk-cookie added the clerk-js label Sep 19, 2023

panteliselef mentioned this pull request Sep 19, 2023

[WIP] update: Mention mutate in useOrganizationList clerk/clerk-docs#310

Closed

panteliselef force-pushed the ORG-222 branch from f6a0374 to 3ef2f60 Compare September 20, 2023 08:11

panteliselef changed the title ~~[WIP] feat(shared): Expose stable mutate + replace unstable__mutate~~ feat(shared): Organization hooks with stable mutate Sep 20, 2023

panteliselef force-pushed the ORG-222 branch from 3ef2f60 to 04ce85c Compare September 21, 2023 07:42

panteliselef changed the title ~~feat(shared): Organization hooks with stable mutate~~ [WIP] feat(shared): Organization hooks with stable mutate Sep 26, 2023

panteliselef force-pushed the ORG-222 branch 2 times, most recently from e12b2d1 to 647a5ea Compare September 27, 2023 09:53

panteliselef changed the title ~~[WIP] feat(shared): Organization hooks with stable mutate~~ feat(shared): Organization hooks with setCache and revalidate Sep 27, 2023

panteliselef force-pushed the ORG-222 branch 3 times, most recently from a2fdb57 to 7a3edc9 Compare October 2, 2023 13:03

panteliselef changed the title ~~feat(shared): Organization hooks with setCache and revalidate~~ feat(shared): Organization hooks with setData and revalidate Oct 3, 2023

SokratisVidros suggested changes Oct 9, 2023

View reviewed changes

panteliselef force-pushed the ORG-222 branch from 7a3edc9 to 34d5e52 Compare October 12, 2023 13:30

panteliselef requested a review from SokratisVidros October 12, 2023 13:35

panteliselef force-pushed the ORG-222 branch from 34d5e52 to a17ac5c Compare October 18, 2023 11:18

panteliselef changed the base branch from main to main-v4 October 25, 2023 18:50

panteliselef force-pushed the ORG-222 branch 3 times, most recently from f98bc23 to b2c6c49 Compare October 30, 2023 07:39

SokratisVidros approved these changes Oct 31, 2023

View reviewed changes

panteliselef changed the base branch from main-v4 to main November 1, 2023 09:09

panteliselef requested a review from a team November 1, 2023 09:09

panteliselef added 8 commits November 1, 2023 12:31

feat(shared): Expose stable mutate

e20e16a

chore(clerk-js): OrganizationList and switcher with stable mutate

870b9d4

chore(clerk-js): OrganizationProfile with stable mutate

612268a

fix(shared): Replace mutate with setCache and revalidate

60474fd

fix(clerk-js): Replace mutate with setCache and revalidate

deb7ea4

chore(clerk-js,shared): Replace setCache with setData

bf36da8

fix(shared): Remove unnecessary complexity in types

67d3c1f

chore(clerk-js): Update after rebase

31645dc

panteliselef force-pushed the ORG-222 branch from b2c6c49 to 31645dc Compare November 1, 2023 10:37

chore(shared): Add comments with context

06a7d7d

panteliselef added this pull request to the merge queue Nov 2, 2023

panteliselef removed this pull request from the merge queue due to a manual request Nov 2, 2023

chore(shared): Update changeset

d760fbc

panteliselef force-pushed the ORG-222 branch from 89b58f4 to d760fbc Compare November 2, 2023 08:30

panteliselef added this pull request to the merge queue Nov 2, 2023

panteliselef removed this pull request from the merge queue due to a manual request Nov 2, 2023

panteliselef added this pull request to the merge queue Nov 2, 2023

dimkl reviewed Nov 2, 2023

View reviewed changes

Merged via the queue into main with commit 743c4d2 Nov 2, 2023

panteliselef deleted the ORG-222 branch November 2, 2023 09:01

clerk-cookie mentioned this pull request Nov 2, 2023

feat(shared): Organization hooks with setData and revalidate (#1745) #2013

Merged

clerk-cookie mentioned this pull request Nov 3, 2023

Version Packages (alpha-v5) #2000

Merged

LekoArts mentioned this pull request Mar 13, 2024

fix(themes): Add tslib and @clerk/types as dependencies to @clerk/themes (#2665) #2987

Merged

LekoArts mentioned this pull request Aug 2, 2024

chore(repo): Correct changeset #3883

Merged

Uh oh!

Conversation

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

Uh oh!

Description

Checklist

Type of change

Packages affected

Uh oh!

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

Uh oh!

🦋 Changeset detected

Uh oh!

panteliselef commented Sep 20, 2023

Uh oh!

clerk-cookie commented Sep 20, 2023

Uh oh!

SokratisVidros commented Sep 22, 2023

Uh oh!

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

Uh oh!

Uh oh!

SokratisVidros commented Sep 22, 2023

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dimkl commented Oct 31, 2023

Uh oh!

Uh oh!

Uh oh!

dimkl Nov 2, 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!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

panteliselef commented Sep 19, 2023 •

edited

Loading

changeset-bot Bot commented Sep 19, 2023 •

edited

Loading

panteliselef commented Sep 22, 2023 •

edited

Loading

dimkl Nov 2, 2023 •

edited

Loading