test(react): verify redirect behavior in AuthProvider - #176
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
More reviews will be available in 20 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughThis PR adds React Router integration for the auth provider by introducing a navigation-aware ChangesReact Router AuthProvider Integration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/react-router/test/context/provider.test.tsx (1)
86-112: 💤 Low valueRedundant tests overlap earlier cases.
calls navigate when no redirect prop is provided and redirect fn is invoked(Lines 86-96) duplicateswraps navigate as the built-in redirect...(Lines 56-66), andnavigate is NOT called when a custom redirect prop overrides the built-in(Lines 98-112) is a strict subset ofcalls the custom redirect fn...(Lines 68-84). Consider folding the unique assertions (toHaveBeenCalledOnce) into the earlier tests and dropping the duplicates to reduce maintenance surface.🤖 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/react-router/test/context/provider.test.tsx` around lines 86 - 112, Remove the two redundant tests ("calls navigate when no redirect prop is provided and redirect fn is invoked" and "navigate is NOT called when a custom redirect prop overrides the built-in") and fold their unique assertions into the earlier tests: add the toHaveBeenCalledOnce() assertion into the existing "wraps navigate as the built-in redirect..." test that renders <AuthProvider client={mockClient}> and uses MockProvider.mock.calls[0][0].redirect to invoke the built-in redirect, and ensure the "calls the custom redirect fn..." test that passes redirect={customRedirect} asserts mockNavigate was not called; this keeps AuthProvider, MockProvider.mock.calls access, mockNavigate checks and customRedirect behavior while removing duplicate test cases.
🤖 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.
Inline comments:
In `@packages/react-router/package.json`:
- Around line 15-17: Add the missing devDependencies for the Vitest tooling used
by the package.json scripts: declare "vitest" and "`@vitest/coverage-v8`" in this
package's devDependencies so the scripts "test", "test:watch", and
"test:coverage" can run in isolation; update the package.json devDependencies
block to pin the same versions used in the repo catalog and run install to
ensure the tooling is available for filtered/isolated test runs.
In `@packages/react-router/src/context.tsx`:
- Line 4: The wrapper currently defines const AuthProvider = ({ client,
children, initialSession, redirect }: AuthProviderProps) which drops the
generic; change it to declare the wrapper as AuthProvider<DefaultUser extends
User = User>(props: AuthProviderProps<DefaultUser>) (or const AuthProvider =
<DefaultUser extends User = User>(props: AuthProviderProps<DefaultUser>) => ...)
and forward those generic props to the upstream component as
<Provider<DefaultUser> ...> (preserving client, children, initialSession,
redirect). Also import User from '`@aura-stack/auth`' so the DefaultUser
constraint matches the base implementation.
---
Nitpick comments:
In `@packages/react-router/test/context/provider.test.tsx`:
- Around line 86-112: Remove the two redundant tests ("calls navigate when no
redirect prop is provided and redirect fn is invoked" and "navigate is NOT
called when a custom redirect prop overrides the built-in") and fold their
unique assertions into the earlier tests: add the toHaveBeenCalledOnce()
assertion into the existing "wraps navigate as the built-in redirect..." test
that renders <AuthProvider client={mockClient}> and uses
MockProvider.mock.calls[0][0].redirect to invoke the built-in redirect, and
ensure the "calls the custom redirect fn..." test that passes
redirect={customRedirect} asserts mockNavigate was not called; this keeps
AuthProvider, MockProvider.mock.calls access, mockNavigate checks and
customRedirect behavior while removing duplicate test cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d2a8ba4-8d8f-4b20-b74e-d2522498e672
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
packages/react-router/package.jsonpackages/react-router/src/client.tspackages/react-router/src/context.tsxpackages/react-router/test/context/provider.test.tsxpackages/react-router/test/context/redirect.test.tsxpackages/react-router/tsconfig.jsonpackages/react-router/vitest.config.tspackages/react/package.jsonpnpm-workspace.yaml
Description
This pull request adds test coverage for redirect behavior across the
AuthProviderimplementations provided by@aura-stack/reactand the React-based framework integrations built on top of it, including@aura-stack/nextand@aura-stack/react-router.The tests verify that authentication actions correctly invoke the configured redirect handler and that framework-specific navigation integrations behave as expected.
For Next.js, coverage includes both supported routing strategies:
useRouterfromnext/navigationuseRouterfromnext/routerFor React Router, the tests verify integration with the
useNavigatehook.Note
This PR was introduced as a follow-up to the custom redirect support added in
@aura-stack/react. The redirect functionality itself is not being modified in this PR; this update focuses exclusively on validating its behavior through automated tests.Key Changes
testtest:watchtest:coverageRelated PRs
redirectfunction via AuthProvider #175