feat(repo): Protect -> Show - WIP #7373
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
WalkthroughReplaces the Protect API with a Show-based API across packages: types moved from ProtectProps to ProtectParams, a new Show component and ShowWhenCondition were introduced, exports and re-exports updated, server/client boundaries adjusted, and a codemod plus tests were added to migrate existing Protect usage to Show. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
jacekradko
changed the title
@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/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: |
nikosdouvlis
force-pushed
the
vincent-and-the-doctor
branch
from
d24d455 to
12a12f5
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: 3
🧹 Nitpick comments (10)
packages/upgrade/src/codemods/transform-protect-to-show.cjs (4)
96-106: Aliased import handling may incorrectly rename local binding.When a user has
import { Protect as MyProtect } from "@clerk/react", the current logic will renamespec.local.nameto'Show'only if it equals'Protect'. This is correct behavior. However, if the user has an alias, the JSX transformation (lines 113-116) only checks for<Protect>, not for the aliased name. The import will be updated to{ Show as MyProtect }but JSX using<MyProtect>won't be transformed.This edge case may cause confusion—consider either:
- Tracking aliased names and transforming JSX accordingly, or
- Adding a test fixture documenting this limitation.
149-171: Condition prop silently takes precedence over auth attributes.When both
conditionand auth attributes (e.g.,role,permission) are present, theconditionis used and auth attributes are silently discarded. Consider emitting a warning or adding a comment explaining this precedence, as users may not expect their auth attributes to be ignored.
158-165: Boolean shorthand attributes produce empty string instead oftrue.For JSX boolean shorthand like
<Protect role>(equivalent torole={true}),attr.valueisnull. The fallback on line 164 would producej.stringLiteral('')instead ofj.booleanLiteral(true).While boolean shorthand is unlikely for these specific props, consider handling it explicitly:
} else if (j.StringLiteral.check(attr.value) || j.Literal.check(attr.value)) { value = attr.value; + } else if (attr.value === null) { + // Boolean shorthand: <Protect role> means role={true} + value = j.booleanLiteral(true); } else { // Default string value value = j.stringLiteral(attr.value?.value || ''); }
1-4: Consider adding@clerk/expoto HYBRID_PACKAGES if Expo supports RSC in the future.Currently
@clerk/expois inCLIENT_ONLY_PACKAGES. This is correct for now, but as React Server Components may come to React Native/Expo in the future, this categorization may need revisiting. A comment noting this assumption would be helpful for future maintainers.packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js (1)
1-10: Consider adding edge case fixtures for completeness.The current fixtures cover common scenarios well. Consider adding test cases for:
- Multiple auth attributes combined:
<Protect role="admin" permission="org:read">to verify they're merged into thewhenobject- Aliased import:
import { Protect as P }to document current behavior- JSX spread attributes:
<Protect {...props} role="admin">to verify spreads are preserved- Import without JSX usage: Verifies import-only transformation works
These would help prevent regressions and document edge case behavior.
Example fixture for multiple auth attributes:
{ name: 'Multiple auth attributes combined', source: ` import { Protect } from "@clerk/react" function App() { return ( <Protect role="admin" permission="org:read"> <Content /> </Protect> ) } `, output: ` import { Show } from "@clerk/react" function App() { return ( <Show when={{ role: "admin", permission: "org:read" }}> <Content /> </Show> ); } `, },packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsx (1)
84-117: Consider adding negative test cases for authorization failures.The tests verify positive authorization scenarios (when
has()and predicate returntrue), but don't cover when authorization fails. Adding tests forhas().mockReturnValue(false)andpredicate.mockReturnValue(false)would verify fallback rendering in authorization failure scenarios.it('renders fallback when has() returns false', async () => { const has = vi.fn().mockReturnValue(false); setAuthReturn({ has, userId: 'user_123' }); const html = await render( Show({ children: <div>authorized</div>, fallback: <div>fallback</div>, treatPendingAsSignedOut: false, when: { role: 'admin' }, }), ); expect(html).toContain('fallback'); }); it('renders fallback when predicate returns false', async () => { const has = vi.fn(); const predicate = vi.fn().mockReturnValue(false); setAuthReturn({ has, userId: 'user_123' }); const html = await render( Show({ children: <div>predicate-pass</div>, fallback: <div>fallback</div>, treatPendingAsSignedOut: false, when: predicate, }), ); expect(html).toContain('fallback'); });packages/nextjs/src/components.client.ts (1)
17-21: Add explicit return type and props type for consistency.The
Protectcomponent lacks an explicit return type annotation. As per coding guidelines, explicit return types should be defined for public APIs. Additionally, consider adding a props type to make the API signature clearer, even though props are ignored.-export const Protect = () => { +export const Protect = (_props?: Record<string, unknown>): never => { throw new Error( '`<Protect>` is only available as a React Server Component. For client components, use `<Show when={...} />` instead.', ); };packages/shared/src/types/protect.ts (1)
110-113: Consider using a more specific type forfallback.The
fallbackprop is typed asunknown, which is overly permissive and may lead to type errors when consumers try to use it. Since this is typically a React node, consider using a framework-agnostic type or generic.Since this is in the shared types package (framework-agnostic), you could:
- Use a generic type parameter for framework-specific rendering
- Or use
React.ReactNodeif React is the primary consumer-export type ShowProps = PendingSessionOptions & { - fallback?: unknown; +export type ShowProps<TFallback = unknown> = PendingSessionOptions & { + fallback?: TFallback; when: ShowWhenCondition; };This allows framework-specific packages to narrow the type appropriately.
packages/nextjs/src/app-router/server/controlComponents.tsx (1)
88-112: Consider removing unnecessaryresolvedWhenvariable.Line 92 assigns
resolvedWhen = whenwithout any transformation. This appears to be an identity assignment that adds no value. Consider usingwhendirectly throughout the function for clarity.Apply this diff if you choose to simplify:
export async function Show(props: AppRouterShowProps): Promise<React.JSX.Element | null> { const { children, fallback, treatPendingAsSignedOut, when } = props; const { has, userId } = await auth({ treatPendingAsSignedOut }); - const resolvedWhen = when; const authorized = <>{children}</>; const unauthorized = fallback ? <>{fallback}</> : null; - if (typeof resolvedWhen === 'string') { - if (resolvedWhen === 'signedOut') { + if (typeof when === 'string') { + if (when === 'signedOut') { return userId ? unauthorized : authorized; } return userId ? authorized : unauthorized; } if (!userId) { return unauthorized; } - if (typeof resolvedWhen === 'function') { - return resolvedWhen(has) ? authorized : unauthorized; + if (typeof when === 'function') { + return when(has) ? authorized : unauthorized; } - return has(resolvedWhen) ? authorized : unauthorized; + return has(when) ? authorized : unauthorized; }Note: The same pattern exists in the client implementation at
packages/react/src/components/controlComponents.tsx:107, so consider applying this refactor consistently across both files.packages/react/src/components/controlComponents.tsx (1)
130-138: LGTM! Helper function provides good type safety.The
checkAuthorizationhelper correctly:
- Excludes string cases via
Exclude<ShowWhenCondition, 'signedIn' | 'signedOut'>, ensuring type safety- Properly types the
hasparameter usingNonNullable<ReturnType<typeof useAuth>['has']>- Handles both function and ProtectParams cases
Consider extracting a similar helper in the server implementation (
packages/nextjs/src/app-router/server/controlComponents.tsxlines 107-112) for consistency:// In server file function checkAuthorization( when: Exclude<ShowWhenCondition, 'signedIn' | 'signedOut'>, has: ReturnType<typeof auth>['has'], ): boolean { if (typeof when === 'function') { return when(has); } return has(when); }This would make both implementations structurally identical and easier to maintain.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (3)
packages/chrome-extension/src/__tests__/__snapshots__/exports.test.ts.snapis excluded by!**/*.snappackages/react-router/src/__tests__/__snapshots__/exports.test.ts.snapis excluded by!**/*.snappackages/tanstack-react-start/src/__tests__/__snapshots__/exports.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
packages/astro/src/react/controlComponents.tsx(3 hunks)packages/chrome-extension/src/react/re-exports.ts(1 hunks)packages/expo/src/components/controlComponents.tsx(1 hunks)packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsx(1 hunks)packages/nextjs/src/app-router/server/controlComponents.tsx(3 hunks)packages/nextjs/src/client-boundary/controlComponents.ts(1 hunks)packages/nextjs/src/components.client.ts(1 hunks)packages/nextjs/src/components.server.ts(1 hunks)packages/nextjs/src/index.ts(2 hunks)packages/react/src/components/controlComponents.tsx(2 hunks)packages/react/src/components/index.ts(1 hunks)packages/shared/src/types/protect.ts(3 hunks)packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js(1 hunks)packages/upgrade/src/codemods/__tests__/transform-protect-to-show.test.js(1 hunks)packages/upgrade/src/codemods/transform-protect-to-show.cjs(1 hunks)packages/vue/src/components/controlComponents.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.jspackages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/react/src/components/index.tspackages/nextjs/src/components.server.tspackages/nextjs/src/app-router/server/controlComponents.tsxpackages/vue/src/components/controlComponents.tspackages/upgrade/src/codemods/__tests__/transform-protect-to-show.test.jspackages/chrome-extension/src/react/re-exports.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/nextjs/src/components.client.tspackages/nextjs/src/index.tspackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/shared/src/types/protect.tspackages/react/src/components/controlComponents.tsx
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.jspackages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/react/src/components/index.tspackages/nextjs/src/components.server.tspackages/nextjs/src/app-router/server/controlComponents.tsxpackages/vue/src/components/controlComponents.tspackages/upgrade/src/codemods/__tests__/transform-protect-to-show.test.jspackages/chrome-extension/src/react/re-exports.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/nextjs/src/components.client.tspackages/nextjs/src/index.tspackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/shared/src/types/protect.tspackages/react/src/components/controlComponents.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.jspackages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/react/src/components/index.tspackages/nextjs/src/components.server.tspackages/nextjs/src/app-router/server/controlComponents.tsxpackages/vue/src/components/controlComponents.tspackages/upgrade/src/codemods/__tests__/transform-protect-to-show.test.jspackages/chrome-extension/src/react/re-exports.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/nextjs/src/components.client.tspackages/nextjs/src/index.tspackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/shared/src/types/protect.tspackages/react/src/components/controlComponents.tsx
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.jspackages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/react/src/components/index.tspackages/nextjs/src/components.server.tspackages/nextjs/src/app-router/server/controlComponents.tsxpackages/vue/src/components/controlComponents.tspackages/upgrade/src/codemods/__tests__/transform-protect-to-show.test.jspackages/chrome-extension/src/react/re-exports.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/nextjs/src/components.client.tspackages/nextjs/src/index.tspackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/shared/src/types/protect.tspackages/react/src/components/controlComponents.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.jspackages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/react/src/components/index.tspackages/nextjs/src/components.server.tspackages/nextjs/src/app-router/server/controlComponents.tsxpackages/vue/src/components/controlComponents.tspackages/upgrade/src/codemods/__tests__/transform-protect-to-show.test.jspackages/chrome-extension/src/react/re-exports.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/nextjs/src/components.client.tspackages/nextjs/src/index.tspackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/shared/src/types/protect.tspackages/react/src/components/controlComponents.tsx
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.jspackages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/react/src/components/index.tspackages/nextjs/src/components.server.tspackages/nextjs/src/app-router/server/controlComponents.tsxpackages/vue/src/components/controlComponents.tspackages/upgrade/src/codemods/__tests__/transform-protect-to-show.test.jspackages/chrome-extension/src/react/re-exports.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/nextjs/src/components.client.tspackages/nextjs/src/index.tspackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/shared/src/types/protect.tspackages/react/src/components/controlComponents.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/react/src/components/index.tspackages/nextjs/src/components.server.tspackages/nextjs/src/app-router/server/controlComponents.tsxpackages/vue/src/components/controlComponents.tspackages/chrome-extension/src/react/re-exports.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/nextjs/src/components.client.tspackages/nextjs/src/index.tspackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/shared/src/types/protect.tspackages/react/src/components/controlComponents.tsx
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{test,spec}.{ts,tsx,js,jsx}: Unit tests are required for all new functionality
Verify proper error handling and edge cases
Include tests for all new features
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/upgrade/src/codemods/__tests__/transform-protect-to-show.test.js
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/react/src/components/index.tspackages/nextjs/src/components.server.tspackages/nextjs/src/app-router/server/controlComponents.tsxpackages/vue/src/components/controlComponents.tspackages/chrome-extension/src/react/re-exports.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/nextjs/src/components.client.tspackages/nextjs/src/index.tspackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/shared/src/types/protect.tspackages/react/src/components/controlComponents.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.tsx: Use error boundaries in React components
Minimize re-renders in React components
**/*.tsx: Use proper type definitions for props and state in React components
Leverage TypeScript's type inference where possible in React components
Use proper event types for handlers in React components
Implement proper generic types for reusable React components
Use proper type guards for conditional rendering in React components
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/nextjs/src/app-router/server/controlComponents.tsxpackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/react/src/components/controlComponents.tsx
**/*.{test,spec,e2e}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use real Clerk instances for integration tests
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/upgrade/src/codemods/__tests__/transform-protect-to-show.test.js
**/*.{md,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Update documentation for API changes
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/nextjs/src/app-router/server/controlComponents.tsxpackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/react/src/components/controlComponents.tsx
**/*.test.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use React Testing Library for component testing
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components (e.g.,UserProfile,NavigationMenu)
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Separate UI components from business logic components
Use useState for simple state management in React components
Use useReducer for complex state logic in React components
Implement proper state initialization in React components
Use proper state updates with callbacks in React components
Implement proper state cleanup in React components
Use Context API for theme/authentication state management
Implement proper state persistence in React applications
Use React.memo for expensive components
Implement proper useCallback for handlers in React components
Use proper useMemo for expensive computations in React components
Implement proper virtualization for lists in React components
Use proper code splitting with React.lazy in React applications
Implement proper cleanup in useEffect hooks
Use proper refs for DOM access in React components
Implement proper event listener cleanup in React components
Use proper abort controllers for fetch in React components
Implement proper subscription cleanup in React components
Use proper HTML elements for semantic HTML in React components
Implement proper ARIA attributes for accessibility in React components
Use proper heading hierarchy in React components
Implement proper form labels in React components
Use proper button types in React components
Implement proper focus management for keyboard navigation in React components
Use proper keyboard shortcuts in React components
Implement proper tab order in React components
Use proper ...
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/nextjs/src/app-router/server/controlComponents.tsxpackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/react/src/components/controlComponents.tsx
**/*.{test,spec}.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{test,spec}.{jsx,tsx}: Use React Testing Library for unit testing React components
Test component behavior, not implementation details
Use proper test queries in React Testing Library tests
Implement proper test isolation in React component tests
Use proper test coverage in React component tests
Test component interactions in integration tests
Use proper test data in React component tests
Implement proper test setup in React component tests
Use proper test cleanup in React component tests
Implement proper test assertions in React component tests
Use proper test structure for React component tests
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsxpackages/react/src/components/index.tspackages/nextjs/src/components.server.tspackages/nextjs/src/app-router/server/controlComponents.tsxpackages/vue/src/components/controlComponents.tspackages/chrome-extension/src/react/re-exports.tspackages/nextjs/src/client-boundary/controlComponents.tspackages/nextjs/src/components.client.tspackages/nextjs/src/index.tspackages/expo/src/components/controlComponents.tsxpackages/astro/src/react/controlComponents.tsxpackages/shared/src/types/protect.tspackages/react/src/components/controlComponents.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use React Testing Library for component testing
Files:
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsx
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/react/src/components/index.tspackages/nextjs/src/index.ts
🧬 Code graph analysis (6)
packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsx (2)
packages/shared/src/types/protect.ts (1)
ShowWhenCondition(82-86)packages/shared/src/react/clerk-rq/use-clerk-query-client.ts (1)
has(46-48)
packages/nextjs/src/components.server.ts (2)
packages/nextjs/src/app-router/server/controlComponents.tsx (2)
Protect(47-83)Show(88-112)packages/react/src/components/controlComponents.tsx (1)
Show(98-128)
packages/nextjs/src/app-router/server/controlComponents.tsx (3)
packages/shared/src/types/protect.ts (2)
ProtectParams(28-70)ShowWhenCondition(82-86)packages/shared/src/types/session.ts (1)
PendingSessionOptions(35-42)packages/react/src/components/controlComponents.tsx (1)
Show(98-128)
packages/vue/src/components/controlComponents.ts (3)
packages/astro/src/react/controlComponents.tsx (1)
ProtectProps(69-71)packages/shared/src/types/protect.ts (2)
ProtectProps(75-75)ProtectParams(28-70)packages/shared/src/types/session.ts (1)
PendingSessionOptions(35-42)
packages/astro/src/react/controlComponents.tsx (2)
packages/shared/src/types/protect.ts (1)
ProtectParams(28-70)packages/shared/src/types/session.ts (1)
PendingSessionOptions(35-42)
packages/react/src/components/controlComponents.tsx (3)
packages/shared/src/types/protect.ts (2)
ShowProps(110-113)ShowWhenCondition(82-86)packages/nextjs/src/app-router/server/controlComponents.tsx (1)
Show(88-112)packages/react/src/hooks/useAuth.ts (1)
useAuth(95-124)
⏰ 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). (27)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (26)
packages/upgrade/src/codemods/transform-protect-to-show.cjs (2)
6-44: Comprehensive directive detection logic.The
hasUseClientDirectivefunction thoroughly handles multiple parser representations of the'use client'directive (ExpressionStatement with Literal/StringLiteral/DirectiveLiteral, directive field, and directives array). This ensures compatibility across different parser configurations.
66-106: Core transformation logic is sound.The approach of checking for hybrid package imports and skipping RSC files is correct. The import transformation properly handles the
Protect→Showrename including the local binding when not aliased.packages/upgrade/src/codemods/__tests__/transform-protect-to-show.test.js (1)
1-18: Clean test structure using parameterized tests.The test file properly uses
it.eachfor data-driven testing with fixtures, and correctly handles thenulloutput case for scenarios where no transformation should occur. The use of.trim()on output comparison handles whitespace differences cleanly.packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js (1)
1-329: Good coverage of common transformation scenarios.The fixtures cover the essential cases: basic imports, various auth props (
permission,role,feature,plan),conditionprop,fallbackpreservation, self-closing elements, dynamic values, and the critical RSC vs client component distinction for@clerk/nextjs.packages/nextjs/src/app-router/server/__tests__/controlComponents.test.tsx (1)
1-28: Well-structured test setup with proper mocking.The test infrastructure is clean: proper mocking of
auth, a reusablerenderhelper that handles async server components, and typed constants forShowWhenCondition. Good use ofafterEachfor mock cleanup.packages/react/src/components/index.ts (1)
31-36: LGTM!The export changes correctly expose
ShowandShowPropswhile removingProtectfrom the public API. The alphabetical ordering is maintained, and the type export is properly separated.packages/chrome-extension/src/react/re-exports.ts (1)
18-18: LGTM!The re-export of
Showfrom@clerk/reactis correctly added, maintaining alphabetical ordering and consistency with other packages.packages/expo/src/components/controlComponents.tsx (1)
1-1: LGTM!The Expo package correctly re-exports
Showfrom@clerk/react, consistent with the migration pattern across other packages.packages/vue/src/components/controlComponents.ts (2)
5-5: Type import update is correct.The import change from the internal
_ProtectPropstoProtectParamsaligns with the shared types refactoring.
115-161: Vue package should addShowcomponent for consistency with other packages.While React, Expo, Chrome Extension, and Next.js packages have migrated to the
Showcomponent, the Vue package still only exportsProtect. Add aShowcomponent (orShowalias) to maintain consistency across packages.packages/nextjs/src/components.server.ts (1)
2-11: LGTM!The server module correctly exports both
Protect(for backward compatibility) and the newShowcomponent. The type definitions properly reflect both exports, enabling gradual migration.packages/nextjs/src/index.ts (2)
17-18: LGTM!The addition of
Showto the client-boundary exports aligns with the PR objective to introduceShowas the client-facing component.
77-81: LGTM! Good documentation for the API distinction.The JSDoc clearly communicates that
<Protect/>is for RSC (App Router) and directs users to<Show when={...} />for client components. This helps prevent the misunderstanding mentioned in the linked issue.packages/nextjs/src/client-boundary/controlComponents.ts (1)
3-18: LGTM!The export list correctly includes
Show,SignedIn, andSignedOutfrom@clerk/react, aligning with the client-boundary pattern for the Next.js package.packages/nextjs/src/components.client.ts (1)
2-2: LGTM!The exports correctly include
Showas the client-facing component alongsideSignedInandSignedOut.packages/shared/src/types/protect.ts (2)
72-75: LGTM! Good backward compatibility approach.The deprecated alias
ProtectPropspointing toProtectParamsprovides a smooth migration path for existing consumers.
82-86: LGTM! Flexible condition type.The
ShowWhenConditiontype allows for intuitive usage patterns: simple strings for common cases ('signedIn','signedOut'), objects for authorization params, or callbacks for complex logic.packages/astro/src/react/controlComponents.tsx (2)
1-7: LGTM!The import consolidation is cleaner and the switch to
ProtectParamsfrom shared types aligns with the broader refactoring in this PR.
69-71: LGTM!The
ProtectPropstype correctly referencesProtectParamsfrom shared types, maintaining consistency across the codebase.packages/nextjs/src/app-router/server/controlComponents.tsx (4)
1-1: LGTM! Import statement correctly updated.The import statement appropriately adds
ShowWhenConditionto support the newShowcomponent API.
6-10: LGTM! Type definition is well-structured.The
AppRouterProtectPropstype correctly combinesProtectParams, optional fallback UI, and pending session options using TypeScript intersection types.
12-17: LGTM! Type definition correctly enforces requiredwhenprop.The
AppRouterShowPropstype appropriately makeswhen: ShowWhenConditionrequired (not optional), which ensures consumers must explicitly specify the condition for rendering.
47-47: LGTM! Protect signature correctly updated.The function signature update to use
AppRouterProtectPropsmaintains backward compatibility while aligning with the new type system.packages/react/src/components/controlComponents.tsx (3)
2-2: LGTM! Import statement correctly updated.The import statement appropriately adds
ShowWhenConditionto support the newShowcomponent API.
72-77: LGTM! Type definition is well-structured and consistent.The
ShowPropstype correctly matches the server implementation inpackages/nextjs/src/app-router/server/controlComponents.tsx(lines 12-17), ensuring consistent API surface across client and server contexts.
98-128: LGTM! Logic is correct and consistent with server implementation.The Show component correctly handles all authorization conditions:
'signedOut'shows content when not signed in'signedIn'shows content when signed in- ProtectParams and function conditions are evaluated via the
checkAuthorizationhelper- Early return while loading prevents UI flicker
The implementation is functionally equivalent to the server version in
packages/nextjs/src/app-router/server/controlComponents.tsx(lines 88-112), just with slightly different code organization.Note: Line 107's
resolvedWhen = whenis an unnecessary identity assignment (same pattern as server implementation). Consider the optional refactor suggested for the server file.
| /** | ||
| * Use `<Protect/>` in order to prevent unauthenticated or unauthorized users from accessing the children passed to the component. | ||
| * Use `<Show/>` to conditionally render content based on user authorization or sign-in state. | ||
| * | ||
| * Examples: | ||
| * ``` | ||
| * <Protect permission="a_permission_key" /> | ||
| * <Protect role="a_role_key" /> | ||
| * <Protect condition={(has) => has({permission:"a_permission_key"})} /> | ||
| * <Protect condition={(has) => has({role:"a_role_key"})} /> | ||
| * <Protect fallback={<p>Unauthorized</p>} /> | ||
| * @example | ||
| * ```tsx | ||
| * <Show when={{ permission: "org:billing:manage" }} fallback={<p>Unauthorized</p>}> | ||
| * <BillingSettings /> | ||
| * </Show> | ||
| * | ||
| * <Show when={{ role: "admin" }}> | ||
| * <AdminPanel /> | ||
| * </Show> | ||
| * | ||
| * <Show when={(has) => has({ permission: "org:read" }) && isFeatureEnabled}> | ||
| * <ProtectedFeature /> | ||
| * </Show> | ||
| * ``` | ||
| * | ||
| */ |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Enhance JSDoc with parameter descriptions and additional examples.
While the JSDoc includes good examples for authorization checks, it's missing:
@paramdescriptions for each prop@returnsdescription- Examples of
when="signedIn"andwhen="signedOut"string conditions
As per coding guidelines, all public APIs must have comprehensive JSDoc including @param, @returns, and @example tags.
Apply this diff to complete the documentation:
/**
* Use `<Show/>` to conditionally render content based on user authorization or sign-in state.
*
+ * @param props - Component props
+ * @param props.when - Condition that determines when to show the children. Can be:
+ * - `'signedIn'`: Show content when user is signed in
+ * - `'signedOut'`: Show content when user is signed out
+ * - ProtectParams object: Show content if user has the specified role/permission/feature/plan
+ * - Function: Custom condition that receives `has` for complex authorization logic
+ * @param props.fallback - Optional content to render when the condition is not met
+ * @param props.treatPendingAsSignedOut - Whether to treat pending sessions as signed out (default: true)
+ * @param props.children - Content to render when the condition is met
+ *
+ * @returns The authorized content, fallback content, or null based on the condition and auth state
+ *
* @example
* ```tsx
+ * // Show content only when user is signed in
+ * <Show when="signedIn">
+ * <UserDashboard />
+ * </Show>
+ *
+ * // Show content only when user is signed out
+ * <Show when="signedOut" fallback={<Redirect to="/dashboard" />}>
+ * <LandingPage />
+ * </Show>
+ *
+ * // Authorization-based rendering
* <Show when={{ permission: "org:billing:manage" }} fallback={<p>Unauthorized</p>}>
* <BillingSettings />
* </Show>
*
* <Show when={{ role: "admin" }}>
* <AdminPanel />
* </Show>
*
* <Show when={(has) => has({ permission: "org:read" }) && isFeatureEnabled}>
* <ProtectedFeature />
* </Show>
* ```
- *
*/
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/nextjs/src/app-router/server/controlComponents.tsx (3)
6-17: Consider adding JSDoc comments for exported types.These types are part of the public API surface and could benefit from brief documentation explaining their purpose and usage context. This aligns with the comprehensive JSDoc added for the
Showcomponent.+/** + * Props for the server-side `<Protect/>` component in App Router. + */ export type AppRouterProtectProps = React.PropsWithChildren< ProtectParams & { fallback?: React.ReactNode; } & PendingSessionOptions >; +/** + * Props for the server-side `<Show/>` component in App Router. + */ export type AppRouterShowProps = React.PropsWithChildren< PendingSessionOptions & { fallback?: React.ReactNode; when: ShowWhenCondition; } >;
85-114: Documentation is comprehensive - consider addingtreatPendingAsSignedOutparam.The JSDoc now covers the main usage patterns well. Consider also documenting the inherited
treatPendingAsSignedOutprop for completeness:* @param props.fallback Optional content rendered when the condition fails. * @param props.children Content rendered when the condition passes. +* @param props.treatPendingAsSignedOut Whether pending sessions should be treated as signed out (default: true).
119-119: Remove redundant variable assignment.The
resolvedWhenvariable is assigned directly fromwhenwithout any transformation. You can usewhendirectly throughout the function.- const resolvedWhen = when; const authorized = <>{children}</>; const unauthorized = fallback ? <>{fallback}</> : null; - if (typeof resolvedWhen === 'string') { - if (resolvedWhen === 'signedOut') { + if (typeof when === 'string') { + if (when === 'signedOut') { return userId ? unauthorized : authorized; } return userId ? authorized : unauthorized; } if (!userId) { return unauthorized; } - if (typeof resolvedWhen === 'function') { - return resolvedWhen(has) ? authorized : unauthorized; + if (typeof when === 'function') { + return when(has) ? authorized : unauthorized; } - return has(resolvedWhen) ? authorized : unauthorized; + return has(when) ? authorized : unauthorized;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/astro/src/react/controlComponents.tsx(3 hunks)packages/nextjs/src/app-router/server/controlComponents.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/astro/src/react/controlComponents.tsx
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
packages/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
**/*.ts?(x)
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.tsx: Use error boundaries in React components
Minimize re-renders in React components
**/*.tsx: Use proper type definitions for props and state in React components
Leverage TypeScript's type inference where possible in React components
Use proper event types for handlers in React components
Implement proper generic types for reusable React components
Use proper type guards for conditional rendering in React components
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
**/*.{md,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Update documentation for API changes
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
**/*.{jsx,tsx}: Always use functional components with hooks instead of class components
Follow PascalCase naming for components (e.g.,UserProfile,NavigationMenu)
Keep components focused on a single responsibility - split large components
Limit component size to 150-200 lines; extract logic into custom hooks
Use composition over inheritance - prefer smaller, composable components
Export components as named exports for better tree-shaking
One component per file with matching filename and component name
Separate UI components from business logic components
Use useState for simple state management in React components
Use useReducer for complex state logic in React components
Implement proper state initialization in React components
Use proper state updates with callbacks in React components
Implement proper state cleanup in React components
Use Context API for theme/authentication state management
Implement proper state persistence in React applications
Use React.memo for expensive components
Implement proper useCallback for handlers in React components
Use proper useMemo for expensive computations in React components
Implement proper virtualization for lists in React components
Use proper code splitting with React.lazy in React applications
Implement proper cleanup in useEffect hooks
Use proper refs for DOM access in React components
Implement proper event listener cleanup in React components
Use proper abort controllers for fetch in React components
Implement proper subscription cleanup in React components
Use proper HTML elements for semantic HTML in React components
Implement proper ARIA attributes for accessibility in React components
Use proper heading hierarchy in React components
Implement proper form labels in React components
Use proper button types in React components
Implement proper focus management for keyboard navigation in React components
Use proper keyboard shortcuts in React components
Implement proper tab order in React components
Use proper ...
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Implement type guards forunknowntypes using the patternfunction isType(value: unknown): value is Type
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details in classes
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Use mixins for shared behavior across unrelated classes in TypeScript
Use generic constraints with bounded type parameters like<T extends { id: string }>
Use utility types likeOmit,Partial, andPickfor data transformation instead of manual type construction
Use discriminated unions instead of boolean flags for state management and API responses
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation at the type level
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Document functions with JSDoc comments including @param, @returns, @throws, and @example tags
Create custom error classes that extend Error for specific error types
Use the Result pattern for error handling instead of throwing exceptions
Use optional chaining (?.) and nullish coalescing (??) operators for safe property access
Let TypeScript infer obvious types to reduce verbosity
Useconst assertionswithas constfor literal types
Usesatisfiesoperator for type checking without widening types
Declare readonly arrays and objects for immutable data structures
Use spread operator and array spread for immutable updates instead of mutations
Use lazy loading for large types...
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/nextjs/src/app-router/server/controlComponents.tsx
🧬 Code graph analysis (1)
packages/nextjs/src/app-router/server/controlComponents.tsx (3)
packages/shared/src/types/protect.ts (2)
ProtectParams(28-70)ShowWhenCondition(82-86)packages/shared/src/types/session.ts (1)
PendingSessionOptions(35-42)packages/react/src/components/controlComponents.tsx (1)
Show(98-128)
⏰ 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). (29)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: triage
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/nextjs/src/app-router/server/controlComponents.tsx (3)
47-47: LGTM!The updated signature properly types the
Protectcomponent using the new consolidatedAppRouterProtectPropstype while maintaining backward compatibility with existing usage.
123-128: LGTM!The string condition handling correctly implements the
signedIn/signedOutlogic. TypeScript'sShowWhenConditiontype ensures only valid string literals are passed, so the else branch safely handles'signedIn'.
115-139: Well-implemented server-side Show component.The implementation correctly handles all
ShowWhenConditionvariants and follows the same authorization logic as the client-side counterpart. The async pattern is appropriate for server components in the App Router.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js (1)
1-338: Good test coverage, but consider adding a few edge cases.The fixtures cover the main transformation scenarios well. However, consider adding tests for:
- Multiple auth attributes combined - e.g.,
<Protect role="admin" permission="org:read">to verify they're merged into a singlewhenobject.- Aliased imports - e.g.,
import { Protect as P } from "@clerk/react"to verify the local binding is handled correctly.@clerk/expopackage - Listed inCLIENT_ONLY_PACKAGESbut not explicitly tested.Example fixture for multiple auth attributes:
{ name: 'Multiple auth attributes combined', source: ` import { Protect } from "@clerk/react" function App() { return ( <Protect role="admin" permission="org:read"> <Content /> </Protect> ) } `, output: ` import { Show } from "@clerk/react" function App() { return ( <Show when={{ role: "admin", permission: "org:read" }}> <Content /> </Show> ); } `, }packages/upgrade/src/components/SDKWorkflow.js (1)
231-252: Consider extracting the repeated codemod chain into a helper.The three-step sequence (CLERK_REACT_V6 → REMOVE_DEPRECATED_PROPS → PROTECT_TO_SHOW) is repeated six times across the workflows. A helper component or render function could reduce duplication and make future codemod additions easier.
Example abstraction:
function CodemodChain({ codemods, sdk, glob, onGlobResolved, onComplete }) { const [completedIndex, setCompletedIndex] = useState(-1); return codemods.map((transform, i) => ( completedIndex >= i - 1 ? ( <Codemod key={transform} callback={() => i === codemods.length - 1 ? onComplete() : setCompletedIndex(i)} sdk={sdk} transform={transform} glob={i > 0 ? glob : undefined} onGlobResolved={i === 0 ? onGlobResolved : undefined} /> ) : null )); }packages/upgrade/src/codemods/transform-protect-to-show.cjs (1)
108-116: Namespaced JSX like<Clerk.Protect>is silently skipped.The check
j.JSXIdentifier.check(openingElement.name)excludesJSXMemberExpression(e.g.,<Clerk.Protect>). This is safe but users with namespaced imports won't get the transform. Consider adding a comment or a warning in the codemod output for such cases.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
packages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js(1 hunks)packages/upgrade/src/codemods/transform-protect-to-show.cjs(1 hunks)packages/upgrade/src/components/SDKWorkflow.js(8 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
All code must pass ESLint checks with the project's configuration
Files:
packages/upgrade/src/components/SDKWorkflow.jspackages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/upgrade/src/components/SDKWorkflow.jspackages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Follow established naming conventions (PascalCase for components, camelCase for variables)
Files:
packages/upgrade/src/components/SDKWorkflow.jspackages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js
packages/**/src/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
packages/**/src/**/*.{ts,tsx,js,jsx}: Maintain comprehensive JSDoc comments for public APIs
Use tree-shaking friendly exports
Validate all inputs and sanitize outputs
All public APIs must be documented with JSDoc
Use dynamic imports for optional features
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Implement proper logging with different levels
Files:
packages/upgrade/src/components/SDKWorkflow.jspackages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use ESLint with custom configurations tailored for different package types
Files:
packages/upgrade/src/components/SDKWorkflow.jspackages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js
**/*.{js,ts,jsx,tsx,json,md,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Use Prettier for code formatting across all packages
Files:
packages/upgrade/src/components/SDKWorkflow.jspackages/upgrade/src/codemods/__tests__/__fixtures__/transform-protect-to-show.fixtures.js
⏰ 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). (28)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (4)
packages/upgrade/src/components/SDKWorkflow.js (1)
185-192: LGTM!The PROTECT_TO_SHOW codemod is correctly chained after REMOVE_DEPRECATED_PROPS, using the same
globpattern and properly settingdonevia the callback. The pattern is consistent across all workflow branches.packages/upgrade/src/codemods/transform-protect-to-show.cjs (3)
130-144: Spread attributes are correctly preserved.The code checks
j.JSXAttribute.check(attr)and pushes non-JSXAttribute nodes (like JSXSpreadAttribute) tootherAttributes, which are preserved. Good handling of spread props.
9-44: Robust directive detection covering multiple parser variations.The
hasUseClientDirectivefunction handles various AST representations: ExpressionStatement with Literal/StringLiteral, DirectiveLiteral, directive field, and babel's directives array. This ensures compatibility across different parsers.
191-194: LGTM!The double-semicolon fix handles a known recast quirk with directive prologues. The regex correctly targets directive strings at the start of lines.
| specifiers.forEach(spec => { | ||
| if (j.ImportSpecifier.check(spec) && spec.imported.name === 'Protect') { | ||
| spec.imported.name = 'Show'; | ||
| if (spec.local && spec.local.name === 'Protect') { | ||
| spec.local.name = 'Show'; | ||
| } | ||
| dirtyFlag = true; | ||
| } | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Aliased import handling may not work as expected.
When the import uses an alias like import { Protect as MyProtect }, the code updates both spec.imported.name and spec.local.name to 'Show'. However, this loses the user's intended alias. The JSX transformation below looks for Protect elements, which won't match MyProtect.
If aliased imports should be supported, the JSX transformation needs to track the local name:
+ const protectLocalNames = new Set();
+
allPackages.forEach(packageName => {
root.find(j.ImportDeclaration, { source: { value: packageName } }).forEach(path => {
const node = path.node;
const specifiers = node.specifiers || [];
specifiers.forEach(spec => {
if (j.ImportSpecifier.check(spec) && spec.imported.name === 'Protect') {
+ const localName = spec.local?.name || 'Protect';
+ protectLocalNames.add(localName);
spec.imported.name = 'Show';
- if (spec.local && spec.local.name === 'Protect') {
- spec.local.name = 'Show';
- }
+ spec.local.name = 'Show';
dirtyFlag = true;
}
});
});
});Then use protectLocalNames in the JSX check at line 114.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/upgrade/src/codemods/transform-protect-to-show.cjs around lines 96
to 106, the current logic replaces both spec.imported.name and spec.local.name
when encountering import { Protect } which destroys user aliases like import {
Protect as MyProtect }; instead, leave spec.local.name alone if an alias exists
(i.e., only change spec.imported.name to 'Show'), record the effective local
identifier (spec.local.name if present, otherwise the original imported name)
into a protectLocalNames array, and keep dirtyFlag=true; then update the JSX
transformation at ~line 114 to check for Protect elements using
protectLocalNames (i.e., match any of the recorded local names) so aliased
imports are handled correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/upgrade/src/codemods/transform-protect-to-show.cjs (1)
58-60: Update docstring to reflect behavior for all hybrid packagesThe comment says:
NOTE: For @clerk/nextjs, this only transforms files with 'use client' directive.
but the logic gates on any import from
HYBRID_PACKAGES(@clerk/astro,@clerk/nextjs) plus thehasUseClientDirectivecheck. That means non‑client files importingProtectfrom either hybrid package are skipped.Consider updating the note to mention all entries in
HYBRID_PACKAGES(or rephrasing in terms of “hybrid packages”) so the behavior matches the documentation.Also applies to: 73-88
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/astro/src/react/controlComponents.tsx(3 hunks)packages/upgrade/src/codemods/transform-protect-to-show.cjs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/astro/src/react/controlComponents.tsx
⏰ 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). (28)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (1)
packages/upgrade/src/codemods/transform-protect-to-show.cjs (1)
9-44:hasUseClientDirectiveimplementation looks solidThe helper covers the common AST variants for
'use client'(expression statements,directivefield, andprogram.directives) and safely bails on empty programs. No changes needed here.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/upgrade/src/codemods/transform-protect-to-show.cjs (1)
197-205: Return logic and semicolon cleanup are correct.The return logic correctly guards against no-op transformations, and the regex at line 203 adequately handles the double semicolon artifact that can occur when jscodeshift reprints directive prologues.
The regex pattern could be made more robust to handle quoted strings with different quote types, but given that directive prologues are typically simple strings like
"use client"or"use strict", the current implementation is sufficient.If you want a more precise pattern:
- result = result.replace(/^(['"`][^'"`]+['"`]);;/gm, '$1;'); + result = result.replace(/^(["']use (client|server|strict)["']);;/gm, '$1;');This explicitly matches known directives and is more restrictive, reducing the chance of false positives.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
packages/upgrade/src/codemods/transform-protect-to-show.cjs(1 hunks)
⏰ 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). (28)
- GitHub Check: Integration Tests (quickstart, chrome, 16)
- GitHub Check: Integration Tests (machine, chrome)
- GitHub Check: Integration Tests (quickstart, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 15)
- GitHub Check: Integration Tests (nextjs, chrome, 16)
- GitHub Check: Integration Tests (tanstack-react-start, chrome)
- GitHub Check: Integration Tests (machine, chrome, RQ)
- GitHub Check: Integration Tests (nextjs, chrome, 16, RQ)
- GitHub Check: Integration Tests (billing, chrome, RQ)
- GitHub Check: Integration Tests (vue, chrome)
- GitHub Check: Integration Tests (nuxt, chrome)
- GitHub Check: Integration Tests (billing, chrome)
- GitHub Check: Integration Tests (astro, chrome)
- GitHub Check: Integration Tests (handshake, chrome)
- GitHub Check: Integration Tests (react-router, chrome)
- GitHub Check: Integration Tests (custom, chrome)
- GitHub Check: Integration Tests (sessions:staging, chrome)
- GitHub Check: Integration Tests (ap-flows, chrome)
- GitHub Check: Integration Tests (handshake:staging, chrome)
- GitHub Check: Integration Tests (sessions, chrome)
- GitHub Check: Integration Tests (generic, chrome)
- GitHub Check: Integration Tests (localhost, chrome)
- GitHub Check: Integration Tests (express, chrome)
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: Build Packages
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
🔇 Additional comments (3)
packages/upgrade/src/codemods/transform-protect-to-show.cjs (3)
1-44: LGTM! Comprehensive directive detection.The package constants and
hasUseClientDirectivefunction are well-implemented. The function correctly handles multiple parser representations of the 'use client' directive (expression statements, directive fields, and program-level directives), ensuring robust detection across different AST formats.
66-88: LGTM! RSC guard logic is correct.The setup and RSC detection logic properly identifies hybrid package imports and skips transformation for server components. This ensures that server-side
Protectusage in frameworks like Next.js remains unchanged, which aligns with the PR's goal to only transform client-side components.
90-111: LGTM! Import transformation correctly handles aliases.The import transformation logic properly handles both standard and aliased imports:
- Standard:
import { Protect }→import { Show }- Aliased:
import { Protect as MyProtect }→import { Show as MyProtect }The tracking of
effectiveLocalNameensures JSX transformation can locate usage of both standard and aliased components.
| const properties = authAttributes.map(attr => { | ||
| const key = j.identifier(attr.name.name); | ||
| let value; | ||
|
|
||
| if (j.JSXExpressionContainer.check(attr.value)) { | ||
| value = attr.value.expression; | ||
| } else if (j.StringLiteral.check(attr.value) || j.Literal.check(attr.value)) { | ||
| value = attr.value; | ||
| } else { | ||
| // Default string value | ||
| value = j.stringLiteral(attr.value?.value || ''); | ||
| } |
There was a problem hiding this comment.
Boolean shorthand attributes may not transform correctly.
The fallback at line 174 creates an empty string literal for attr.value when it's not a JSXExpressionContainer or StringLiteral. This would affect boolean shorthand syntax like <Protect role>, which would transform to when={{ role: '' }} instead of when={{ role: true }}.
However, this is likely a rare edge case since Clerk's Protect component typically expects string or array values for role, permission, etc., making boolean shorthand semantically invalid. If this syntax is genuinely unsupported, the current behavior is acceptable.
If boolean shorthand should be supported, apply this diff:
if (j.JSXExpressionContainer.check(attr.value)) {
value = attr.value.expression;
} else if (j.StringLiteral.check(attr.value) || j.Literal.check(attr.value)) {
value = attr.value;
+ } else if (attr.value === null) {
+ // Boolean shorthand: <Protect role> means role={true}
+ value = j.booleanLiteral(true);
} else {
// Default string value
value = j.stringLiteral(attr.value?.value || '');
}📝 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 properties = authAttributes.map(attr => { | |
| const key = j.identifier(attr.name.name); | |
| let value; | |
| if (j.JSXExpressionContainer.check(attr.value)) { | |
| value = attr.value.expression; | |
| } else if (j.StringLiteral.check(attr.value) || j.Literal.check(attr.value)) { | |
| value = attr.value; | |
| } else { | |
| // Default string value | |
| value = j.stringLiteral(attr.value?.value || ''); | |
| } | |
| const properties = authAttributes.map(attr => { | |
| const key = j.identifier(attr.name.name); | |
| let value; | |
| if (j.JSXExpressionContainer.check(attr.value)) { | |
| value = attr.value.expression; | |
| } else if (j.StringLiteral.check(attr.value) || j.Literal.check(attr.value)) { | |
| value = attr.value; | |
| } else if (attr.value === null) { | |
| // Boolean shorthand: <Protect role> means role={true} | |
| value = j.booleanLiteral(true); | |
| } else { | |
| // Default string value | |
| value = j.stringLiteral(attr.value?.value || ''); | |
| } |
🤖 Prompt for AI Agents
In packages/upgrade/src/codemods/transform-protect-to-show.cjs around lines
164–175, the fallback creates an empty string literal for attributes that aren't
JSXExpressionContainer or StringLiteral, which turns boolean shorthand JSX
attributes (e.g. <Protect role /> where attr.value is null) into empty strings;
change the fallback to detect boolean shorthand (attr.value === null or
attr.value == null) and set value to a boolean literal true in that case,
otherwise keep the existing stringLiteral fallback for other unexpected shapes;
update any related tests to cover the boolean-shorthand case.
Description
Fixes: USER-3389
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Refactor
Types
Tests / Tools
✏️ Tip: You can customize this high-level summary in your review settings.