fix(shared): correct isValidBrowser/isBrowserOnline for React Native#8086
Closed
chriscanin wants to merge 1 commit intomainfrom
Closed
fix(shared): correct isValidBrowser/isBrowserOnline for React Native#8086chriscanin wants to merge 1 commit intomainfrom
chriscanin wants to merge 1 commit intomainfrom
Conversation
…owser environments
In React Native, there is no `window` or `navigator`, so `isValidBrowser()` and
`isBrowserOnline()` were returning `false`. This caused core-3's token fetching
to skip FAPI calls for templated tokens (`getToken({ template })`) since it
believed the environment was invalid/offline.
Non-browser runtimes like React Native have their own networking layer and should
not be treated as bots or offline. Return `true` when browser APIs are absent.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: c3e2e81 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 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 |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@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: |
Contributor
📝 WalkthroughWalkthroughThis change modifies the behavior of browser detection functions in the shared package to support token retrieval in React Native environments. The 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/shared/src/browser.ts`:
- Around line 60-63: The functions isValidBrowser() and isBrowserOnline() should
not return true for all non-navigator runtimes; narrow the "true" case to only
React Native. Update isValidBrowser() and isBrowserOnline() to detect React
Native explicitly (e.g., check navigator?.product === 'ReactNative' or another
RN-specific signal) and return true only in that case, otherwise return false
for generic non-browser runtimes so code like the guard in webauthn.ts
("isValidBrowser() && typeof window.PublicKeyCredential === 'function'") does
not attempt to access window in SSR/Node environments; ensure the detection
logic is used in the existing functions named isValidBrowser and
isBrowserOnline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6d624320-01f8-40cd-9142-3372eccc1f4f
📒 Files selected for processing (3)
.changeset/fix-gettoken-template-rn.mdpackages/shared/src/__tests__/browser.spec.tspackages/shared/src/browser.ts
Comment on lines
+60
to
+63
| // Not in a browser environment (e.g. React Native, SSR). | ||
| // Return true so non-browser runtimes are not incorrectly | ||
| // treated as bots or automated browsers. | ||
| return true; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Find call sites that use isValidBrowser() as a browser/window guard:"
rg -nP --type=ts --type=tsx -C3 'isValidBrowser\(\)\s*&&\s*(typeof\s+window\.|window\.)'
echo
echo "2) Inspect key downstream paths impacted by non-browser=true behavior:"
rg -nP -C3 '\bisBrowserOnline\(\)|\bisValidBrowserOnline\(\)' \
packages/clerk-js/src/core/resources/Session.ts \
packages/clerk-js/src/core/fapiClient.ts \
packages/shared/src/webauthn.ts
echo
echo "Expected:"
echo "- If any server-side/SSR path relies on isValidBrowser() to guard window access,"
echo " current change is unsafe and should be narrowed to RN detection."Repository: clerk/javascript
Length of output: 176
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Step 1: Check actual code in browser.ts around lines 60-63 and 76-79 ==="
cat -n packages/shared/src/browser.ts | sed -n '50,85p'
echo
echo "=== Step 2: Find all imports and usages of isValidBrowser ==="
rg -n 'isValidBrowser' packages/ --max-count=20
echo
echo "=== Step 3: Find all imports and usages of isBrowserOnline ==="
rg -n 'isBrowserOnline' packages/ --max-count=20
echo
echo "=== Step 4: Check webauthn.ts guard pattern ==="
cat -n packages/shared/src/webauthn.ts | head -20Repository: clerk/javascript
Length of output: 7354
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Test expectations for isValidBrowser with no navigator ==="
cat -n packages/shared/src/__tests__/browser.spec.ts | sed -n '40,75p'
echo
echo "=== Check if webauthn.ts has any build-time guards or SSR exclusion ==="
grep -r "webauthn" packages/ --include="*.ts" --include="*.tsx" --include="*.json" | grep -E "(exclude|server|ssr|node)" || echo "No SSR-specific guards found"
echo
echo "=== Check for React Native detection utility in codebase ==="
rg -n "ReactNative|react.native|react-native" packages/shared/src/ | head -20
echo
echo "=== Check inBrowser() implementation ==="
cat -n packages/shared/src/browser.ts | sed -n '1,50p'Repository: clerk/javascript
Length of output: 3163
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Where is isWebAuthnSupported() actually called? ==="
rg -rn "isWebAuthnSupported|webauthn" packages/ --type=ts --type=tsx --include="*.ts" --include="*.tsx" | grep -v "node_modules" | grep -v "test" | head -30
echo
echo "=== Check if webauthn module is conditionally loaded ==="
rg -rn "from.*webauthn|import.*webauthn" packages/ --include="*.ts" --include="*.tsx" | grep -v test
echo
echo "=== Full webauthn.ts to understand usage pattern ==="
cat packages/shared/src/webauthn.tsRepository: clerk/javascript
Length of output: 574
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Where is isWebAuthnSupported() or webauthn functions called? ==="
rg -n "isWebAuthnSupported|isWebAuthnAutofillSupported" packages/
echo
echo "=== Check all webauthn imports ==="
rg -n "from.*webauthn|import.*webauthn" packages/
echo
echo "=== Full webauthn.ts content ==="
cat packages/shared/src/webauthn.tsRepository: clerk/javascript
Length of output: 8269
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if webauthn.ts is server-side accessible or client-only ==="
rg -n "webauthn" packages/shared/package.json packages/shared/tsconfig.json 2>/dev/null || echo "Not in explicit exports"
echo
echo "=== Check how webauthn.ts is exported from `@clerk/shared` ==="
cat packages/shared/package.json | grep -A 20 '"exports"'
echo
echo "=== Look at complete test for isValidBrowser with no navigator ==="
cat -n packages/shared/src/__tests__/browser.spec.ts | sed -n '41,47p'
echo
echo "=== Does the test actually expect this behavior? Look at comment ==="
cat -n packages/shared/src/__tests__/browser.spec.ts | sed -n '40,50p'
echo
echo "=== Check if there's conditional execution or build-time webauthn exclusion ==="
grep -r "webauthn" packages/clerk-js/src --include="*.json" --include="*.config.*" 2>/dev/null || echo "No special build config for webauthn"
echo
echo "=== Key: check if window.PublicKeyCredential will actually throw in SSR ==="
echo "Testing: in Node.js SSR, accessing window.PublicKeyCredential"
node -e "try { console.log(typeof window.PublicKeyCredential); } catch(e) { console.log('Error:', e.message); }"Repository: clerk/javascript
Length of output: 2066
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Final verification: Can isWebAuthnSupported ever run in SSR? ==="
echo "Check if SignIn/Session resources are server-side initialized:"
rg -n "new SignIn\|new Session\|__internal_isWebAuthnSupported" packages/clerk-js/src/core/clerk.ts | head -15
echo
echo "=== Confirm: in SSR with new isValidBrowser() change, this will crash ==="
echo "Simulating: inBrowser()=false, navigator=null, then evaluating typeof window.PublicKeyCredential"
cat > /tmp/test.js << 'EOF'
// Simulate isValidBrowser() with new change
function isValidBrowser_NEW() {
// In SSR: inBrowser() is false, so navigator becomes null
const navigator = false ? null : null; // inBrowser() returns false
if (!navigator) {
return true; // NEW BEHAVIOR: return true for non-browser
}
return false;
}
// Now the guard in isWebAuthnSupported
function isWebAuthnSupported() {
return (
isValidBrowser_NEW() &&
typeof window.PublicKeyCredential === 'function' // <-- will crash
);
}
try {
console.log("Result:", isWebAuthnSupported());
} catch (e) {
console.log("CRASH:", e.message);
}
EOF
node /tmp/test.jsRepository: clerk/javascript
Length of output: 191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if clerk resources are instantiated in SSR initialization ==="
rg -n "initializeClerk|new SignIn|new Session" packages/clerk-js/src/core/clerk.ts | head -5
echo
echo "=== Confirm window check in isValidBrowser matches inBrowser pattern ==="
cat -n packages/shared/src/browser.ts | sed -n '57,66p'Repository: clerk/javascript
Length of output: 131
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Verify: is webauthn always guarded with typeof checks on import side? ==="
cat -n packages/clerk-js/src/core/resources/SignIn.ts | sed -n '70,80p'
echo
echo "=== Check full context of how isWebAuthnSupported is used in SignIn ==="
cat -n packages/clerk-js/src/core/resources/SignIn.ts | sed -n '514,525p'
echo
echo "=== Simplified: In SSR, would isWebAuthnSupported() be called? ==="
echo "Check if SignIn methods are accessible in SSR or only browser-side"
grep -n "class SignIn" packages/clerk-js/src/core/resources/SignIn.ts | head -1
</script>Repository: clerk/javascript
Length of output: 1639
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check: When are webauthn functions evaluated? At load time or at runtime? ==="
cat -n packages/shared/src/webauthn.ts
echo
echo "=== Check: Are these webauthn functions called during SSR server init? ==="
echo "They're exported from `@clerk/shared` and imported in clerk-js"
echo "But are they called during non-interactive SSR, or only during client-side flows?"
echo
echo "=== The critical question: Does the guard actually protect? ==="
echo "In webauthn.ts line 8:"
echo " isValidBrowser() && typeof window.PublicKeyCredential === 'function'"
echo ""
echo "With new isValidBrowser() = true in SSR:"
echo " true && typeof window.PublicKeyCredential === 'function'"
echo " ^ This evaluates to the right side"
echo " ^ But we're in Node.js where window is undefined"
echo " -> ReferenceError: window is not defined"Repository: clerk/javascript
Length of output: 1880
isValidBrowser() and isBrowserOnline() returning true for all non-navigator cases breaks webauthn guard semantics
Lines 60 and 76 returning true for all non-browser runtimes (not just React Native) creates a safety issue. In packages/shared/src/webauthn.ts:8–10, the guard pattern isValidBrowser() && typeof window.PublicKeyCredential === 'function' relies on the first condition being false in non-browser environments. With this change, SSR/Node.js environments will pass the first condition (true), then evaluate typeof window.PublicKeyCredential, which throws ReferenceError: window is not defined since typeof doesn't protect property access.
The test comment correctly identifies the intent ("React Native"), but the implementation conflates React Native with generic non-browser runtimes. Narrow the true return to React Native detection only:
+function isReactNativeRuntime(): boolean {
+ return typeof navigator !== 'undefined' && navigator.product === 'ReactNative';
+}
+
export function isValidBrowser(): boolean {
const navigator = inBrowser() ? window?.navigator : null;
if (!navigator) {
- return true;
+ return isReactNativeRuntime();
}
return !userAgentIsRobot(navigator?.userAgent) && !navigator?.webdriver;
}
export function isBrowserOnline(): boolean {
const navigator = inBrowser() ? window?.navigator : null;
if (!navigator) {
- return true;
+ return isReactNativeRuntime();
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/shared/src/browser.ts` around lines 60 - 63, The functions
isValidBrowser() and isBrowserOnline() should not return true for all
non-navigator runtimes; narrow the "true" case to only React Native. Update
isValidBrowser() and isBrowserOnline() to detect React Native explicitly (e.g.,
check navigator?.product === 'ReactNative' or another RN-specific signal) and
return true only in that case, otherwise return false for generic non-browser
runtimes so code like the guard in webauthn.ts ("isValidBrowser() && typeof
window.PublicKeyCredential === 'function'") does not attempt to access window in
SSR/Node environments; ensure the detection logic is used in the existing
functions named isValidBrowser and isBrowserOnline.
Member
Author
|
This PR is being closed in favor of #8084. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
isValidBrowser()andisBrowserOnline()in@clerk/sharedwere returningfalsewhenwindow/navigatorare undefined (React Native environments)falsecausedgetToken({ template: '...' })to silently fail on React Native whilegetToken()(which reads from local cache) continued to worktruein non-browser environments since React Native has its own networking layer and should not be treated as a bot or offlineReported by
Bledar via Discord —
getToken({ template: 'one_hour_token' })returns nothing on Expo after upgrading to latest ClerkTest plan
browser.spec.tstest to verifyisValidBrowser()returnstruewhen window is undefined@clerk/sharedpassgetToken({ template })works on React Native after fix🤖 Generated with Claude Code
Summary by CodeRabbit