refactor(js): extract a generic store from TokenCache#8860
Conversation
Lock in current cross-tab, lifecycle, degradation, and audience-keying behavior as the regression bar for the TokenCache decoupling (SDK-117). The broadcast postMessage-failure case is a known pre-existing bug (SDK-119), captured as an it.fails tripwire.
Lift the raw key/value storage out of MemoryTokenCache into a small generic createTokenStore<V> (pure storage: no timers, no BroadcastChannel, no JWT knowledge) and rewire the cache onto it. No behavior change; the identity-based deleteKey guard and overwrite check are preserved. First structural step of SDK-117.
🦋 Changeset detectedLatest commit: 5741c5c The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types 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 GitHub.
|
📝 WalkthroughWalkthroughIntroduces a new ChangesTokenCache Store Decoupling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/clerk-js/src/core/tokenStore.ts (1)
22-24: ⚡ Quick winAdd
@returnstag to JSDoc for customer-facing documentation.Since this is a public/reference-facing API, the JSDoc should include a
@returnstag. Clerk Docs generates reference documentation from JSDoc comments in this repository, so completeness is important for customer-facing clarity.📝 Suggested JSDoc enhancement
/** * Creates an empty in-memory {`@link` TokenStore} backed by a Map. + * + * `@returns` A new TokenStore instance with no entries */As per coding guidelines: "Document functions with JSDoc comments including
@param,@returns,@throws, and@exampletags" and "Treat JSDoc on public/reference-facing APIs as customer-facing documentation."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/clerk-js/src/core/tokenStore.ts` around lines 22 - 24, The JSDoc comment for the function that creates an empty in-memory TokenStore is missing the `@returns` tag. Add a `@returns` tag to the JSDoc block that documents what type of object is returned by this function, describing that it returns a TokenStore instance. This will ensure the auto-generated customer-facing documentation is complete and provides clear information about the function's return value.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/clerk-js/src/core/tokenStore.ts`:
- Around line 22-24: The JSDoc comment for the function that creates an empty
in-memory TokenStore is missing the `@returns` tag. Add a `@returns` tag to the
JSDoc block that documents what type of object is returned by this function,
describing that it returns a TokenStore instance. This will ensure the
auto-generated customer-facing documentation is complete and provides clear
information about the function's return value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 9c96bed2-1827-46a8-8152-0445c5ffc9ad
📒 Files selected for processing (5)
.changeset/decouple-tokencache-store.mdpackages/clerk-js/src/core/__tests__/tokenCache.test.tspackages/clerk-js/src/core/__tests__/tokenStore.test.tspackages/clerk-js/src/core/tokenCache.tspackages/clerk-js/src/core/tokenStore.ts
1 participant
First slice of the TokenCache decoupling (SDK-117), in two no-behavior-change steps.
The first commit backfills characterization tests that pin down current cache behavior (cross-tab
close()lifecycle, graceful degradation whenBroadcastChannelis missing, audience key coalescing) to serve as the regression bar for the rest of the refactor. The second lifts the raw key/value storage out ofMemoryTokenCacheinto a small genericcreateTokenStore<V>and rewires the cache onto it, so storage is testable on its own without timers or channel mocks.The part worth a careful read is the
cache.* -> store.*rewire intokenCache.ts. ThedeleteKeyidentity check and the overwrite guard both rely onstore.get(key) !== valuepreserving reference identity, which the Map-backed store does. The fullsrc/coresuite stays green (632 tests, with Session/Client as the real consumers).Writing the backfill surfaced a pre-existing bug: a broadcast
postMessagefailure evicts a freshly cached token, because the throw lands insetInternal's.catch(deleteKey). I left it as anit.failstripwire and filed it separately as SDK-119 (backport candidate) rather than fixing it in this refactor.Empty changeset, since nothing here is user-facing.
Summary by CodeRabbit
Tests
Refactor