diff --git a/.changeset/deprecate-update-user-metadata.md b/.changeset/deprecate-update-user-metadata.md new file mode 100644 index 00000000000..23a802994a0 --- /dev/null +++ b/.changeset/deprecate-update-user-metadata.md @@ -0,0 +1,23 @@ +--- +'@clerk/backend': minor +--- + +Add `clerkClient.users.replaceUserMetadata(userId, params)` for replacing a user's metadata fields in full. + +Use `replaceUserMetadata` when the provided metadata should become the complete value for that metadata field: + +```ts +await clerkClient.users.replaceUserMetadata(userId, { + publicMetadata: { plan: 'pro' }, +}); +``` + +Use `clerkClient.users.updateUserMetadata(userId, params)` when you want to partially update metadata with deep-merge semantics: + +```ts +await clerkClient.users.updateUserMetadata(userId, { + publicMetadata: { onboardingComplete: true }, +}); +``` + +The `publicMetadata`, `privateMetadata`, and `unsafeMetadata` parameters on `clerkClient.users.updateUser()` are now deprecated. They continue to work, but new code should use `updateUserMetadata()` for partial updates or `replaceUserMetadata()` for full replacement. diff --git a/.changeset/route-unsafe-metadata-to-merge-endpoint.md b/.changeset/route-unsafe-metadata-to-merge-endpoint.md new file mode 100644 index 00000000000..8151d235f3b --- /dev/null +++ b/.changeset/route-unsafe-metadata-to-merge-endpoint.md @@ -0,0 +1,24 @@ +--- +'@clerk/clerk-js': patch +'@clerk/shared': patch +--- + +Deprecate passing `unsafeMetadata` to `user.update()`. + +Use `user.updateMetadata()` when you want to partially update unsafe metadata with deep-merge semantics: + +```ts +await user.updateMetadata({ + unsafeMetadata: { onboardingComplete: true }, +}); +``` + +`user.update({ unsafeMetadata })` continues to work for now and preserves its existing full-replacement behavior: + +```ts +await user.update({ + unsafeMetadata: { theme: 'dark' }, +}); +``` + +New code should prefer `user.updateMetadata({ unsafeMetadata })` for metadata-only updates. diff --git a/integration/tests/unsafeMetadata.test.ts b/integration/tests/unsafeMetadata.test.ts index aebc0b3e80e..e44a34d464e 100644 --- a/integration/tests/unsafeMetadata.test.ts +++ b/integration/tests/unsafeMetadata.test.ts @@ -68,4 +68,105 @@ testAgainstRunningApps({ withEnv: [appConfigs.envs.withEmailCodes] })('unsafeMet await fakeUser.deleteIfExists(); }); + + // Helper: sign up a user via the UI and return the BAPI user id once the + // client session is established. Mirrors the existing sign-up test flow so + // these specs share the same baseline (`unsafeMetadata: { position: 'goalie' }`). + const signUpAndGetUser = async ({ page, context }: { page: any; context: any }) => { + const u = createTestUtils({ app, page, context }); + const fakeUser = u.services.users.createFakeUser({ + fictionalEmail: true, + withPhoneNumber: true, + withUsername: true, + }); + + await u.po.signUp.goTo(); + await u.po.signUp.signUpWithEmailAndPassword({ + email: fakeUser.email, + password: fakeUser.password, + }); + await u.po.signUp.enterTestOtpCode(); + await u.po.expect.toBeSignedIn(); + + const bapiUser = await u.services.users.getUser({ email: fakeUser.email }); + expect(bapiUser?.unsafeMetadata).toEqual({ position: 'goalie' }); + + return { u, fakeUser, bapiUser: bapiUser! }; + }; + + test('user.update({ unsafeMetadata }) preserves replace semantics end-to-end', async ({ page, context }) => { + const { u, fakeUser, bapiUser } = await signUpAndGetUser({ page, context }); + + // Drive the deprecated path from the browser. The SDK should route + // metadata through PATCH /v1/me/metadata after computing a merge patch + // against the locally-cached value; the server-side outcome must match + // a true replace (the original `position` key is gone). + await page.evaluate(async () => { + await window.Clerk.user.update({ unsafeMetadata: { city: 'Toronto' } }); + }); + + const refreshed = await u.services.users.getUser({ id: bapiUser.id }); + expect(refreshed?.unsafeMetadata).toEqual({ city: 'Toronto' }); + + await fakeUser.deleteIfExists(); + }); + + test('user.updateMetadata({ unsafeMetadata }) deep-merges (recommended path)', async ({ page, context }) => { + const { u, fakeUser, bapiUser } = await signUpAndGetUser({ page, context }); + + // The recommended migration target. Unlike `update(...)`, this is a + // partial update — the original `position` key must survive. + await page.evaluate(async () => { + await window.Clerk.user.updateMetadata({ unsafeMetadata: { city: 'Toronto' } }); + }); + + const refreshed = await u.services.users.getUser({ id: bapiUser.id }); + expect(refreshed?.unsafeMetadata).toEqual({ position: 'goalie', city: 'Toronto' }); + + await fakeUser.deleteIfExists(); + }); + + test('user.update with metadata + non-metadata fields persists both', async ({ page, context }) => { + const { u, fakeUser, bapiUser } = await signUpAndGetUser({ page, context }); + + // Mixed call: PATCH /v1/me for the non-metadata field, then + // PATCH /v1/me/metadata for the computed patch. Both must land. + await page.evaluate(async () => { + await window.Clerk.user.update({ + firstName: 'Updated', + unsafeMetadata: { city: 'Toronto' }, + }); + }); + + const refreshed = await u.services.users.getUser({ id: bapiUser.id }); + expect(refreshed?.firstName).toBe('Updated'); + expect(refreshed?.unsafeMetadata).toEqual({ city: 'Toronto' }); + + await fakeUser.deleteIfExists(); + }); + + test('user.update reloads before diffing so server-side mutations are not lost', async ({ page, context }) => { + const { u, fakeUser, bapiUser } = await signUpAndGetUser({ page, context }); + + // Simulate a server-side mutation made by *another* actor + // after the browser cached the user. + // The browser's local `unsafeMetadata` is now stale, + // missing the `adminAdded` key. + await u.services.clerk.users.updateUserMetadata(bapiUser.id, { + unsafeMetadata: { adminAdded: 'yes' }, + }); + + // From the browser, call the deprecated path with replace intent. + // Without the pre-diff reload, the SDK would diff against stale `{ position: 'goalie' }` + // send `{ position: null, city: 'Toronto' }`, and the server-side `adminAdded` would silently survive violating replace semantics. + // The reload makes the SDK observe the fresh state and null-delete the server-added key too. + await page.evaluate(async () => { + await window.Clerk.user.update({ unsafeMetadata: { city: 'Toronto' } }); + }); + + const refreshed = await u.services.users.getUser({ id: bapiUser.id }); + expect(refreshed?.unsafeMetadata).toEqual({ city: 'Toronto' }); + + await fakeUser.deleteIfExists(); + }); }); diff --git a/packages/backend/src/api/__tests__/UserApi.test.ts b/packages/backend/src/api/__tests__/UserApi.test.ts new file mode 100644 index 00000000000..abeb42bf361 --- /dev/null +++ b/packages/backend/src/api/__tests__/UserApi.test.ts @@ -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; + // 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 }); + }); + }); +}); diff --git a/packages/backend/src/api/endpoints/UserApi.ts b/packages/backend/src/api/endpoints/UserApi.ts index 1124da452ea..bf0ab1289fe 100644 --- a/packages/backend/src/api/endpoints/UserApi.ts +++ b/packages/backend/src/api/endpoints/UserApi.ts @@ -171,8 +171,34 @@ type UpdateUserParams = { /** The maximum number of Organizations the user can create. 0 means unlimited. */ createOrganizationsLimit?: number; -} & UserMetadataParams & - (UserPasswordHashingParams | object); + + /** + * Metadata visible to your Frontend and Backend APIs. + * + * @deprecated Updating metadata via `updateUser` is deprecated. Use + * `updateUserMetadata` for partial updates (deep merge) or + * `replaceUserMetadata` for full replacement. + */ + publicMetadata?: UserPublicMetadata; + + /** + * Metadata visible only to your Backend API. + * + * @deprecated Updating metadata via `updateUser` is deprecated. Use + * `updateUserMetadata` for partial updates (deep merge) or + * `replaceUserMetadata` for full replacement. + */ + privateMetadata?: UserPrivateMetadata; + + /** + * Metadata writeable from both the Frontend and Backend APIs. + * + * @deprecated Updating metadata via `updateUser` is deprecated. Use + * `updateUserMetadata` for partial updates (deep merge) or + * `replaceUserMetadata` for full replacement. + */ + unsafeMetadata?: UserUnsafeMetadata; +} & (UserPasswordHashingParams | object); type GetOrganizationMembershipListParams = ClerkPaginationRequest<{ userId: string; @@ -252,10 +278,38 @@ export class UserAPI extends AbstractAPI { public async updateUser(userId: string, params: UpdateUserParams = {}) { this.requireId(userId); + const { publicMetadata, privateMetadata, unsafeMetadata, ...rest } = params as UpdateUserParams & + UserMetadataParams; + const hasMetadata = publicMetadata !== undefined || privateMetadata !== undefined || unsafeMetadata !== undefined; + const hasRest = Object.keys(rest).length > 0; + + if (hasMetadata) { + deprecated( + 'updateUser(userId, { publicMetadata | privateMetadata | unsafeMetadata })', + 'Use updateUserMetadata for partial updates (merge) or replaceUserMetadata for full replacement.', + ); + } + + if (!hasMetadata) { + return this.request({ + method: 'PATCH', + path: joinPaths(basePath, userId), + bodyParams: rest, + }); + } + + if (hasRest) { + await this.request({ + method: 'PATCH', + path: joinPaths(basePath, userId), + bodyParams: rest, + }); + } + return this.request({ - method: 'PATCH', - path: joinPaths(basePath, userId), - bodyParams: params, + method: 'PUT', + path: joinPaths(basePath, userId, 'metadata'), + bodyParams: { publicMetadata, privateMetadata, unsafeMetadata }, }); } @@ -282,6 +336,21 @@ export class UserAPI extends AbstractAPI { }); } + /** + * Replace a user's metadata. Supplied fields are overwritten in full; fields + * omitted from `params` are left unchanged. Prefer `updateUserMetadata` for + * partial updates with deep-merge semantics. + */ + public async replaceUserMetadata(userId: string, params: UserMetadataParams) { + this.requireId(userId); + + return this.request({ + method: 'PUT', + path: joinPaths(basePath, userId, 'metadata'), + bodyParams: params, + }); + } + public async deleteUser(userId: string) { this.requireId(userId); return this.request({ diff --git a/packages/clerk-js/src/core/resources/User.ts b/packages/clerk-js/src/core/resources/User.ts index 9ec8dfd5ea3..8fb5f062a9f 100644 --- a/packages/clerk-js/src/core/resources/User.ts +++ b/packages/clerk-js/src/core/resources/User.ts @@ -1,4 +1,5 @@ import { getFullName } from '@clerk/shared/internal/clerk-js/user'; +import { isDevelopmentFromPublishableKey } from '@clerk/shared/keys'; import type { BackupCodeJSON, BackupCodeResource, @@ -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,51 @@ export class User extends BaseResource implements UserResource { return new BackupCode(json); }; - update = (params: UpdateUserParams): Promise => { - return this._basePatch({ - body: normalizeUnsafeMetadata(params), + update = async (params: UpdateUserParams): Promise => { + const { unsafeMetadata, ...rest } = params; + const hasMetadata = unsafeMetadata !== undefined; + const hasRest = Object.keys(rest).length > 0; + + if (!hasMetadata) { + return this._basePatch({ + body: normalizeUnsafeMetadata(params), + }); + } + + if (isDevelopmentFromPublishableKey(BaseResource.clerk.publishableKey)) { + console.warn( + 'Clerk - DEPRECATION WARNING: "user.update({ unsafeMetadata })" is deprecated and will be removed in the next major release.\nUse 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). + // + // + // When `hasRest` is true the PATCH /me below refreshes `this` in place + // via `fromJSON` before we read `this.unsafeMetadata` for the diff. + // When it's false (only-metadata update), no upstream call refreshes the cache, + // so we `reload()` explicitly. + if (hasRest) { + await this._basePatch({ + body: normalizeUnsafeMetadata(rest as UpdateUserParams), + }); + } else { + await this.reload(); + } + + const patch = computeMergePatch(this.unsafeMetadata, unsafeMetadata); + + // 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, }); }; diff --git a/packages/clerk-js/src/core/resources/__tests__/User.test.ts b/packages/clerk-js/src/core/resources/__tests__/User.test.ts index 20e8074cf16..4a581aae4d6 100644 --- a/packages/clerk-js/src/core/resources/__tests__/User.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/User.test.ts @@ -1,5 +1,5 @@ import type { EnterpriseConnectionJSON, UserJSON } from '@clerk/shared/types'; -import { describe, expect, it, vi } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { BaseResource } from '../internal'; import { User } from '../User'; @@ -757,4 +757,224 @@ describe('User', () => { }, }); }); + + describe('.update with metadata routing', () => { + beforeEach(() => { + // @ts-ignore + BaseResource.clerk = { publishableKey: 'pk_test_foo' }; + }); + + it('calls PATCH /me only when no unsafeMetadata is provided', async () => { + // @ts-ignore + BaseResource._fetch = vi.fn().mockReturnValue(Promise.resolve({ response: {} })); + + const user = new User({} as unknown as UserJSON); + await user.update({ firstName: 'Jane' }); + + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenCalledTimes(1); + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenCalledWith({ + method: 'PATCH', + path: '/me', + body: { firstName: 'Jane' }, + }); + }); + + it('routes only-metadata updates to /me/metadata as an RFC 7396 merge patch', async () => { + // Server still reflects the locally-cached state; the reload returns + // the same metadata, so the diff is computed identically. + // @ts-ignore + BaseResource._fetch = vi.fn().mockImplementation((opts: any) => { + if (opts.method === 'GET') { + return Promise.resolve({ + response: { unsafe_metadata: { theme: 'dark', layout: 'compact' } }, + }); + } + return Promise.resolve({ response: {} }); + }); + + // Seed current state: { theme: 'dark', layout: 'compact' }. Desired + // state drops `layout` and changes `theme` — the merge patch must + // null-delete `layout` to preserve replace semantics. + const user = new User({ + unsafe_metadata: { theme: 'dark', layout: 'compact' }, + } as unknown as UserJSON); + + await user.update({ unsafeMetadata: { theme: 'light' } }); + + // Two calls now: a GET /me reload to refresh the diff baseline, then + // PATCH /me/metadata with the computed patch. + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenCalledTimes(2); + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ method: 'GET', path: '/me' }), + expect.anything(), + ); + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenNthCalledWith(2, { + method: 'PATCH', + path: '/me/metadata', + body: { + unsafeMetadata: JSON.stringify({ theme: 'light', layout: null }), + }, + }); + }); + + it('reloads before diffing so server-side mutations are not lost', async () => { + // The local cache thinks unsafeMetadata is { a: 1 }, but the server + // has actually drifted to { a: 1, b: 2 }. Without the pre-diff reload + // the SDK would compute mergePatch({ a: 1 }, { a: 99 }) = { a: 99 } + // and `b` would survive on the server, silently violating the + // caller's intended replace semantics. + // @ts-ignore + BaseResource._fetch = vi.fn().mockImplementation((opts: any) => { + if (opts.method === 'GET') { + return Promise.resolve({ + response: { unsafe_metadata: { a: 1, b: 2 } }, + }); + } + return Promise.resolve({ response: {} }); + }); + + const user = new User({ + unsafe_metadata: { a: 1 }, + } as unknown as UserJSON); + + await user.update({ unsafeMetadata: { a: 99 } }); + + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenCalledTimes(2); + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenNthCalledWith(2, { + method: 'PATCH', + path: '/me/metadata', + body: { + // The patch null-deletes `b` because the reload surfaced it as a + // key the server has and the desired state does not. + unsafeMetadata: JSON.stringify({ a: 99, b: null }), + }, + }); + }); + + it('splits mixed calls: PATCH /me for non-metadata, then PATCH /me/metadata for metadata', async () => { + const calls: Array<{ method: string; path: string | undefined }> = []; + // @ts-ignore + BaseResource._fetch = vi.fn().mockImplementation((opts: any) => { + calls.push({ method: opts.method, path: opts.path }); + return Promise.resolve({ response: {} }); + }); + + const user = new User({ + unsafe_metadata: { foo: 'old' }, + } as unknown as UserJSON); + + await user.update({ + firstName: 'Jane', + unsafeMetadata: { foo: 'new', bar: 'added' }, + }); + + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenCalledTimes(2); + expect(calls[0]).toEqual({ method: 'PATCH', path: '/me' }); + expect(calls[1]).toEqual({ method: 'PATCH', path: '/me/metadata' }); + + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenNthCalledWith(1, { + method: 'PATCH', + path: '/me', + body: { firstName: 'Jane' }, + }); + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenNthCalledWith(2, { + method: 'PATCH', + path: '/me/metadata', + body: { + unsafeMetadata: JSON.stringify({ foo: 'new', bar: 'added' }), + }, + }); + }); + + it('makes only a reload call when desired metadata equals current (no PUT)', async () => { + // The pre-diff reload always runs, but if the fresh server state + // matches `desired` the computed patch is empty and we skip the PUT. + // @ts-ignore + BaseResource._fetch = vi.fn().mockImplementation((opts: any) => { + if (opts.method === 'GET') { + return Promise.resolve({ + response: { unsafe_metadata: { theme: 'dark' } }, + }); + } + return Promise.resolve({ response: {} }); + }); + + const user = new User({ + unsafe_metadata: { theme: 'dark' }, + } as unknown as UserJSON); + + await user.update({ unsafeMetadata: { theme: 'dark' } }); + + // Exactly one call: the reload. No PATCH /me/metadata. + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenCalledTimes(1); + // @ts-ignore + expect(BaseResource._fetch).toHaveBeenCalledWith( + expect.objectContaining({ method: 'GET', path: '/me' }), + expect.anything(), + ); + }); + + it('logs a deprecation warning when unsafeMetadata is passed to update()', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore + BaseResource._fetch = vi.fn().mockImplementation((opts: any) => { + if (opts.method === 'GET') { + return Promise.resolve({ response: { unsafe_metadata: {} } }); + } + return Promise.resolve({ response: {} }); + }); + + const user = new User({} as unknown as UserJSON); + await user.update({ unsafeMetadata: { theme: 'dark' } }); + + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('user.update({ unsafeMetadata })')); + + warnSpy.mockRestore(); + }); + + it('does not log a deprecation warning when unsafeMetadata is not passed to update()', async () => { + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore + BaseResource._fetch = vi.fn().mockReturnValue(Promise.resolve({ response: {} })); + + const user = new User({} as unknown as UserJSON); + await user.update({ firstName: 'Jane' }); + + expect(warnSpy).not.toHaveBeenCalled(); + + warnSpy.mockRestore(); + }); + + it('does not log a deprecation warning for production publishable keys', async () => { + // @ts-ignore + BaseResource.clerk = { publishableKey: 'pk_live_foo' }; + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + // @ts-ignore + BaseResource._fetch = vi.fn().mockImplementation((opts: any) => { + if (opts.method === 'GET') { + return Promise.resolve({ response: { unsafe_metadata: {} } }); + } + return Promise.resolve({ response: {} }); + }); + + const user = new User({} as unknown as UserJSON); + await user.update({ unsafeMetadata: { theme: 'dark' } }); + + expect(warnSpy).not.toHaveBeenCalled(); + + warnSpy.mockRestore(); + }); + }); }); diff --git a/packages/clerk-js/src/utils/__tests__/mergePatch.test.ts b/packages/clerk-js/src/utils/__tests__/mergePatch.test.ts new file mode 100644 index 00000000000..bc0dddf8686 --- /dev/null +++ b/packages/clerk-js/src/utils/__tests__/mergePatch.test.ts @@ -0,0 +1,136 @@ +import { describe, expect, it } from 'vitest'; + +import { computeMergePatch } from '../mergePatch'; + +describe('computeMergePatch', () => { + it('returns added keys verbatim', () => { + expect(computeMergePatch({ a: 1 }, { a: 1, b: 2 })).toEqual({ b: 2 }); + }); + + it('nulls keys absent from desired to delete them (RFC 7396)', () => { + expect(computeMergePatch({ a: 1, b: 2 }, { a: 1 })).toEqual({ b: null }); + }); + + it('overwrites primitive values that changed', () => { + expect(computeMergePatch({ a: 1 }, { a: 2 })).toEqual({ a: 2 }); + }); + + it('skips keys whose value is unchanged', () => { + expect(computeMergePatch({ a: 1, b: 2 }, { a: 1, b: 2 })).toEqual({}); + }); + + it('recurses into nested objects, only emitting changed sub-keys', () => { + expect( + computeMergePatch({ profile: { theme: 'dark', font: 'sans' } }, { profile: { theme: 'light', font: 'sans' } }), + ).toEqual({ profile: { theme: 'light' } }); + }); + + it('nulls a removed nested key while keeping siblings untouched', () => { + expect(computeMergePatch({ profile: { theme: 'dark', font: 'sans' } }, { profile: { font: 'sans' } })).toEqual({ + profile: { theme: null }, + }); + }); + + it('returns full replacement of the desired side when shapes differ', () => { + expect(computeMergePatch({ a: 1 }, 'replaced')).toBe('replaced'); + expect(computeMergePatch('was-string', { a: 1 })).toEqual({ a: 1 }); + }); + + it('passes null through verbatim (caller decides what null means)', () => { + expect(computeMergePatch({ a: 1 }, null)).toBeNull(); + }); + + it('treats arrays as opaque values (replace, not merge)', () => { + // RFC 7396 itself says arrays are atomic. + expect(computeMergePatch({ tags: ['a', 'b'] }, { tags: ['a'] })).toEqual({ tags: ['a'] }); + }); + + it('clears every existing key when desired is empty', () => { + expect(computeMergePatch({ a: 1, b: 2 }, {})).toEqual({ a: null, b: null }); + }); + + it('starts from empty current → returns desired verbatim', () => { + expect(computeMergePatch({}, { a: 1, b: { c: 2 } })).toEqual({ a: 1, b: { c: 2 } }); + }); + + it('when applied, the patch round-trips current → desired (sanity check)', () => { + const current = { a: 1, nested: { x: 1, y: 2 }, removed: true }; + const desired = { a: 2, nested: { x: 1, z: 3 }, added: 'yes' }; + + const patch = computeMergePatch(current, desired) as Record; + + // Manually apply the patch with RFC 7396 semantics to confirm it produces + // the desired state. + const applyMergePatch = (target: any, p: any): any => { + if (p === null || typeof p !== 'object' || Array.isArray(p)) { + return p; + } + const out: Record = + typeof target === 'object' && target !== null && !Array.isArray(target) ? { ...target } : {}; + for (const key of Object.keys(p)) { + const v = p[key]; + if (v === null) { + delete out[key]; + } else { + out[key] = applyMergePatch(out[key], v); + } + } + return out; + }; + + expect(applyMergePatch(current, patch)).toEqual(desired); + }); + + describe('non-POJO values are treated as atomic', () => { + it('emits a Date when the value changed', () => { + const current = { lastSeen: new Date('2024-01-01T00:00:00Z') }; + const desired = { lastSeen: new Date('2024-01-02T00:00:00Z') }; + expect(computeMergePatch(current, desired)).toEqual({ + lastSeen: new Date('2024-01-02T00:00:00Z'), + }); + }); + + it('skips a Date whose value is unchanged', () => { + const ts = '2024-01-01T00:00:00.000Z'; + expect(computeMergePatch({ lastSeen: new Date(ts) }, { lastSeen: new Date(ts) })).toEqual({}); + }); + + it('emits a Map when the value changed', () => { + const current = { tags: new Map([['a', 1]]) }; + const desired = { tags: new Map([['b', 2]]) }; + expect(computeMergePatch(current, desired)).toEqual({ tags: new Map([['b', 2]]) }); + }); + + it('omits a Map when the value is unchanged', () => { + const map = new Map([['a', 1]]); + expect(computeMergePatch({ tags: map }, { tags: map })).toEqual({}); + }); + + it('emits a class instance when the value changed', () => { + class Tag { + constructor(public name: string) {} + } + const current = { tag: new Tag('one') }; + const desired = { tag: new Tag('two') }; + expect(computeMergePatch(current, desired)).toEqual({ tag: new Tag('two') }); + }); + + it('omits a class instance when the value is unchanged', () => { + class Tag { + constructor(public name: string) {} + } + const tag = new Tag('one'); + expect(computeMergePatch({ tag }, { tag })).toEqual({}); + }); + + it('does not match Object.create(null) hash maps as opaque values', () => { + // Prototype-less objects with no class identity SHOULD still recurse as POJOs. + const current: Record = Object.create(null); + current.a = 1; + current.b = 2; + const desired: Record = Object.create(null); + desired.a = 1; + expect(computeMergePatch(current, desired)).toEqual({ b: null }); + }); + }); +}); diff --git a/packages/clerk-js/src/utils/mergePatch.ts b/packages/clerk-js/src/utils/mergePatch.ts new file mode 100644 index 00000000000..98fab5ea2a6 --- /dev/null +++ b/packages/clerk-js/src/utils/mergePatch.ts @@ -0,0 +1,63 @@ +import { dequal } from 'dequal'; + +/** + * Computes a JSON Merge Patch (RFC 7396) that, when deep-merged into `current`, + * produces `desired`. Keys present in `current` but absent from `desired` + * receive `null` in the patch (RFC 7396 null-delete semantics). + * + * Used to express *replace* semantics through a merge endpoint: the SDK + * holds the current resource state locally, the consumer passes the desired + * state, and we send the diff that makes the server side end up at the + * desired state. + * + * Behaviour: + * - both plain objects: recurse; emit only keys whose value changes + * - `desired === null`: returned verbatim (caller decides what null means) + * - any other type mismatch: `desired` is returned (full replace at that node) + */ +export function computeMergePatch(current: unknown, desired: unknown): unknown { + if (desired === null) { + return null; + } + if (!isPlainObject(current) || !isPlainObject(desired)) { + return desired; + } + + const patch: Record = {}; + + for (const key of Object.keys(desired)) { + const cur = current[key]; + const des = desired[key]; + + if (!(key in current)) { + patch[key] = des; + continue; + } + + if (isPlainObject(cur) && isPlainObject(des)) { + const sub = computeMergePatch(cur, des); + if (isPlainObject(sub) && Object.keys(sub).length === 0) { + continue; + } + patch[key] = sub; + } else if (!dequal(cur, des)) { + patch[key] = des; + } + } + + for (const key of Object.keys(current)) { + if (!(key in desired)) { + patch[key] = null; + } + } + + return patch; +} + +function isPlainObject(value: unknown): value is Record { + if (typeof value !== 'object' || value === null) { + return false; + } + const proto = Object.getPrototypeOf(value); + return proto === null || proto === Object.prototype; +} diff --git a/packages/shared/src/types/user.ts b/packages/shared/src/types/user.ts index 5f6c392cc93..72daa643eff 100644 --- a/packages/shared/src/types/user.ts +++ b/packages/shared/src/types/user.ts @@ -186,7 +186,14 @@ type UpdateUserJSON = Pick< | 'unsafe_metadata' >; -export type UpdateUserParams = Partial>; +export type UpdateUserParams = Omit>, 'unsafeMetadata'> & { + /** + * @deprecated Updating `unsafeMetadata` via `user.update()` is deprecated. + * Use `user.updateMetadata({ unsafeMetadata })` for partial updates (deep + * merge). The parameter will be removed in a future major version. + */ + unsafeMetadata?: UserUnsafeMetadata; +}; /** * Parameters for {@link UserResource.updateMetadata}. Only `unsafeMetadata`