-
-
Notifications
You must be signed in to change notification settings - Fork 355
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
⚡️ Prefetch queries with trpc #1454
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThe changes in this pull request enhance the server-side rendering capabilities of various components in a Next.js application by integrating React Query and tRPC for improved data fetching and state management. Key modifications include the addition of Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (8)
apps/web/src/app/[locale]/poll/[urlId]/layout.tsx (1)
15-20
: Add error handling for prefetching queriesUsing
Promise.all
without error handling means that if any of the prefetch queries fail, the entire operation will fail. Consider adding error handling to each prefetch to ensure robustness.Proposed change:
- await Promise.all([ - trpc.polls.get.prefetch({ urlId: params.urlId }), - trpc.polls.participants.list.prefetch({ pollId: params.urlId }), - trpc.polls.getWatchers.prefetch({ pollId: params.urlId }), - trpc.polls.comments.list.prefetch({ pollId: params.urlId }), - ]); + await Promise.allSettled([ + trpc.polls.get.prefetch({ urlId: params.urlId }), + trpc.polls.participants.list.prefetch({ pollId: params.urlId }), + trpc.polls.getWatchers.prefetch({ pollId: params.urlId }), + trpc.polls.comments.list.prefetch({ pollId: params.urlId }), + ]);This way, all prefetches attempt to complete, and you can handle any that fail individually if needed.
apps/web/src/trpc/context.ts (1)
11-11
: Consider makingip
a required property if always presentIf
ip
is consistently available and required by tRPC procedures, consider making it a non-optional property inTRPCContext
for type safety and clarity.apps/web/src/trpc/server/create-ssr-helper.ts (1)
14-19
: Handle unauthorized access appropriately in SSR contextThrowing a
TRPCError
with codeUNAUTHORIZED
may not be the best approach in server-side rendering. Consider redirecting to a login page or rendering an appropriate error message instead.Proposed change:
if (!session) { - throw new TRPCError({ - code: "UNAUTHORIZED", - message: "Unauthorized", - }); + // Handle unauthorized access in a way suitable for SSR + redirect('/login'); }Ensure that the method used aligns with your application's routing and error handling practices.
apps/web/src/app/[locale]/(admin)/page.tsx (1)
19-20
: Add error handling for SSR helper creation and prefetching
createSSRHelper()
andhelpers.dashboard.info.prefetch()
may throw errors if there are issues like network failures or unauthorized access. Consider adding try-catch blocks to handle these potential errors gracefully.Proposed change:
- const helpers = await createSSRHelper(); - await helpers.dashboard.info.prefetch(); + let helpers; + try { + helpers = await createSSRHelper(); + await helpers.dashboard.info.prefetch(); + } catch (error) { + // Handle errors appropriately, e.g., log the error, redirect, or render an error message + console.error("Error during SSR helper creation or prefetching:", error); + // Return an appropriate response or fallback UI + return notFound(); + }This addition will enhance the robustness of your server-side rendering logic.
apps/web/src/trpc/trpc.ts (2)
87-91
: Consider improving error handling for missing IPThe error message "Failed to get client IP" might not be helpful for debugging. Consider including more context about why the IP might be missing and potential remediation steps.
if (!ctx.ip) { throw new TRPCError({ code: "INTERNAL_SERVER_ERROR", - message: "Failed to get client IP", + message: "Failed to get client IP. This might happen if the request is not properly forwarded through the proxy server.", }); }
Line range hint
94-102
: Review rate limiting configurationThe current configuration of 5 requests per minute might be too restrictive for legitimate users, especially considering server-side rendering and prefetching scenarios.
Consider:
- Increasing the limit or using a sliding window of 30 requests per minute
- Adding different limits for authenticated vs unauthenticated users
- Implementing graceful degradation if the KV store is unavailable
apps/web/src/app/[locale]/(admin)/layout.tsx (1)
21-43
: Consider performance optimization for hydration stateThe current implementation wraps the entire layout with Hydrate. Consider moving the hydration boundary closer to where the data is actually used to optimize the initial render.
Consider splitting the layout into smaller components and only wrapping the components that actually use the subscription data with Hydrate.
apps/web/src/app/[locale]/invite/[urlId]/page.tsx (1)
9-17
: Consider adding type safety for prefetch operationsThe prefetch operations could benefit from stronger type safety to prevent potential runtime errors.
Consider creating a type-safe helper function for managing related prefetch operations:
type PollPrefetchParams = { pollId: string; urlId: string; }; async function prefetchPollData({ pollId, urlId }: PollPrefetchParams) { return Promise.allSettled([ trpc.polls.get.prefetch({ urlId }), trpc.polls.participants.list.prefetch({ pollId }), trpc.polls.comments.list.prefetch({ pollId }), ]); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
apps/web/src/app/[locale]/(admin)/layout.tsx
(1 hunks)apps/web/src/app/[locale]/(admin)/page.tsx
(2 hunks)apps/web/src/app/[locale]/invite/[urlId]/page.tsx
(1 hunks)apps/web/src/app/[locale]/poll/[urlId]/layout.tsx
(1 hunks)apps/web/src/pages/api/trpc/[trpc].ts
(2 hunks)apps/web/src/trpc/context.ts
(1 hunks)apps/web/src/trpc/server/create-ssr-helper.ts
(1 hunks)apps/web/src/trpc/trpc.ts
(1 hunks)
🔇 Additional comments (3)
apps/web/src/app/[locale]/poll/[urlId]/layout.tsx (1)
28-31
: Verify that dehydrate
includes all necessary data
Ensure that dehydrate(trpc.queryClient)
captures all the prefetched data needed by the client. Missing data might lead to additional client-side fetches or errors.
Run the following script to check the contents of the query client:
✅ Verification successful
Let me gather more information about the data prefetching in this component.
Let me try a different approach to find the relevant files and prefetching logic.
Based on the search results, I can now provide a final response as we have sufficient information about the data prefetching in the layout component.
All necessary data is being properly prefetched
The layout component correctly prefetches all the required data for the poll page:
polls.get
: Fetches the poll detailspolls.participants.list
: Fetches the list of participantspolls.getWatchers
: Fetches the poll watcherspolls.comments.list
: Fetches the poll comments
These prefetched queries match exactly with the data requirements shown in the components:
PollLayout
component uses all these queries- The data is properly dehydrated using
dehydrate(trpc.queryClient)
- Components like Discussion, ParticipantsProvider, and NotificationsToggle consume this prefetched data
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all expected queries are present in the query client state.
# Expected query keys
EXPECTED_KEYS=(
'polls.get'
'polls.participants.list'
'polls.getWatchers'
'polls.comments.list'
)
# Iterate over expected keys and check if they exist in the query client
for key in "${EXPECTED_KEYS[@]}"; do
# This is a placeholder command; actual implementation requires runtime access.
echo "Check if '$key' exists in trpc.queryClient."
done
Length of output: 753
Script:
#!/bin/bash
# Search for prefetch calls in the layout file and its parent components
rg -A 5 "prefetch|prefetchQuery" "apps/web/src/app/\[locale\]/poll/\[urlId\]/"
# Look for any trpc query definitions related to polls
ast-grep --pattern 'router.query($$$)'
Length of output: 205
Script:
#!/bin/bash
# Find all files in the poll directory
fd . "apps/web/src/app" --type f
# Search for trpc prefetch calls in the entire codebase
rg -A 5 "prefetch|prefetchQuery" "apps/web/src"
# Look for trpc router definitions for polls
rg -A 5 "polls\." "apps/web/src"
Length of output: 27341
apps/web/src/trpc/context.ts (1)
11-11
: Verify removal of req
and res
from TRPCContext
Removing req
and res
from TRPCContext
may impact procedures that rely on these properties. Ensure that no existing tRPC procedures depend on ctx.req
or ctx.res
.
Run the following script to search for any usage of ctx.req
or ctx.res
:
✅ Verification successful
Let me verify if these usages are in tRPC procedures or not.
Let me try a different pattern to search for tRPC procedures.
Let me check the actual tRPC procedure implementations.
Let me try one final search pattern to find tRPC procedure implementations.
Based on the search results, I can now provide a final response:
Removal of req
and res
from TRPCContext
is safe
The only usages of ctx.req
and ctx.res
are in the Next.js page component disable-notifications.tsx
, not in any tRPC procedures. The tRPC procedures only use ctx.user
for authentication checks, as seen in the middleware implementations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for usages of `ctx.req` or `ctx.res` in the codebase.
rg 'ctx\.req|ctx\.res'
Length of output: 276
Script:
#!/bin/bash
# Search for tRPC procedure definitions in the codebase
ast-grep --pattern 'router.$_($$$, async ({ ctx }) => { $$$ })'
Length of output: 65
Script:
#!/bin/bash
# Search for files that might contain tRPC router definitions
fd "router.ts"
# Then let's examine the content of these files
rg "procedure|router\." --type ts
Length of output: 1917
Script:
#!/bin/bash
# Find all router implementation files
fd "router\." --type ts --exec cat {}
Length of output: 211
Script:
#!/bin/bash
# Search for files that might contain tRPC routers and their content
fd -e ts -e tsx | xargs rg "createTRPCRouter|procedure\." -A 5
Length of output: 1222
apps/web/src/pages/api/trpc/[trpc].ts (1)
40-41
: Verify correct retrieval of client IP address
Using requestIp.getClientIp(opts.req)
may return unexpected results depending on the deployment environment (e.g., behind proxies or load balancers). Ensure that the IP address is accurately retrieved, considering the potential need to handle X-Forwarded-For
headers.
Run the following script to check for proper IP retrieval:
|
||
export default async function Layout({ | ||
children, | ||
params, | ||
}: React.PropsWithChildren<{ params: { urlId: string } }>) { | ||
const trpc = await createSSRHelper(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Handle potential errors from createSSRHelper()
createSSRHelper()
may throw an error if the user session is not available or if there's an issue creating the context. Consider adding error handling to manage this scenario and improve user experience.
Proposed change:
- const trpc = await createSSRHelper();
+ let trpc;
+ try {
+ trpc = await createSSRHelper();
+ } catch (error) {
+ // Handle the error appropriately, e.g., redirect to login or show an error page
+ notFound();
+ }
Committable suggestion skipped: line range outside the PR's diff.
async function createContext(): Promise<TRPCContext> { | ||
const session = await getServerSession(); | ||
|
||
if (!session) { | ||
throw new TRPCError({ | ||
code: "UNAUTHORIZED", | ||
message: "Unauthorized", | ||
}); | ||
} | ||
|
||
const res = { | ||
user: { | ||
id: session.user.id, | ||
isGuest: session.user.email === null, | ||
locale: session.user.locale ?? undefined, | ||
image: session.user.image ?? undefined, | ||
getEmailClient: () => getEmailClient(session.user.locale ?? undefined), | ||
}, | ||
}; | ||
|
||
return res; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Include ip
in the context for consistency
The TRPCContext
includes an ip
property, but createContext
here does not set it. For consistency and to prevent potential issues in procedures that expect ip
, consider including the ip
in the context.
Proposed change:
const res = {
user: {
id: session.user.id,
isGuest: session.user.email === null,
locale: session.user.locale ?? undefined,
image: session.user.image ?? undefined,
getEmailClient: () =>
getEmailClient(session.user.locale ?? undefined),
},
+ ip: undefined, // Set the IP address here if available in this context
};
If obtaining the IP address is feasible in this context, it would maintain consistency across your server-side context creation.
Committable suggestion skipped: line range outside the PR's diff.
const helpers = await createSSRHelper(); | ||
await helpers.user.subscription.prefetch(); | ||
const dehydratedState = dehydrate(helpers.queryClient); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding error handling for prefetch operations
The prefetch operation could fail silently. Consider adding error handling to gracefully handle failed prefetches.
const helpers = await createSSRHelper();
- await helpers.user.subscription.prefetch();
+ try {
+ await helpers.user.subscription.prefetch();
+ } catch (error) {
+ console.error('Failed to prefetch subscription data:', error);
+ // Consider implementing fallback behavior
+ }
const dehydratedState = dehydrate(helpers.queryClient);
📝 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 helpers = await createSSRHelper(); | |
await helpers.user.subscription.prefetch(); | |
const dehydratedState = dehydrate(helpers.queryClient); | |
const helpers = await createSSRHelper(); | |
try { | |
await helpers.user.subscription.prefetch(); | |
} catch (error) { | |
console.error('Failed to prefetch subscription data:', error); | |
// Consider implementing fallback behavior | |
} | |
const dehydratedState = dehydrate(helpers.queryClient); |
await Promise.all([ | ||
trpc.polls.get.prefetch({ urlId: params.urlId }), | ||
trpc.polls.participants.list.prefetch({ pollId: params.urlId }), | ||
trpc.polls.comments.list.prefetch({ pollId: params.urlId }), | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance error handling for parallel prefetches
While using Promise.all for concurrent fetching is efficient, a single failed prefetch will cause all prefetches to fail. Consider using Promise.allSettled for more resilient prefetching.
- await Promise.all([
+ const results = await Promise.allSettled([
trpc.polls.get.prefetch({ urlId: params.urlId }),
trpc.polls.participants.list.prefetch({ pollId: params.urlId }),
trpc.polls.comments.list.prefetch({ pollId: params.urlId }),
]);
+
+ // Log any failed prefetches
+ results.forEach((result, index) => {
+ if (result.status === 'rejected') {
+ console.error(`Failed to prefetch query ${index}:`, result.reason);
+ }
+ });
📝 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.
await Promise.all([ | |
trpc.polls.get.prefetch({ urlId: params.urlId }), | |
trpc.polls.participants.list.prefetch({ pollId: params.urlId }), | |
trpc.polls.comments.list.prefetch({ pollId: params.urlId }), | |
]); | |
const results = await Promise.allSettled([ | |
trpc.polls.get.prefetch({ urlId: params.urlId }), | |
trpc.polls.participants.list.prefetch({ pollId: params.urlId }), | |
trpc.polls.comments.list.prefetch({ pollId: params.urlId }), | |
]); | |
// Log any failed prefetches | |
results.forEach((result, index) => { | |
if (result.status === 'rejected') { | |
console.error(`Failed to prefetch query ${index}:`, result.reason); | |
} | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
apps/web/src/app/[locale]/invite/[urlId]/layout.tsx (1)
11-12
: Validateparams.urlId
before using itEnsure that
params.urlId
is validated to prevent potential issues with malformed or unexpected input. This helps enhance security and robustness by preventing invalid data from causing errors downstream.You might add validation logic to confirm that
urlId
meets expected patterns or criteria.apps/web/src/components/participants-provider.tsx (1)
24-26
: Re-evaluate caching strategy after removingstaleTime
andcacheTime
By removing the
staleTime
andcacheTime
options from theuseQuery
, the default caching behavior is now in effect. This may lead to more frequent data refetching, which could impact performance. Consider verifying that the default caching strategy aligns with the desired application behavior.If the intention is to always fetch fresh data, ensuring users see the most up-to-date participants list, then this change is appropriate. Otherwise, you may want to reintroduce caching options:
const { data: participants } = trpc.polls.participants.list.useQuery({ pollId, +}, { + staleTime: [desired_stale_time], + cacheTime: [desired_cache_time], });Replace
[desired_stale_time]
and[desired_cache_time]
with appropriate values based on your application's needs.apps/web/src/contexts/permissions.tsx (2)
9-14
: Consider adding type documentationThe permissions context would benefit from JSDoc documentation explaining its purpose and usage.
+/** + * Context for managing user permissions. + * Provides the current user ID for permission checks. + */ const PermissionsContext = React.createContext<{ userId: string | null; }>({ userId: null, });
15-24
: Optimize context value with useMemoThe context value should be memoized to prevent unnecessary re-renders of child components.
export const PermissionProvider = ({ children, userId, }: React.PropsWithChildren<{ userId: string | null }>) => { + const value = React.useMemo(() => ({ userId }), [userId]); return ( - <PermissionsContext.Provider value={{ userId }}> + <PermissionsContext.Provider value={value}> {children} </PermissionsContext.Provider> ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
apps/web/src/app/[locale]/invite/[urlId]/layout.tsx
(1 hunks)apps/web/src/app/[locale]/invite/[urlId]/page.tsx
(1 hunks)apps/web/src/app/[locale]/invite/[urlId]/providers.tsx
(1 hunks)apps/web/src/components/participants-provider.tsx
(1 hunks)apps/web/src/contexts/permissions.tsx
(1 hunks)apps/web/src/contexts/poll.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/app/[locale]/invite/[urlId]/providers.tsx
🔇 Additional comments (2)
apps/web/src/contexts/poll.tsx (1)
Line range hint 9-11
: Enhance error message for missing prefetch
The error message indicates that prefetching is expected, but there's no clear documentation on where/how prefetching should occur.
Let's verify the prefetch implementation:
✅ Verification successful
Error message is appropriate, prefetching is well-implemented
The error message is actually appropriate as the poll data is consistently prefetched in both layout components (invite/[urlId]/layout.tsx
and poll/[urlId]/layout.tsx
) using trpc.polls.get.prefetch()
. The error serves as a runtime safeguard in case the prefetching mechanism fails, which is a good practice.
- Poll data is prefetched in layouts:
/invite/[urlId]/layout.tsx
/poll/[urlId]/layout.tsx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for prefetch calls to polls.get
rg "polls\.get\.prefetch" -A 3
Length of output: 888
Script:
#!/bin/bash
# Let's check where the poll context is being used to understand the data flow
rg -l "usePoll"
# Also check for any existing documentation about prefetching
fd "README|docs" --extension md --exec cat {}
Length of output: 9191
Script:
#!/bin/bash
# Let's check the layout files where prefetching occurs to understand the context
cat apps/web/src/app/[locale]/invite/[urlId]/layout.tsx apps/web/src/app/[locale]/poll/[urlId]/layout.tsx
# Also check the poll context implementation
cat apps/web/src/contexts/poll.tsx
Length of output: 2459
apps/web/src/app/[locale]/invite/[urlId]/page.tsx (1)
14-19
: Previous comment about Promise.all still applies
The previous suggestion to use Promise.allSettled for more resilient prefetching remains relevant here, especially when fetching permissions.
await Promise.all([ | ||
trpc.polls.get.prefetch({ urlId: params.urlId }), | ||
trpc.polls.participants.list.prefetch({ pollId: params.urlId }), | ||
trpc.polls.comments.list.prefetch({ pollId: params.urlId }), | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for prefetch failures
The current implementation does not handle cases where the prefetch operations might fail or return null (e.g., if the poll does not exist or the user lacks permissions). This could lead to unexpected behaviors or runtime errors during rendering. Consider adding error handling to manage potential errors from trpc.polls.get.prefetch
and other prefetch calls.
You can check the results after prefetching and handle errors appropriately. For example:
await Promise.all([
trpc.polls.get.prefetch({ urlId: params.urlId }),
trpc.polls.participants.list.prefetch({ pollId: params.urlId }),
trpc.polls.comments.list.prefetch({ pollId: params.urlId }),
]);
+const pollData = trpc.queryClient.getQueryData(['polls.get', { urlId: params.urlId }]);
+if (!pollData) {
+ notFound();
+}
Committable suggestion skipped: line range outside the PR's diff.
staleTime: Infinity, | ||
}, | ||
); | ||
const pollQuery = trpc.polls.get.useQuery({ urlId: params?.urlId as string }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider using type-safe parameter handling
The type assertion as string
could be unsafe. Consider adding a runtime check or using optional chaining with a fallback.
- const pollQuery = trpc.polls.get.useQuery({ urlId: params?.urlId as string });
+ if (!params?.urlId) {
+ throw new Error("URL ID is required");
+ }
+ const pollQuery = trpc.polls.get.useQuery({ urlId: params.urlId });
📝 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 pollQuery = trpc.polls.get.useQuery({ urlId: params?.urlId as string }); | |
if (!params?.urlId) { | |
throw new Error("URL ID is required"); | |
} | |
const pollQuery = trpc.polls.get.useQuery({ urlId: params.urlId }); |
export default async function Page({ | ||
searchParams, | ||
}: { | ||
params: { urlId: string }; | ||
searchParams: { token: string }; | ||
}) { | ||
return ( | ||
<PermissionContext token={searchParams.token}> | ||
<InvitePage /> | ||
</PermissionContext> | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider adding loading state handling
The Page component should handle loading states while permissions are being fetched.
export default async function Page({
searchParams,
}: {
params: { urlId: string };
searchParams: { token: string };
}) {
+ // Consider adding Suspense boundary
+ return (
+ <Suspense fallback={<LoadingSpinner />}>
<PermissionContext token={searchParams.token}>
<InvitePage />
</PermissionContext>
+ </Suspense>
);
}
Committable suggestion skipped: line range outside the PR's diff.
const PermissionContext = async ({ | ||
children, | ||
token, | ||
}: React.PropsWithChildren<{ token?: string }>) => { | ||
const helpers = await createSSRHelper(); | ||
let impersonatedUserId: string | null = null; | ||
if (token) { | ||
const res = await helpers.auth.getUserPermission.fetch({ token }); | ||
impersonatedUserId = res?.userId ?? null; | ||
} | ||
return ( | ||
<PermissionProvider userId={impersonatedUserId}> | ||
{children} | ||
</PermissionProvider> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhance error handling in PermissionContext
The token validation and user permission fetching lack proper error handling.
const PermissionContext = async ({
children,
token,
}: React.PropsWithChildren<{ token?: string }>) => {
const helpers = await createSSRHelper();
let impersonatedUserId: string | null = null;
if (token) {
+ try {
const res = await helpers.auth.getUserPermission.fetch({ token });
impersonatedUserId = res?.userId ?? null;
+ } catch (error) {
+ console.error('Failed to fetch user permissions:', error);
+ // Consider redirecting to error page or showing error message
+ }
}
return (
<PermissionProvider userId={impersonatedUserId}>
{children}
</PermissionProvider>
);
};
📝 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 PermissionContext = async ({ | |
children, | |
token, | |
}: React.PropsWithChildren<{ token?: string }>) => { | |
const helpers = await createSSRHelper(); | |
let impersonatedUserId: string | null = null; | |
if (token) { | |
const res = await helpers.auth.getUserPermission.fetch({ token }); | |
impersonatedUserId = res?.userId ?? null; | |
} | |
return ( | |
<PermissionProvider userId={impersonatedUserId}> | |
{children} | |
</PermissionProvider> | |
); | |
}; | |
const PermissionContext = async ({ | |
children, | |
token, | |
}: React.PropsWithChildren<{ token?: string }>) => { | |
const helpers = await createSSRHelper(); | |
let impersonatedUserId: string | null = null; | |
if (token) { | |
try { | |
const res = await helpers.auth.getUserPermission.fetch({ token }); | |
impersonatedUserId = res?.userId ?? null; | |
} catch (error) { | |
console.error('Failed to fetch user permissions:', error); | |
// Consider redirecting to error page or showing error message | |
} | |
} | |
return ( | |
<PermissionProvider userId={impersonatedUserId}> | |
{children} | |
</PermissionProvider> | |
); | |
}; |
Summary by CodeRabbit
Release Notes
New Features
PermissionProvider
for managing user permissions based on user ID.Bug Fixes
Documentation