fix: ensures turnstile is completely disabled when NEXT_PUBLIC_TURNSTILE_ENABLED is false#2434
fix: ensures turnstile is completely disabled when NEXT_PUBLIC_TURNSTILE_ENABLED is false#2434
Conversation
📝 WalkthroughWalkthroughTurnstile now short-circuits when disabled to return a fixed disabled token, refactors renderAndWaitResponse to use explicit success/error listeners with mutual cleanup, updates the imperative handle dependency, adds a test for the disabled path, and updates non-functional env/config comments and samples. Changes
Sequence Diagram(s)(omitted — changes are localized to a single component and tests; no multi-component sequence requiring visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
Comment |
stalniy
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2434 +/- ##
==========================================
- Coverage 51.25% 50.98% -0.28%
==========================================
Files 1072 1062 -10
Lines 29398 29055 -343
Branches 6476 6416 -60
==========================================
- Hits 15069 14813 -256
+ Misses 13934 13847 -87
Partials 395 395
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
stalniy
force-pushed
the
fix/turnstile-enable
branch
from
1571842 to
82b0abb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/deploy-web/env/.env.sample (1)
49-52: Reorder environment variables alphabetically.The static analysis tool flagged that
E2E_TESTING_CLIENT_TOKEN(line 52) should come beforeNEXT_PUBLIC_TURNSTILE_ENABLED(line 49) to maintain alphabetical key ordering. While this has no functional impact, sorting them will improve consistency with linter expectations.🔎 Proposed fix
-NEXT_PUBLIC_TURNSTILE_ENABLED= -NEXT_PUBLIC_TURNSTILE_SITE_KEY= -TURNSTILE_SECRET_KEY= -E2E_TESTING_CLIENT_TOKEN= +E2E_TESTING_CLIENT_TOKEN= +NEXT_PUBLIC_TURNSTILE_ENABLED= +NEXT_PUBLIC_TURNSTILE_SITE_KEY= +TURNSTILE_SECRET_KEY=apps/deploy-web/src/components/turnstile/Turnstile.tsx (1)
75-88: Good refactor for memory leak fix.The explicit listener cleanup pattern correctly removes both listeners when either event fires, addressing the memory leak.
However, the
statusvariable captured inerrorListener(line 84) reflects the state whenrenderAndWaitResponsewas called, not when the error occurs. If the status changes between the call and the error event, the rejected object will contain stale state.🔎 Proposed fix to capture status from the error event context
If the current status at error time is needed, consider dispatching it as part of the event detail from
onError/onExpirehandlers, rather than relying on the closure:const errorListener = (event: Event) => { eventBus.current.removeEventListener("success", successListener); eventBus.current.removeEventListener("error", errorListener); const details = (event as CustomEvent<{ reason: string; error?: string }>).detail; - reject({ status, ...details }); + reject(details); };Alternatively, if
statuscontext is valuable, ensure the dispatching handlers include it in the event detail.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/deploy-web/.env.local.sampleapps/deploy-web/env/.env.sampleapps/deploy-web/src/components/turnstile/Turnstile.spec.tsxapps/deploy-web/src/components/turnstile/Turnstile.tsxapps/deploy-web/src/config/browser-env.config.tsapps/deploy-web/src/config/server-env.config.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/deploy-web/src/config/server-env.config.ts
- apps/deploy-web/src/components/turnstile/Turnstile.spec.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/components/turnstile/Turnstile.tsxapps/deploy-web/src/config/browser-env.config.ts
🧠 Learnings (4)
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, akashnetwork/env-loader is used at the top of next.config.js files to automatically load environment variables from env/.env files into process.env. SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env and are loaded this way, while only SENTRY_AUTH_TOKEN is handled as a GitHub secret in workflows.
Applied to files:
apps/deploy-web/.env.local.sampleapps/deploy-web/env/.env.sample
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env file and loaded by akashnetwork/env-loader, not as GitHub secrets. Only SENTRY_AUTH_TOKEN should be handled as a secret.
Applied to files:
apps/deploy-web/env/.env.sample
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Don't add unnecessary comments to the code.
Applied to files:
apps/deploy-web/src/config/browser-env.config.ts
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use deprecated methods from libraries.
Applied to files:
apps/deploy-web/src/config/browser-env.config.ts
🪛 dotenv-linter (4.0.0)
apps/deploy-web/env/.env.sample
[warning] 52-52: [UnorderedKey] The E2E_TESTING_CLIENT_TOKEN key should go before the NEXT_PUBLIC_TURNSTILE_ENABLED key
(UnorderedKey)
⏰ 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). (3)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/deploy-web/env/.env.sample (1)
49-52: Good security practice: empty template values.Using empty values in the sample file (rather than dummy credentials) is the right approach—it ensures developers must explicitly provide their own values and avoids accidentally committing test credentials.
apps/deploy-web/.env.local.sample (2)
41-43: Excellent use of Cloudflare's documented dummy test keys.The dummy test keys (
1x00000000000000000000AA,1x0000000000000000000AA) are the official Cloudflare test values, and the documentation link provides helpful context for local development. This is the correct approach for sample configuration files.
54-54: E2E testing token added appropriately.The new
E2E_TESTING_CLIENT_TOKENwith a placeholder value aligns with the test infrastructure and mirrors the addition to.env.sample. The placeholder is suitable for local development.apps/deploy-web/src/config/browser-env.config.ts (1)
3-3: LGTM!The deprecation notice update aligns with the broader config naming migration from
appConfigtopublicConfig.apps/deploy-web/src/components/turnstile/Turnstile.tsx (2)
69-71: LGTM!The early return correctly bypasses the widget when disabled and returns a consistent disabled token. This satisfies the PR objective.
92-97: LGTM!The dependency array correctly includes
enabledsince it's now used in the imperative handle. The early return at line 95-97 prevents unnecessary rendering when disabled, completing the fix.
…ILE_ENABLED is false
stalniy
force-pushed
the
fix/turnstile-enable
branch
from
82b0abb to
e40abab
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/deploy-web/env/.env.sample (1)
49-52: Consider reordering keys per linter suggestion.The dotenv-linter suggests E2E_TESTING_CLIENT_TOKEN should come before NEXT_PUBLIC_TURNSTILE_ENABLED for alphabetical consistency. However, the current grouping of Turnstile-related variables is also logical.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/deploy-web/.env.local.sampleapps/deploy-web/env/.env.sampleapps/deploy-web/src/components/turnstile/Turnstile.spec.tsxapps/deploy-web/src/components/turnstile/Turnstile.tsxapps/deploy-web/src/config/browser-env.config.tsapps/deploy-web/src/config/server-env.config.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/deploy-web/src/config/browser-env.config.ts
- apps/deploy-web/.env.local.sample
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/components/turnstile/Turnstile.spec.tsxapps/deploy-web/src/config/server-env.config.tsapps/deploy-web/src/components/turnstile/Turnstile.tsx
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/deploy-web/src/components/turnstile/Turnstile.spec.tsx
{apps/deploy-web,apps/provider-console}/**/*.spec.tsx
📄 CodeRabbit inference engine (.cursor/rules/query-by-in-tests.mdc)
Use
queryBymethods instead ofgetBymethods in test expectations
Files:
apps/deploy-web/src/components/turnstile/Turnstile.spec.tsx
🧠 Learnings (2)
📚 Learning: 2025-08-12T13:52:38.708Z
Learnt from: stalniy
Repo: akash-network/console PR: 1800
File: apps/deploy-web/next.config.js:163-165
Timestamp: 2025-08-12T13:52:38.708Z
Learning: In the Akash Console project, akashnetwork/env-loader is used at the top of next.config.js files to automatically load environment variables from env/.env files into process.env. SENTRY_ORG and SENTRY_PROJECT are stored as public configuration values in apps/deploy-web/env/.env and are loaded this way, while only SENTRY_AUTH_TOKEN is handled as a GitHub secret in workflows.
Applied to files:
apps/deploy-web/env/.env.sample
📚 Learning: 2025-11-25T17:45:39.561Z
Learnt from: CR
Repo: akash-network/console PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T17:45:39.561Z
Learning: Applies to **/*.{ts,tsx,js} : Never use deprecated methods from libraries.
Applied to files:
apps/deploy-web/src/config/server-env.config.ts
🧬 Code graph analysis (1)
apps/deploy-web/src/components/turnstile/Turnstile.spec.tsx (1)
apps/deploy-web/tests/ui/fixture/base-test.ts (1)
expect(13-13)
🪛 dotenv-linter (4.0.0)
apps/deploy-web/env/.env.sample
[warning] 52-52: [UnorderedKey] The E2E_TESTING_CLIENT_TOKEN key should go before the NEXT_PUBLIC_TURNSTILE_ENABLED key
(UnorderedKey)
⏰ 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). (3)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (6)
apps/deploy-web/src/config/server-env.config.ts (1)
5-5: LGTM!The deprecation notice update correctly guides developers to the new import path.
apps/deploy-web/src/components/turnstile/Turnstile.tsx (4)
69-71: LGTM! Clean short-circuit for disabled state.The early return with a fixed disabled token elegantly bypasses widget interaction when Turnstile is disabled, addressing the PR's goal to restore local development flow.
75-88: Excellent refactor of event listener pattern.The explicit
successListeneranderrorListenerfunctions with mutual cleanup prevent memory leaks by ensuring both listeners are removed regardless of which event fires first. This directly addresses the memory leak mentioned in the PR objectives.
92-92: Correct dependency array update.Adding
enabledto the dependency array ensures the imperative handle closure captures the current enabled state, which is necessary given the early return behavior at lines 69-71.
95-97: No action needed. Version 1.1.0 of @marsidev/react-turnstile is not deprecated and is compatible with current React versions. The methods used in this file (remove, render, execute) are standard ref methods with no deprecations between versions 1.1.0 and the latest stable releases. The lines specified (95-97) contain no library method calls—only a conditional early return.apps/deploy-web/src/components/turnstile/Turnstile.spec.tsx (1)
154-159: LGTM! Test coverage for disabled path.The test correctly verifies that
renderAndWaitResponseresolves with the disabled token when Turnstile is disabled, providing coverage for the new short-circuit behavior. The test follows all coding guidelines including proper naming conventions and the use of the setup function.
2 participants
Why
To fix local development flow and fix memory leak
Summary by CodeRabbit
Bug Fixes
Tests
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.