-
Notifications
You must be signed in to change notification settings - Fork 451
feat(backend, clerk-js): Deprecate metadata updates in user update methods #8587
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
16fdd04
bf5b092
aebfef5
926d666
c4ccc16
34c0df9
34c3fc8
e0df27e
4cf3a28
d63df08
90e5e32
b30b0fa
a6fc584
0e1a541
a458bb9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| --- | ||
| '@clerk/backend': minor | ||
| '@clerk/clerk-js': patch | ||
| '@clerk/shared': patch | ||
| --- | ||
|
|
||
| Deprecate metadata updates via `clerkClient.users.updateUser` and `user.update()` to mirror the deprecation of `publicMetadata`, `privateMetadata`, and `unsafeMetadata` on `PATCH /v1/users/{userId}` and `PATCH /v1/me`. | ||
|
|
||
| `@clerk/backend`: add `clerkClient.users.replaceUserMetadata(userId, params)` for full-replacement updates. Deprecate the `publicMetadata`, `privateMetadata`, and `unsafeMetadata` parameters on `clerkClient.users.updateUser` — use `updateUserMetadata` for partial updates (deep merge) or `replaceUserMetadata` for full replacement. | ||
|
|
||
| `@clerk/clerk-js`: deprecate `unsafeMetadata` on `user.update()`. Use `user.updateMetadata({ unsafeMetadata })` for partial updates (deep merge) instead. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,179 @@ | ||
| import { http, HttpResponse } from 'msw'; | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
|
|
||
| import { server, validateHeaders } from '../../mock-server'; | ||
| import { createBackendApiClient } from '../factory'; | ||
|
|
||
| describe('UserAPI', () => { | ||
| const apiClient = createBackendApiClient({ | ||
| apiUrl: 'https://api.clerk.test', | ||
| secretKey: 'deadbeef', | ||
| }); | ||
|
|
||
| const mockUserResponse = { | ||
| object: 'user', | ||
| id: 'user_123', | ||
| public_metadata: {}, | ||
| private_metadata: {}, | ||
| unsafe_metadata: {}, | ||
| }; | ||
|
|
||
| describe('updateUser', () => { | ||
| it('calls PATCH /users/{id} when no metadata fields are provided', async () => { | ||
| const patchHandler = vi.fn(async ({ request }: { request: Request }) => { | ||
| const body = await request.json(); | ||
| expect(body).toEqual({ first_name: 'Jane' }); | ||
| return HttpResponse.json(mockUserResponse); | ||
| }); | ||
|
|
||
| server.use(http.patch('https://api.clerk.test/v1/users/user_123', validateHeaders(patchHandler))); | ||
|
|
||
| const response = await apiClient.users.updateUser('user_123', { firstName: 'Jane' }); | ||
|
|
||
| expect(patchHandler).toHaveBeenCalledTimes(1); | ||
| expect(response.id).toBe('user_123'); | ||
| }); | ||
|
|
||
| it('routes metadata to PUT /users/{id}/metadata when only metadata is provided', async () => { | ||
| const patchHandler = vi.fn(() => HttpResponse.json(mockUserResponse)); | ||
| const putHandler = vi.fn(async ({ request }: { request: Request }) => { | ||
| const body = await request.json(); | ||
| expect(body).toEqual({ | ||
| public_metadata: { foo: 'bar' }, | ||
| }); | ||
| return HttpResponse.json({ | ||
| ...mockUserResponse, | ||
| public_metadata: { foo: 'bar' }, | ||
| }); | ||
| }); | ||
|
|
||
| server.use( | ||
| http.patch('https://api.clerk.test/v1/users/user_123', validateHeaders(patchHandler)), | ||
| http.put('https://api.clerk.test/v1/users/user_123/metadata', validateHeaders(putHandler)), | ||
| ); | ||
|
|
||
| const response = await apiClient.users.updateUser('user_123', { | ||
| publicMetadata: { foo: 'bar' }, | ||
| }); | ||
|
|
||
| expect(patchHandler).not.toHaveBeenCalled(); | ||
| expect(putHandler).toHaveBeenCalledTimes(1); | ||
| expect(response.publicMetadata).toEqual({ foo: 'bar' }); | ||
| }); | ||
|
|
||
| it('splits mixed calls: PATCH for non-metadata, then PUT for metadata', async () => { | ||
| const calls: string[] = []; | ||
|
|
||
| const patchHandler = vi.fn(async ({ request }: { request: Request }) => { | ||
| calls.push('patch'); | ||
| const body = await request.json(); | ||
| expect(body).toEqual({ first_name: 'Jane' }); | ||
| return HttpResponse.json(mockUserResponse); | ||
| }); | ||
|
|
||
| const putHandler = vi.fn(async ({ request }: { request: Request }) => { | ||
| calls.push('put'); | ||
| const body = await request.json(); | ||
| expect(body).toEqual({ | ||
| public_metadata: { plan: 'pro' }, | ||
| private_metadata: { invoice: 'inv_1' }, | ||
| }); | ||
| return HttpResponse.json({ | ||
| ...mockUserResponse, | ||
| first_name: 'Jane', | ||
| public_metadata: { plan: 'pro' }, | ||
| private_metadata: { invoice: 'inv_1' }, | ||
| }); | ||
| }); | ||
|
|
||
| server.use( | ||
| http.patch('https://api.clerk.test/v1/users/user_123', validateHeaders(patchHandler)), | ||
| http.put('https://api.clerk.test/v1/users/user_123/metadata', validateHeaders(putHandler)), | ||
| ); | ||
|
|
||
| const response = await apiClient.users.updateUser('user_123', { | ||
| firstName: 'Jane', | ||
| publicMetadata: { plan: 'pro' }, | ||
| privateMetadata: { invoice: 'inv_1' }, | ||
| }); | ||
|
|
||
| expect(patchHandler).toHaveBeenCalledTimes(1); | ||
| expect(putHandler).toHaveBeenCalledTimes(1); | ||
| // PATCH must run before PUT so the user state from PUT is the latest. | ||
| expect(calls).toEqual(['patch', 'put']); | ||
| expect(response.firstName).toBe('Jane'); | ||
| expect(response.publicMetadata).toEqual({ plan: 'pro' }); | ||
| }); | ||
|
|
||
| it('passes only metadata fields that were explicitly provided to PUT', async () => { | ||
| const putHandler = vi.fn(async ({ request }: { request: Request }) => { | ||
| const body = (await request.json()) as Record<string, unknown>; | ||
| // Only unsafe_metadata was provided. The other two should be undefined, | ||
| // which serializes to "field omitted" on the wire — leaving those | ||
| // columns untouched server-side. | ||
| expect(body.unsafe_metadata).toEqual({ device: 'mobile' }); | ||
| expect(body).not.toHaveProperty('public_metadata'); | ||
| expect(body).not.toHaveProperty('private_metadata'); | ||
| return HttpResponse.json({ | ||
| ...mockUserResponse, | ||
| unsafe_metadata: { device: 'mobile' }, | ||
| }); | ||
| }); | ||
|
|
||
| server.use(http.put('https://api.clerk.test/v1/users/user_123/metadata', validateHeaders(putHandler))); | ||
|
|
||
| await apiClient.users.updateUser('user_123', { | ||
| unsafeMetadata: { device: 'mobile' }, | ||
| }); | ||
|
|
||
| expect(putHandler).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); | ||
|
|
||
| describe('updateUserMetadata', () => { | ||
| it('still hits PATCH /users/{id}/metadata (unchanged)', async () => { | ||
| const patchHandler = vi.fn(async ({ request }: { request: Request }) => { | ||
| const body = await request.json(); | ||
| expect(body).toEqual({ | ||
| public_metadata: { merge: true }, | ||
| }); | ||
| return HttpResponse.json({ | ||
| ...mockUserResponse, | ||
| public_metadata: { merge: true }, | ||
| }); | ||
| }); | ||
|
|
||
| server.use(http.patch('https://api.clerk.test/v1/users/user_123/metadata', validateHeaders(patchHandler))); | ||
|
|
||
| await apiClient.users.updateUserMetadata('user_123', { | ||
| publicMetadata: { merge: true }, | ||
| }); | ||
|
|
||
| expect(patchHandler).toHaveBeenCalledTimes(1); | ||
| }); | ||
| }); | ||
|
|
||
| describe('replaceUserMetadata', () => { | ||
| it('hits PUT /users/{id}/metadata', async () => { | ||
| const putHandler = vi.fn(async ({ request }: { request: Request }) => { | ||
| const body = await request.json(); | ||
| expect(body).toEqual({ | ||
| public_metadata: { replaced: true }, | ||
| }); | ||
| return HttpResponse.json({ | ||
| ...mockUserResponse, | ||
| public_metadata: { replaced: true }, | ||
| }); | ||
| }); | ||
|
|
||
| server.use(http.put('https://api.clerk.test/v1/users/user_123/metadata', validateHeaders(putHandler))); | ||
|
|
||
| const response = await apiClient.users.replaceUserMetadata('user_123', { | ||
| publicMetadata: { replaced: true }, | ||
| }); | ||
|
|
||
| expect(putHandler).toHaveBeenCalledTimes(1); | ||
| expect(response.publicMetadata).toEqual({ replaced: true }); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import { deprecated } from '@clerk/shared/deprecated'; | ||
| import { getFullName } from '@clerk/shared/internal/clerk-js/user'; | ||
| import type { | ||
| BackupCodeJSON, | ||
|
|
@@ -47,6 +48,7 @@ import type { | |
|
|
||
| import { convertPageToOffsetSearchParams } from '../../utils/convertPageToOffsetSearchParams'; | ||
| import { unixEpochToDate } from '../../utils/date'; | ||
| import { computeMergePatch } from '../../utils/mergePatch'; | ||
| import { normalizeUnsafeMetadata } from '../../utils/resourceParams'; | ||
| import { eventBus, events } from '../events'; | ||
| import { addPaymentMethod, getPaymentMethods, initializePaymentMethod } from '../modules/billing'; | ||
|
|
@@ -235,9 +237,42 @@ export class User extends BaseResource implements UserResource { | |
| return new BackupCode(json); | ||
| }; | ||
|
|
||
| update = (params: UpdateUserParams): Promise<UserResource> => { | ||
| return this._basePatch({ | ||
| body: normalizeUnsafeMetadata(params), | ||
| update = async (params: UpdateUserParams): Promise<UserResource> => { | ||
| const { unsafeMetadata, ...rest } = params; | ||
| const hasMetadata = unsafeMetadata !== undefined; | ||
| const hasRest = Object.keys(rest).length > 0; | ||
|
|
||
| if (!hasMetadata) { | ||
| return this._basePatch({ | ||
| body: normalizeUnsafeMetadata(params), | ||
| }); | ||
| } | ||
|
|
||
| deprecated( | ||
| 'user.update({ unsafeMetadata })', | ||
| 'Use user.updateMetadata({ unsafeMetadata }) for partial updates (deep merge) instead.', | ||
| ); | ||
|
|
||
| // The FAPI endpoint deprecates `unsafe_metadata` on PATCH /me. Route | ||
| // metadata through PATCH /me/metadata (deep-merge) while preserving the | ||
| // *replace* semantics of `user.update({ unsafeMetadata })` by | ||
| // diffing the locally-cached value against the desired one and sending | ||
| // an RFC 7396 merge patch (null-deletes for removed keys). | ||
| if (hasRest) { | ||
| await this._basePatch({ | ||
| body: normalizeUnsafeMetadata(rest as UpdateUserParams), | ||
| }); | ||
| } | ||
|
|
||
| const patch = computeMergePatch(this.unsafeMetadata, unsafeMetadata); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this preserve replacement semantics if Correct me if Im wrong 😄
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes you are right . There are two scenarios:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @wobsoriano I pushed a fix. Let me know what you think! |
||
|
|
||
| // An empty patch means current already equals desired — short-circuit. | ||
| if (patch !== null && typeof patch === 'object' && Object.keys(patch).length === 0) { | ||
| return this; | ||
| } | ||
|
|
||
| return this.updateMetadata({ | ||
| unsafeMetadata: patch as UserUnsafeMetadata, | ||
| }); | ||
| }; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's split this into separate changesets so each package changelog stays focused 👍🏼
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done