-
Notifications
You must be signed in to change notification settings - Fork 408
feat(clerk-js): Add debug=skip_cache param to token requests #7155
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
Conversation
🦋 Changeset detectedLatest commit: e4c878b The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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.
|
WalkthroughToken.create gained an optional skipCache parameter that, when true, causes the token creation request to include a debug=skip_cache query parameter. Session now forwards skipCache to Token.create. constants.ts had two constant declarations reordered. Tests and a changeset were added. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Session
participant Token
participant API
Caller->>Session: requestToken(..., skipCache?)
activate Session
Session->>Token: create(path, body, skipCache)
deactivate Session
activate Token
alt skipCache == true
Note right of Token `#e6f7ff`: build search\ndebug=skip_cache
Token->>API: POST /path?debug=skip_cache (body)
else skipCache == false or undefined
Token->>API: POST /path (body)
end
deactivate Token
API-->>Token: TokenResource
Token-->>Caller: TokenResource
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (33)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
|
Found 46 test failures on Blacksmith runners:
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.changeset/old-wombats-tease.md (2)
5-5: Remove unnecessary Markdown escape sequence.The backslash before the underscore in
\_clerk_skip_cacheis not needed in changeset descriptions and will render as a literal backslash in release notes. The changeset parser treats this as plain text, not Markdown.Apply this diff to remove the unnecessary escape:
-Added \_clerk_skip_cache query string param to token requests initiated with skipCache option +Added _clerk_skip_cache query string param to token requests initiated with skipCache option
5-5: Clarify description for release notes.The change description could be more specific about when this parameter is added (i.e., when
skipCacheistrue). This will help users understand the new behavior from release notes.Consider updating to:
-Added _clerk_skip_cache query string param to token requests initiated with skipCache option +Added _clerk_skip_cache query string parameter to token requests when the skipCache option is set to true
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.changeset/old-wombats-tease.md(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/**
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Automated releases must use Changesets.
Files:
.changeset/old-wombats-tease.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (30)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 14)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (expo-web, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (elements, chrome)
- GitHub Check: Static analysis
- GitHub Check: Publish with pkg-pr-new
- GitHub Check: Unit Tests (22, **)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/clerk-js/src/core/resources/Token.ts (1)
3-5: Consider separating import path refactoring.The import path changes appear unrelated to the skipCache feature. While not problematic, mixing refactoring with feature changes can make PRs harder to review and revert if needed.
Consider moving import path updates to a separate commit or PR for cleaner history.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/clerk-js/src/core/constants.ts(1 hunks)packages/clerk-js/src/core/resources/Token.ts(1 hunks)packages/clerk-js/src/core/resources/__tests__/Token.test.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/clerk-js/src/core/constants.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/clerk-js/src/core/resources/__tests__/Token.test.ts (3)
packages/clerk-js/src/test/core-fixtures.ts (1)
mockFetch(262-272)packages/clerk-js/src/core/fapiClient.ts (1)
createFapiClient(80-299)packages/clerk-js/src/core/resources/Token.ts (1)
Token(7-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (6)
packages/clerk-js/src/core/resources/__tests__/Token.test.ts (5)
4-4: LGTM: Import addition supports new test cases.The
mockJwtimport is properly added to support the new token creation tests.
47-47: LGTM: Improved type precision.The explicit
ReturnType<typeof vi.spyOn>type annotation improves type safety.
126-134: Good coverage of default behavior.This test correctly validates that the
skipCacheparameter defaults tofalseand the query parameter is not added to the URL. However, the same query parameter naming concern from the previous test applies here.
136-154: Comprehensive test coverage for skipCache parameter.These tests properly validate both explicit
skipCache=trueandskipCache=falsebehaviors. The test coverage is thorough, checking both positive and negative cases.
107-124: I need to check the PR description and any related documentation to verify the claimed discrepancy.Code and tests are consistent; the review comment's premise cannot be verified.
The implementation in
Token.tsusesdebug=skip_cacheconsistently, and all tests verify this parameter correctly. Tests on line 116 confirm the URL should NOT contain this parameter whenskipCache=false(the default), and line 143 confirms it SHOULD appear whenskipCache=true. The test at lines 107-124 correctly validates the default behavior.The review comment claims the PR description specifies
_clerk_skip_cache=true, but this parameter name does not appear anywhere in the codebase or in publicly available Clerk documentation. The actual implementation usesdebug=skip_cache, which is the parameter consistently tested throughout the test file.Without access to the specific PR description referenced in the review, the claimed discrepancy cannot be verified. The code itself is internally consistent.
Likely an incorrect or invalid review comment.
packages/clerk-js/src/core/resources/Token.ts (1)
15-20: Good: Explicit parameter passing improves clarity.Making the
method,path, andsearchparameters explicit in the_fetchcall improves code clarity and enables the skipCache feature.
Description
Adding
debug=skip_cachequery string param to token requests that are explicitly invoked withskipCacheoptionChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.