feat(clerk-js): Track fapi requests triggered by UI components #5299
Conversation
🦋 Changeset detectedLatest commit: 777380f The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
…en-they-originate-from
|
|
||
| export const usageByUIComponents = createStore(0); | ||
|
|
||
| const isThenable = (value: unknown): value is Promise<unknown> => { |
There was a problem hiding this comment.
| const isThenable = (value: unknown): value is Promise<unknown> => { | |
| const isPromise = (value: unknown): value is Promise<unknown> => { |
🙃 I'd suggest updating the name to match the type predicate as well, thenable was a bit hard to understand at a first glance
| return !!value && typeof (value as any).then === 'function'; | ||
| }; | ||
|
|
||
| export const makeUICaller = <T>(resource: T): T => { |
There was a problem hiding this comment.
| export const makeUICaller = <T>(resource: T): T => { | |
| /** @internal **/ | |
| export const makeUICaller = <T>(resource: T): T => { |
🙃 We could add JSDocs to indicate for external contributors that this is a tooling for internal usage
| const clientCtx = React.useMemo(() => ({ value: client }), [client]); | ||
| const sessionCtx = React.useMemo(() => ({ value: session }), [session]); | ||
| const userCtx = React.useMemo(() => ({ value: user }), [user]); | ||
| const clerkCtx = React.useMemo(() => ({ value: makeUICaller(clerk) }), []); |
There was a problem hiding this comment.
💡Not against this approach, mostly curious regarding other options - have you thought of forwarding a property to Clerk if it loads on a React context, and then accessing this on the FAPI client?
There was a problem hiding this comment.
Can you share more about this. Clerk is already loaded when we mount the providers and the UI components.
There was a problem hiding this comment.
Once the CoreClerkContextWrapper loads, it sets a property in Clerk that can be accessed on the fapiClient
if (clerk.hasUIListeners) {
searchParams.append('_clerk_ui_triggered', 'true');
}
There was a problem hiding this comment.
I think this increases the margin of faulty detections by a lot, since all requests triggered outside of a UI component will be flagged incorrectly while a UI component is mounted.
Description
This PR decorates all requests towards the Frontend API with a new query param
_clerk_ui_triggered=truewhich indicates that the request originated from our UI components.The solution proposed by this PR, uses a proxy to wrap all resources (e.g. user and client) before passing them to their respective react context. The proxy will detect any function usage and will flag it as "used by UI", so that fapiClient can use this to attach the new query param. This solution allows to not make any changes to public API or affect the implementation of our resources.
Attention: This solution does not guarantee that the detection will work every time since it uses a stack to determine if the request should be flagged as "used by UI". If a request from a custom flow fires while there is at least one pending request triggered by a UI component, the former will be wrongly flagged as "used by UI". This is acceptable because we are using the query param for analytics, and such errors will not affect the performance or the behaviour of the application.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change