Skip to content

feat(react): add useOAuthTokens hook - #1213

Merged
joshuaellis merged 1 commit into
mainfrom
feat/SDK-2420
Sep 22, 2026
Merged

joshuaellis merged 1 commit into
mainfrom
feat/SDK-2420

Conversation

@joshuaellis

Copy link
Copy Markdown
Member

Description

Adds useOAuthTokens, a React hook exposing the stored OAuth token state along with refresh and revoke. It's a thin consumer of the token actions and state accessor from the core OAuth foundation (SDK-2471), continuing the move to bring SDK apps onto Sanity's OAuth authorization-code + PKCE flow (SDK-2416). tokens reads through core's subscription-based state source, so it stays in sync across tabs; refresh runs the refresh_token grant and revoke clears token state.

@joshuaellis
joshuaellis requested a review from a team as a code owner September 10, 2026 23:11
@claude

claude Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @joshuaellis's task in 4m 29s —— View job


Review

  • Gather context (diff, AGENTS.md, .claude/rules/*)
  • Review changes
  • Post findings

Three items, one of which is a release-blocker to confirm.

1. Changeset says minor, but this narrows an already-public core API

getOAuthTokensState and refreshOAuthTokens are @public and already exported from packages/core/src/_exports/index.ts on main (this PR modifies them, and bundle-stats shows only +116 B on the @sanity/sdk export, so they are not new here). There is no pending changeset in .changeset/ covering them, which implies they went out in a release (last: 3.3.0, 2026-09-17).

Dropping refreshToken from their return values is therefore a breaking change: tokens.refreshToken stops type-checking and stops existing at runtime for anyone already reading it.

packages/core/src/auth/oauth/oauthActions.ts:280, :374-399 and .changeset/oauth-usetokens.md:2.

Fix: bump '@sanity/sdk' to major in the changeset — or, if you can confirm these two exports have never appeared in a published version, leave minor and say so on the PR so the next reviewer doesn't re-raise it.

Fix this →

2. The public token shape is an anonymous Omit<OAuthTokens, 'refreshToken'>

It now appears in two public core signatures and three places in UseOAuthTokensResult (useOAuthTokens.tsx:22,40), plus throughout the tests. Consumers who want to type a variable or a prop holding these tokens have to re-spell the Omit themselves, and any future change to what core strips has to be edited in every site.

Fix: export a named @public type from core (e.g. PublicOAuthTokens = Omit<OAuthTokens, 'refreshToken'>) beside OAuthTokens in packages/core/src/auth/oauth/types.ts, and use it in refreshOAuthTokens, getOAuthTokensState, selectPublicOAuthTokens, and UseOAuthTokensResult.

Fix this →

3. Non-blocking: expiry comparison lives in the React package

useOAuthTokens.tsx:52-55 is the only expiry check in the SDK — core has none, despite owning expiresAt and the refresh flow. Any core-side auto-refresh added later will duplicate this. You already flagged moving it to core as a follow-up; noting it so it doesn't get lost.

Nothing else actionable: the closure-staleness fix (reading the state source at call time rather than the render snapshot) is correct and covered by useOAuthTokens.test.tsx:128-150, the reselect memoisation is the right fix for useSyncExternalStore identity (isEqual on createStateSourceAction would not help, since it only guards the observable pipeline, not getCurrent), the refresh-token rotation fallback at oauthActions.ts:312 still preserves the stored refresh token, and reselect is already an inline runtime dep of packages/core so no renovate.json change is needed.

I did not run pnpm test / ts:check / fallow audit — dependencies are not installed in this environment. CI covers them.
· branch feat/SDK-2420

@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Bundle Stats

✅ No significant changes.

All scenario measurements (8)

🗺️ @sanity/sdk / @sanity/sdk · @sanity/sdk / @sanity/sdk/_internal · @sanity/sdk / @sanity/sdk/agent · @sanity/sdk / @sanity/sdk/comlink · @sanity/sdk / @sanity/sdk/dashboard · @sanity/sdk-react / @sanity/sdk-react · @sanity/sdk-react / @sanity/sdk-react/dashboard · @sanity/sdk-react / @sanity/sdk-react/dashboard/_internal · Artifacts

Scenario Kind Bundle (raw / gzip) Gzip change Import time Import change
⚪ @sanity/sdk / @sanity/sdk export 1.14 MB / 259.5 KB +116 B, +0.0% 176 ms -1 ms, -0.6%
⚪ @sanity/sdk / @sanity/sdk/_internal export 599.2 KB / 137.0 KB +13 B, +0.0% 131 ms -1 ms, -1.0%
⚪ @sanity/sdk / @sanity/sdk/agent export 414.5 KB / 90.9 KB None 119 ms +1 ms, +0.8%
⚪ @sanity/sdk / @sanity/sdk/comlink export 260.2 KB / 60.6 KB None 130 ms -1 ms, -0.8%
⚪ @sanity/sdk / @sanity/sdk/dashboard export 105.0 KB / 23.2 KB None 118 ms +1 ms, +1.2%
⚪ @sanity/sdk-react / @sanity/sdk-react export 1.42 MB / 334.2 KB +927 B, +0.3% 215 ms +2 ms, +1.1%
⚪ @sanity/sdk-react / @sanity/sdk-react/dashboard export 827.1 KB / 189.1 KB None 210 ms +2 ms, +1.0%
⚪ @sanity/sdk-react / @sanity/sdk-react/dashboard/_internal export 107.8 KB / 23.7 KB None 133 ms -2 ms, -1.2%

Significant means at least 1.0 KB and 1% gzip, or at least 5 ms and 10% import time.

@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

📚 TypeDoc Generation Result

✅ TypeDoc generated successfully!

  • JSON file size: 3.2M
  • Total exports: 447
  • Artifact: typedoc-html-pr-1213
  • HTML docs preview: Download artifact

The package TypeDoc JSON files and combined HTML site have been generated successfully.

@github-actions

github-actions Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🟢 Lines 97.34% (🎯 94%) 5892 / 6053
🟢 Statements 96.04% (🎯 93%) 6408 / 6672
🟢 Functions 97.34% (🎯 94%) 1763 / 1811
🟢 Branches 89.97% (🎯 86%) 3402 / 3781
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/core/src/auth/oauth/oauthActions.ts 100% 94.73% 100% 100%
packages/react/src/hooks/auth/useOAuthTokens.tsx 100% 100% 100% 100%
Generated in workflow #6455 for commit b42efef by the Vitest Coverage Report Action

@joshuaellis
joshuaellis added this pull request to stack #1218 September 11, 2026 12:51
@joshuaellis
joshuaellis force-pushed the feat/SDK-2420 branch 2 times, most recently from 7eb53bd to e30b4a4 Compare September 11, 2026 14:41
@joshuaellis

Copy link
Copy Markdown
Member Author

Actioned across the last few pushes: refresh contract test now matches core (tokens become null on the no-refresh-token path), documented the null/logout and 4xx semantics on refresh, added rejecting-refresh tests, wrapped the example in try/catch, dropped the unused now param, and removed the useLogOut pointer from the revoke docs.

Not actioning here:

const tokens = useOAuthTokensState()
return {
tokens,
isExpired: () => isOAuthTokenExpired(tokens),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this closure will "cache" the expired state

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isOAuthTokenExpired reads Date.now at call time so I'm not sure I agree – there's a test that disproves this iiuc

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I (actually claude) was able to put a test together that explains what I am talking about

it('isExpired reads the current tokens, not the render-time snapshot', async () => {
  const source = createFakeTokenSource(makeTokens({expiresAt: new Date(Date.now() - 1000)}))
  mockGetState.mockReturnValue(source)
  const fresh = makeTokens({accessToken: 'new'})
  mockRefresh.mockImplementation(() => {
    source.set(fresh)
    return Promise.resolve(fresh)
  })
  const {result} = renderHook(() => useOAuthTokens(), {wrapper})
  const {isExpired, refresh} = result.current
  expect(isExpired()).toBe(true)
  await act(async () => {
    await refresh()
  })
  expect(result.current.isExpired()).toBe(false) // fresh render: passes
  expect(isExpired()).toBe(false) // captured closure: fails, still true
})

@joshuaellis joshuaellis Sep 21, 2026 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Will look at fixing 🥸 TDD ftw

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed!

Comment thread packages/react/src/hooks/auth/useOAuthTokens.tsx Outdated
@joshuaellis
joshuaellis force-pushed the feat/SDK-2420 branch 5 times, most recently from 4d4fee4 to 8592b26 Compare September 21, 2026 12:24
@joshuaellis
joshuaellis merged commit 13c4166 into main Sep 22, 2026
27 checks passed
@squiggler-app squiggler-app Bot mentioned this pull request Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants