fix(shared): Prevent stale closures in useReverification hook #6201
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
bratsos
changed the title
| @@ -14,7 +14,7 @@ const CLERK_API_REVERIFICATION_ERROR_CODE = 'session_reverification_required'; | |||
| async function resolveResult<T>(result: Promise<T> | T): Promise<T | ReturnType<typeof reverificationError>> { | |||
| try { | |||
| const r = await result; | |||
| if (r instanceof Response) { | |||
| if (typeof Response !== 'undefined' && r instanceof Response) { | |||
There was a problem hiding this comment.
❓ Just curious, did this ever run in an env where Response was not defined as a global identifier?
There was a problem hiding this comment.
Great question! My tests were failing without this, and after digging a bit deeper, I figured that even though we're using jsdom fetch is not supported and its maintainers have confirmed they won't be using Node's native fetch. This is because they need their own spec-compliant implementation that integrates with jsdom's internals, so the missing Response global is intentional.
Instead of changing the production code to handle this, the best solution was to patch our test environment directly with Node's native fetch APIs (which are stable since v18). This makes the tests pass without altering the hook's logic.
Thanks for the prompt, it definitely led to a better solution 🎉
Configures Jest to use a custom test environment that patches the JSDOM global scope with `fetch`, `Request`, and `Response` from Node.js. Since Node.js v18+ provides these fetch-related APIs as stable globals, we can reliably polyfill the test environment to ensure they are always available
📝 WalkthroughWalkthroughA new custom Jest environment, 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🧰 Additional context used📓 Path-based instructions (6)`**/*.{js,ts,tsx,jsx}`: All code must pass ESLint checks with the project's configuration. Use Prettier for consistent code formatting.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc) List of files the instruction was applied to:
`packages/**`: All publishable packages under the @clerk namespace must be located in the packages/ directory.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/global.mdc) List of files the instruction was applied to:
`**/*.{ts,tsx}`: Maintain comprehensive JSDoc comments for public APIs.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc) List of files the instruction was applied to:
`**/*.ts`: Always define explicit return types for functions, especially public ...
📄 Source: CodeRabbit Inference Engine (.cursor/rules/typescript.mdc) List of files the instruction was applied to:
`**/*.{test,spec}.{js,ts,tsx,jsx}`: Unit tests are required for all new functionality.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc) List of files the instruction was applied to:
`**/__tests__/**/*.{js,ts,tsx,jsx}`: Test files should be co-located with source files or in `__tests__` directories.
📄 Source: CodeRabbit Inference Engine (.cursor/rules/development.mdc) List of files the instruction was applied to:
🧠 Learnings (4)📓 Common learningspackages/shared/jest.config.js (11)packages/shared/src/react/__tests__/useReverification.test.ts (11)packages/shared/src/react/hooks/useReverification.ts (11)🔇 Additional comments (7)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
@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: |
Description
Addresses a stale closure bug within the
useReverificationhook and improves the stability of our test environment.The primary issue was that the hook could capture an initial version of the fetcher function and would not use the updated version on subsequent re-renders.
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
ResponseAPI during tests.Bug Fixes
useReverificationhook by updating its internal logic for handler creation and invocation.Tests
useReverificationhook.