feat(nextjs): Read dynamic keys from clerkClient within middleware runtime - #3617
Conversation
|
| return clerkClient; | ||
| }; | ||
|
|
||
| const clerkClient: ClerkClient & typeof clerkClientForRequest = Object.assign( |
There was a problem hiding this comment.
Must update docs to call clerkClient() instead of clerkClient https://clerk.com/docs/references/nextjs/route-handlers#interact-with-clerks-backend-api
7f14956 to
23f837e
Compare
| let handlerResult: Response = NextResponse.next(); | ||
| try { | ||
| handlerResult = | ||
| (await clerkClientStorage.run( |
There was a problem hiding this comment.
It was necessary to place another ALS context wrapped around the returned handler, otherwise clerkClient calls within the middleware handler (👇🏻 ) wouldn't have access to the store:
export default clerkMiddleware((auth)=> {
clerkClient().users().get("...")
})
23f837e to
1b83516
Compare
| * @deprecated | ||
| * This singleton is deprecated and will be removed in a future release. Please use `clerkClient()` as a function instead. | ||
| */ | ||
| const clerkClientSingletonProxy = new Proxy(clerkClientSingleton, { |
There was a problem hiding this comment.
Calling out for JS help 🙇🏻 We want to log a deprecation warning only if clerkClient gets called as a singleton.
The Proxy is being placed around the singleton reference here, but in line 90, it's properties are being merged with clerkClient() on Object.assign, therefore the deprecation warnings is being logged for both cases.
It's important to keep exporting clerkClient and allow it to be either a singleton or a function but to do so, it requires that Object.assign bit - is there another way to merge those properties while performing a proxy for the deprecation warning?
There was a problem hiding this comment.
Do you mean that the usage of Object.assign() is causing the deprecation warning to be logged? Or because we're using the proxied client object under the hood for the non-dynamic keys case, it logs for both:
clerkClient().users.getUser()
clerkClient.users.getUser()
There was a problem hiding this comment.
It's logging for both cases, and I think this is due to Object.assign since it's executing Proxy.get for every single property.
46e5bbf to
7332037
Compare
1b83516 to
152212d
Compare
| get(target, prop, receiver) { | ||
| deprecated('clerkClient object', 'Use `clerkClient()` as a function instead.'); | ||
|
|
||
| return Reflect.get(target, prop, receiver); | ||
| }, |
There was a problem hiding this comment.
How does this work with nested properties? E.g. if I access clerkClient.users.getUser() will this still log the deprecation warning?
152212d to
21a4f1e
Compare
a3fc154 to
7fd9a0f
Compare
7fd9a0f to
4a4b66c
Compare
| await app.teardown(); | ||
| }); | ||
|
|
||
| // test('redirects to `signInUrl` on `auth.protect`') |
There was a problem hiding this comment.
Still have to implement those tests here, leaving this as a WIP, will continue to work today/tomorrow!
49275a8 to
4a4b66c
Compare
496de39 to
b101927
Compare
| const PORT = 4199; | ||
|
|
||
| test.skip('Client handshake @generic', () => { | ||
| test.describe('Client handshake @generic', () => { |
There was a problem hiding this comment.
@nikosdouvlis I've went ahead and removed the skip from handshake tests here.
2677ce2 to
cda14e6
Compare
cda14e6 to
aa18e35
Compare
3 participants
Description
Resolves SDK-1253, SDK-1811, SDK-1770
PR on top of #3525 to facilitate review on
clerkClientchanges.Refactoring
clerkClientto be called as a function, instead of singleton, in order to read request data. It reads based on the following:clerkClientgets accessed on middleware runtime, then use ALS store to access dynamic keys dataclerkClientgets accessed on application server, then useNextRequestto access dynamic keys dataChecklist
npm testruns as expected.npm run buildruns as expected.Type of change