FIx errors when turnstile token expiring near checkout time#304
FIx errors when turnstile token expiring near checkout time#304devksingh4 merged 1 commit intomainfrom
Conversation
WalkthroughThis pull request introduces a new Turnstile widget component in Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Deploying with Cloudflare Pages
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/TurnstileWidget.tsx (1)
7-55:tokenstate is set but never consumed by the only current caller
StoreItem.tsxexclusively usesensureFreshToken()and never readsturnstile.token. ThesetTokencalls insideensureFreshTokenandonSuccessupdate React state (triggering re-renders) without any consumer reacting to those changes. If no component needs the reactivetokenvalue, you could dropsetTokenfromensureFreshTokento avoid unnecessary re-renders during the refresh flow. If you plan to usetokenelsewhere (e.g., to gate the checkout button), this is fine — just noting the current re-render cost.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TurnstileWidget.tsx` around lines 7 - 55, The token state is updated in useTurnstile but never consumed by callers like StoreItem.tsx, causing unnecessary re-renders; remove the redundant setToken and tokenObtainedAt mutations from ensureFreshToken (leave onSuccess/resetToken behavior intact so explicit success/expire flows still update state if you later consume token) and have ensureFreshToken simply await and return ref.current?.getResponsePromise(...) or ref.current?.getResponse() without calling setToken or updating tokenObtainedAt; update references to ensureFreshToken, setToken, onSuccess, resetToken, token, and ref in that function accordingly.src/components/store/StoreItem.tsx (1)
946-953: Minor:idprop is the product ID — consider a dedicated widget ID
TurnstileWidgetreceivesid={id}whereidis the store product ID. This becomes the HTMLidattribute on the Turnstile iframe container. If a product ID ever contains characters invalid for HTML IDs (spaces, special chars) or collides with another DOM element, it could cause subtle issues. A namespaced ID would be safer:- <TurnstileWidget - id={id} + <TurnstileWidget + id={`turnstile-${id}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/store/StoreItem.tsx` around lines 946 - 953, TurnstileWidget is being passed the product `id` prop (variable `id`) which may contain characters invalid for an HTML id or collide with other elements; update the prop passed to TurnstileWidget (component TurnstileWidget in this JSX) to use a dedicated, namespaced/sanitized id such as prefixing with a constant (e.g. `turnstile-${id}`) or applying a simple sanitizer that replaces non-alphanumeric characters, so the rendered HTML id is safe and unique while still referencing the product id via the namespaced value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/TurnstileWidget.tsx`:
- Around line 23-44: The logic in ensureFreshToken duplicates reset behavior;
replace the inline reset steps with a call to the existing resetToken() helper:
when isStale is true, invoke resetToken() (which should perform
setToken(undefined), tokenObtainedAt.current = 0, and ref.current?.reset()),
then await and return ref.current?.getResponsePromise(60_000) inside the
try/catch as before; keep the final return of ref.current?.getResponse()
unchanged and retain ensureFreshToken's signature and error handling.
---
Nitpick comments:
In `@src/components/store/StoreItem.tsx`:
- Around line 946-953: TurnstileWidget is being passed the product `id` prop
(variable `id`) which may contain characters invalid for an HTML id or collide
with other elements; update the prop passed to TurnstileWidget (component
TurnstileWidget in this JSX) to use a dedicated, namespaced/sanitized id such as
prefixing with a constant (e.g. `turnstile-${id}`) or applying a simple
sanitizer that replaces non-alphanumeric characters, so the rendered HTML id is
safe and unique while still referencing the product id via the namespaced value.
In `@src/components/TurnstileWidget.tsx`:
- Around line 7-55: The token state is updated in useTurnstile but never
consumed by callers like StoreItem.tsx, causing unnecessary re-renders; remove
the redundant setToken and tokenObtainedAt mutations from ensureFreshToken
(leave onSuccess/resetToken behavior intact so explicit success/expire flows
still update state if you later consume token) and have ensureFreshToken simply
await and return ref.current?.getResponsePromise(...) or
ref.current?.getResponse() without calling setToken or updating tokenObtainedAt;
update references to ensureFreshToken, setToken, onSuccess, resetToken, token,
and ref in that function accordingly.
| const ensureFreshToken = useCallback(async (): Promise< | ||
| string | undefined | ||
| > => { | ||
| const age = Date.now() - tokenObtainedAt.current; | ||
| const isStale = | ||
| !ref.current?.getResponse() || | ||
| ref.current?.isExpired() || | ||
| age >= TOKEN_LIFETIME_MS - REFRESH_BUFFER_MS; | ||
|
|
||
| if (isStale) { | ||
| setToken(undefined); | ||
| tokenObtainedAt.current = 0; | ||
| ref.current?.reset(); | ||
| try { | ||
| return await ref.current?.getResponsePromise(60_000); | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| return ref.current?.getResponse(); | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the TurnstileWidget.tsx file
find . -name "TurnstileWidget.tsx" -type fRepository: acm-uiuc/website
Length of output: 96
🏁 Script executed:
# Check package.json for `@marsidev/react-turnstile` version
cat package.json | grep -A 2 -B 2 "react-turnstile"Repository: acm-uiuc/website
Length of output: 263
🏁 Script executed:
# Read the TurnstileWidget.tsx file to see the actual code
cat -n src/components/TurnstileWidget.tsx | head -100Repository: acm-uiuc/website
Length of output: 2669
🌐 Web query:
@marsidev/react-turnstile getResponsePromise reset method API documentation
💡 Result:
@marsidev/react-turnstile documents both getResponsePromise() and reset() under “useRef methods” (the ref API exposed by the <Turnstile /> component). (docs.page)
getResponsePromise(): “Returns the widget’s response as a promise, it waits until the widget is rendered and solved. It has a timeout of 30 seconds.”
Return type:Promise<string>. (docs.page)reset(): “Resets the widget. Useful if a given widget has timed out, expired or needs to be reloaded.”
Return type:void. (docs.page)
Citations:
- 1: https://docs.page/marsidev/react-turnstile/use-ref-methods?utm_source=openai
- 2: https://docs.page/marsidev/react-turnstile/use-ref-methods?utm_source=openai
- 3: https://docs.page/marsidev/react-turnstile/use-ref-methods?utm_source=openai
Refactor to reuse resetToken() instead of duplicating reset logic
Lines 33–35 duplicate the resetToken function. Extract to avoid maintenance drift:
Suggested refactor
if (isStale) {
- setToken(undefined);
- tokenObtainedAt.current = 0;
- ref.current?.reset();
+ resetToken();
try {
return await ref.current?.getResponsePromise(60_000);The library's reset() → getResponsePromise() sequence is designed for this use case per the docs, so no race condition is expected.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const ensureFreshToken = useCallback(async (): Promise< | |
| string | undefined | |
| > => { | |
| const age = Date.now() - tokenObtainedAt.current; | |
| const isStale = | |
| !ref.current?.getResponse() || | |
| ref.current?.isExpired() || | |
| age >= TOKEN_LIFETIME_MS - REFRESH_BUFFER_MS; | |
| if (isStale) { | |
| setToken(undefined); | |
| tokenObtainedAt.current = 0; | |
| ref.current?.reset(); | |
| try { | |
| return await ref.current?.getResponsePromise(60_000); | |
| } catch { | |
| return undefined; | |
| } | |
| } | |
| return ref.current?.getResponse(); | |
| }, []); | |
| const ensureFreshToken = useCallback(async (): Promise< | |
| string | undefined | |
| > => { | |
| const age = Date.now() - tokenObtainedAt.current; | |
| const isStale = | |
| !ref.current?.getResponse() || | |
| ref.current?.isExpired() || | |
| age >= TOKEN_LIFETIME_MS - REFRESH_BUFFER_MS; | |
| if (isStale) { | |
| resetToken(); | |
| try { | |
| return await ref.current?.getResponsePromise(60_000); | |
| } catch { | |
| return undefined; | |
| } | |
| } | |
| return ref.current?.getResponse(); | |
| }, []); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/TurnstileWidget.tsx` around lines 23 - 44, The logic in
ensureFreshToken duplicates reset behavior; replace the inline reset steps with
a call to the existing resetToken() helper: when isStale is true, invoke
resetToken() (which should perform setToken(undefined), tokenObtainedAt.current
= 0, and ref.current?.reset()), then await and return
ref.current?.getResponsePromise(60_000) inside the try/catch as before; keep the
final return of ref.current?.getResponse() unchanged and retain
ensureFreshToken's signature and error handling.
1 participant
Summary by CodeRabbit