feat(shared,ui): promote the Protect check runner to a public @clerk/shared/protect-check entry - #9391
feat(shared,ui): promote the Protect check runner to a public @clerk/shared/protect-check entry#9391mwickett wants to merge 1 commit into
Conversation
…shared/protect-check entry
🦋 Changeset detectedLatest commit: f6193d9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 23 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.
|
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@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
@clerk/sharedCurrent version: 4.28.1 Subpath
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6193d98ba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Moved to the public `@clerk/shared/protect-check` entry; this subpath is kept as an alias so | ||
| // existing imports keep resolving. |
There was a problem hiding this comment.
Remove the comment that restates the alias
This comment only narrates that the following re-export preserves the existing subpath, which is already evident from the code. Remove it rather than adding a multi-line restatement; repository guidance permits comments only for critical, non-obvious rationale and requires them to be terse.
AGENTS.md reference: AGENTS.md:L13-L13
Useful? React with 👍 / 👎.
📝 WalkthroughWalkthroughThe shared package adds a public Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/ui/src/components/SignIn/__tests__/SignInProtectCheck.test.tsx (1)
22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve the shared helper signature in both mocks.
Both tests cast
executeProtectCheckWithTimeouttoReturnType<typeof vi.fn>. This removes compile-time validation of mock parameters and return values. Usevi.mocked(executeProtectCheckWithTimeout)or an equivalentMockedFunctiontype in both files.
packages/ui/src/components/SignIn/__tests__/SignInProtectCheck.test.tsx#L22-L22: Replace the broad cast with a typed Vitest mock.packages/ui/src/components/SignUp/__tests__/SignUpProtectCheck.test.tsx#L23-L23: Replace the broad cast with a typed Vitest mock.As per coding guidelines, use precise TypeScript types and avoid broad mock types when a narrower type is available.
🤖 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/ui/src/components/SignIn/__tests__/SignInProtectCheck.test.tsx` at line 22, Replace the broad ReturnType<typeof vi.fn> casts for executeProtectCheckWithTimeout with vi.mocked or an equivalent precise MockedFunction type in both packages/ui/src/components/SignIn/__tests__/SignInProtectCheck.test.tsx:22-22 and packages/ui/src/components/SignUp/__tests__/SignUpProtectCheck.test.tsx:23-23, preserving compile-time validation of the shared helper’s parameters and return value.Source: Coding guidelines
packages/shared/src/protect-check/executeProtectCheck.ts (1)
59-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
PROTECT_CHECK_ERROR_CODESfor all thrown error codes inexecuteProtectCheck.ts. Replace the ten hardcoded literals at lines 60, 65, 70, 108, 121, 126, 131, 144, 148, and 155.🤖 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/shared/src/protect-check/executeProtectCheck.ts` around lines 59 - 72, Update all thrown error objects in executeProtectCheck to use the corresponding constants from PROTECT_CHECK_ERROR_CODES instead of hardcoded error-code string literals, including the validation and later failure paths identified in the comment. Preserve each error’s existing semantic code while centralizing the values through that shared constant.
🤖 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/shared/src/protect-check/executeProtectCheck.ts`:
- Around line 59-72: Update all thrown error objects in executeProtectCheck to
use the corresponding constants from PROTECT_CHECK_ERROR_CODES instead of
hardcoded error-code string literals, including the validation and later failure
paths identified in the comment. Preserve each error’s existing semantic code
while centralizing the values through that shared constant.
In `@packages/ui/src/components/SignIn/__tests__/SignInProtectCheck.test.tsx`:
- Line 22: Replace the broad ReturnType<typeof vi.fn> casts for
executeProtectCheckWithTimeout with vi.mocked or an equivalent precise
MockedFunction type in both
packages/ui/src/components/SignIn/__tests__/SignInProtectCheck.test.tsx:22-22
and
packages/ui/src/components/SignUp/__tests__/SignUpProtectCheck.test.tsx:23-23,
preserving compile-time validation of the shared helper’s parameters and return
value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 307fd263-40d0-4c7f-9b07-63201917dc3d
📒 Files selected for processing (14)
.changeset/protect-check-public-entry.md.changeset/protect-check-ui-refactor.mdpackages/shared/package.jsonpackages/shared/src/internal/clerk-js/protectCheck.tspackages/shared/src/protect-check/__tests__/executeProtectCheck.test.tspackages/shared/src/protect-check/__tests__/lifecycle.test.tspackages/shared/src/protect-check/errors.tspackages/shared/src/protect-check/executeProtectCheck.tspackages/shared/src/protect-check/index.tspackages/shared/src/protect-check/lifecycle.tspackages/shared/tsdown.config.mtspackages/ui/src/components/SignIn/__tests__/SignInProtectCheck.test.tsxpackages/ui/src/components/SignUp/__tests__/SignUpProtectCheck.test.tsxpackages/ui/src/hooks/useProtectCheckRunner.ts
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
clerk/clerk_go(manual)clerk/dashboard(manual)clerk/accounts(manual)clerk/backoffice(manual)clerk/clerk(manual)clerk/clerk-docs(manual)clerk/cloudflare-workers(manual)clerk/cli(auto-detected)clerk/clerk-ios(auto-detected)clerk/clerk-android(auto-detected)
Description
Custom-flow (headless) apps can already see a Protect gate (
signIn.protectCheck/signUp.protectCheck) and submit a proof token (submitProtectCheck()), but the piece that actually runs a challenge lives on the internal@clerk/shared/internal/clerk-js/protectChecksubpath, and the lifecycle handling around it is private to@clerk/ui'suseProtectCheckRunner.This PR is the extraction step that unblocks a public custom-flow challenge API, with no behavioral changes:
@clerk/shared/protect-checkexposing:executeProtectCheck(moved verbatim; the internal subpath re-exports it as an alias so existing imports keep resolving)executeProtectCheckWithTimeout— container ownership (clears stale widgets), the script timeout race, one-way caller-signal linking, and the zombie-scriptsetWidgetVisibleguardsubmitProtectCheckProof— proof submission withprotect_check_already_resolvedtreated as soft success (reload + continue)isProtectCheckExpired,MAX_EXPIRED_RELOADS,PROTECT_CHECK_SCRIPT_TIMEOUT_MS,PROTECT_CHECK_ERROR_CODES@clerk/ui'suseProtectCheckRunnernow drives its lifecycle through those shared helpers and keeps only the React orchestration (effect keying on the challenge token, mount/cancel races,flushSyncvisibility handshake, card error state, the no-RHC fail-closed guard — which must stay in the consumer layer since@clerk/sharedcompiles with__BUILD_DISABLE_RHC__hard-codedfalse).import(), so no-RHC bundles (chrome-extension,clerk.no-rhc.js) continue to dead-code-eliminate the loader entirely.Follow-ups building on this entry:
resolveProtectCheck()resource methods for imperative/legacy flows and auseProtectCheck()hook in@clerk/shared/reactfor the hooks API.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change