[FE-Feat] 로그인 및 캘린더 인증 로직을 beforeLoad로 이전 #430
Conversation
WalkthroughThis update introduces a new mutation utility for handling asynchronous operations outside React components and refactors OAuth redirect routes and login mutation logic to use route-level lifecycle hooks and the new mutation utility. Additional exports are added for utility modules, and the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant RouteBeforeLoad
participant MutationUtil
participant QueryClient
User->>Router: Navigates to /oauth.redirect/login?code=...
Router->>RouteBeforeLoad: Calls beforeLoad with params
RouteBeforeLoad->>MutationUtil: Calls jwtMutation().mutateAsync(code)
MutationUtil->>MutationUtil: Performs login mutation
alt Success
MutationUtil->>RouteBeforeLoad: Throws redirect to lastPath or /home
else Error
MutationUtil->>RouteBeforeLoad: Throws redirect to /login
end
RouteBeforeLoad->>Router: Handles redirect
sequenceDiagram
participant User
participant Router
participant RouteBeforeLoad
participant QueryClient
User->>Router: Navigates to /oauth.redirect/calendar
Router->>RouteBeforeLoad: Calls beforeLoad
RouteBeforeLoad->>QueryClient: Invalidate calendar queries
RouteBeforeLoad->>Router: Throws redirect to /my-calendar
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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 (2)
frontend/packages/core/src/utils/network/createMutation.ts (2)
44-51: Consider consistent error handling between mutate and mutateAsync.The
mutatemethod has inconsistent error handling compared tomutateAsync:
- Missing redirect error check:
mutatedoesn't check for redirect errors before calling the global handler- No error rethrowing: Non-redirect errors are swallowed in
mutate, which might hide important errorsconst mutate = (args: TArgs) => { options.mutationFn(args) .then(res => options.onSuccess?.(res, args)) .catch(err => { + if (isRedirectError(err)) { + // For redirect errors, we might want to handle them differently + // or at least not call the global error handler + options.onError?.(err, args); + return; + } globalHandleError?.(err); options.onError?.(err, args); }); };Consider whether
mutateshould handle redirect errors differently since it's fire-and-forget.
25-27: Add JSDoc documentation for the complex utility function.Given the complexity noted in the Korean comment, consider adding comprehensive JSDoc documentation to help other developers understand the utility:
/** - * 리액트 컴포넌트 외부에서 사용할 수 있는 뮤테이션 함수 생성기. + * Creates mutation functions that can be used outside React components. + * + * @template TArgs - The type of arguments passed to the mutation function + * @template TRes - The type of response returned by the mutation function + * + * @param options - Configuration object for the mutation + * @param options.mutationFn - The async function to execute + * @param options.onSuccess - Callback executed on successful mutation + * @param options.onError - Callback executed on mutation error + * + * @returns Object with mutate (fire-and-forget) and mutateAsync (awaitable) methods + * + * @example + * ```ts + * const { mutateAsync } = createMutation({ + * mutationFn: async (data) => api.post('/endpoint', data), + * onSuccess: (response, args) => { + * throw redirect({ to: '/success' }); + * }, + * onError: () => { + * throw redirect({ to: '/error' }); + * } + * }); + * ``` */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
frontend/.gitignore(1 hunks)frontend/apps/client/src/features/login/api/mutations.ts(2 hunks)frontend/apps/client/src/main.tsx(2 hunks)frontend/apps/client/src/routes/oauth.redirect/calendar/index.tsx(1 hunks)frontend/apps/client/src/routes/oauth.redirect/login/index.tsx(1 hunks)frontend/packages/core/src/utils/index.ts(1 hunks)frontend/packages/core/src/utils/network/createMutation.ts(1 hunks)frontend/packages/core/src/utils/network/index.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: frontend
🔇 Additional comments (7)
frontend/.gitignore (1)
14-14: LGTM! Essential security practice.Adding
.env*to .gitignore is crucial for preventing accidental commits of environment files that may contain sensitive configuration data, especially relevant for OAuth authentication flows.frontend/apps/client/src/main.tsx (2)
1-1: LGTM! Consistent error handling setup.Importing
setDefaultMutationErrorHandlerestablishes global mutation error handling that complements the existing query error handling infrastructure.
45-45: LGTM! Proper global error handler configuration.Using the same
handleErrorfunction for mutations that's already used for queries ensures consistent error handling across all async operations.frontend/packages/core/src/utils/index.ts (1)
6-6: LGTM! Consistent module export pattern.The addition follows the established pattern of re-exporting utility modules, making the new network utilities properly accessible.
frontend/packages/core/src/utils/network/index.ts (1)
1-1: LGTM! Clean module organization.Simple and standard index file pattern for re-exporting the createMutation utilities.
frontend/apps/client/src/routes/oauth.redirect/calendar/index.tsx (1)
6-9: LGTM! Excellent refactoring to avoid React StrictMode issues.Moving the query invalidation and redirect logic from React component lifecycle to route
beforeLoadis the correct approach to prevent duplicate executions under React StrictMode. The implementation properly:
- Uses async/await for the lifecycle method
- Accesses queryClient from route context
- Invalidates queries before redirecting
- Uses the throw redirect pattern correctly
frontend/apps/client/src/features/login/api/mutations.ts (1)
12-29: LGTM! Well-implemented mutation refactor with proper redirect handling.The refactor from React Query to the custom
createMutationutility is well-executed:
- Proper redirect pattern: Using
throw redirect()instead of imperative navigation aligns with the route-level lifecycle approach.- Consistent error handling: Both success and error paths use redirects, ensuring predictable navigation flow.
- Clean API: Returning
{ loginMutate: mutateAsync }provides a clear interface for async usage inbeforeLoad.The mutation will properly handle authentication flow by redirecting to the intended destination on success or back to login on failure.
| beforeLoad: async ({ search }: { search: SearchWithCode }) => { | ||
| const lastPath = getLastRoutePath(); | ||
| const { loginMutate } = jwtMutation(); | ||
| if (search.code) { | ||
| await loginMutate({ code: search.code, lastPath }); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Add error handling and validation for the beforeLoad lifecycle.
The beforeLoad function lacks proper error handling and validation:
- Missing code validation: The function should validate that
search.codeexists and is a valid string before proceeding with the mutation. - Unhandled mutation errors: If
loginMutatethrows an error that isn't a redirect, it will cause the route loading to fail without proper user feedback. - Type safety concern: The
SearchWithCodetype allowscodeto be undefined, but the logic assumes it exists.
export const Route = createFileRoute('/oauth/redirect/login/')({
- beforeLoad: async ({ search }: { search: SearchWithCode }) => {
+ beforeLoad: async ({ search }: { search: SearchWithCode }) => {
+ if (!search.code) {
+ throw redirect({ to: '/login' });
+ }
+
const lastPath = getLastRoutePath();
const { loginMutate } = jwtMutation();
- if (search.code) {
- await loginMutate({ code: search.code, lastPath });
- }
+
+ try {
+ await loginMutate({ code: search.code, lastPath });
+ } catch (error) {
+ // Let redirect errors bubble up, handle others
+ if (error && typeof error === 'object' && 'isRedirect' in error) {
+ throw error;
+ }
+ throw redirect({ to: '/login' });
+ }
},
});📝 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.
| beforeLoad: async ({ search }: { search: SearchWithCode }) => { | |
| const lastPath = getLastRoutePath(); | |
| const { loginMutate } = jwtMutation(); | |
| if (search.code) { | |
| await loginMutate({ code: search.code, lastPath }); | |
| } | |
| }, | |
| export const Route = createFileRoute('/oauth/redirect/login/')({ | |
| beforeLoad: async ({ search }: { search: SearchWithCode }) => { | |
| // Validate that we actually have a code, otherwise redirect to login | |
| if (!search.code) { | |
| throw redirect({ to: '/login' }); | |
| } | |
| const lastPath = getLastRoutePath(); | |
| const { loginMutate } = jwtMutation(); | |
| try { | |
| await loginMutate({ code: search.code, lastPath }); | |
| } catch (error) { | |
| // If it’s a redirect-error, let it bubble up; otherwise go back to login | |
| if (error && typeof error === 'object' && 'isRedirect' in error) { | |
| throw error; | |
| } | |
| throw redirect({ to: '/login' }); | |
| } | |
| }, | |
| }); |
🤖 Prompt for AI Agents
In frontend/apps/client/src/routes/oauth.redirect/login/index.tsx around lines
11 to 17, the beforeLoad function needs improved error handling and validation.
First, add a check to ensure search.code is a defined, valid string before
calling loginMutate. Then, wrap the loginMutate call in a try-catch block to
handle any errors; if the error is not a redirect, catch it and provide
appropriate user feedback or logging to prevent route loading failure. Also,
adjust the type handling to safely account for the possibility that search.code
may be undefined.
| const isRedirectError = (error: unknown): boolean => ( | ||
| typeof error === 'object' && error !== null | ||
| && 'isRedirect' in error && error.isRedirect === true | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety of the redirect error detection.
The isRedirectError function uses loose type checking that could lead to false positives:
-const isRedirectError = (error: unknown): boolean => (
- typeof error === 'object' && error !== null
- && 'isRedirect' in error && error.isRedirect === true
-);
+const isRedirectError = (error: unknown): error is { isRedirect: true } => (
+ typeof error === 'object' &&
+ error !== null &&
+ 'isRedirect' in error &&
+ (error as { isRedirect: unknown }).isRedirect === true
+);This provides:
- Type guard return type: The function now acts as a proper type guard
- Safer property access: Explicitly casts to access the
isRedirectproperty - Better type inference: TypeScript will know the error type after the guard
📝 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 isRedirectError = (error: unknown): boolean => ( | |
| typeof error === 'object' && error !== null | |
| && 'isRedirect' in error && error.isRedirect === true | |
| ); | |
| const isRedirectError = (error: unknown): error is { isRedirect: true } => ( | |
| typeof error === 'object' && | |
| error !== null && | |
| 'isRedirect' in error && | |
| (error as { isRedirect: unknown }).isRedirect === true | |
| ); |
🤖 Prompt for AI Agents
In frontend/packages/core/src/utils/network/createMutation.ts around lines 16 to
19, the isRedirectError function uses loose type checking that may cause false
positives. Refactor the function to be a proper TypeScript type guard by
specifying its return type accordingly, explicitly cast the error to a type that
includes the isRedirect property before accessing it, and ensure the function
narrows the error type for better type inference and safer property access.
| mutateAsync: (args: TArgs) => Promise<void>; | ||
| }; | ||
|
|
||
| let globalHandleError: (error: unknown) => unknown; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Initialize the global error handler to prevent runtime errors.
The global error handler is declared but not initialized, which could cause issues if createMutation is used before setDefaultMutationErrorHandler is called:
-let globalHandleError: (error: unknown) => unknown;
+let globalHandleError: (error: unknown) => unknown = () => {
+ // Default no-op handler to prevent runtime errors
+};Or alternatively, add proper null checking:
- globalHandleError?.(error);
+ if (globalHandleError) {
+ globalHandleError(error);
+ }Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/packages/core/src/utils/network/createMutation.ts at line 14, the
global error handler variable is declared but not initialized, which can cause
runtime errors if used before being set. Initialize globalHandleError with a
default no-op function or a function that safely handles errors to prevent
undefined behavior. Alternatively, add null or undefined checks before invoking
globalHandleError to ensure safe usage.
dioo1461
left a comment
dioo1461
left a comment
There was a problem hiding this comment.
잘 동작하는 것 확인했어요! 고생하셨습니다!!
| .then(res => options.onSuccess?.(res, args)) | ||
| .catch(err => { | ||
| globalHandleError?.(err); | ||
| options.onError?.(err, args); |
There was a problem hiding this comment.
just ask;
이 부분에서는 에러를 던져주지 않아도 괜찮을까요? (궁금해서 여쭤봅니다)
#️⃣ 연관된 이슈>
📝 작업 내용> 이번 PR에서 작업한 내용을 간략히 설명해주세요(이미지 첨부 가능)
StrictMode에서 로그인 요청이 중복으로 나가는 문제를 해결했습니다.
해결과정: https://hamo0.tistory.com/100
🙏 여기는 꼭 봐주세요! > 리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요
컴포넌트 외부에서 사용할 수 있는 뮤테이션 유틸을 만들었는데 조금 복잡한 것 같아 이부분 위주로 봐주시면 좋을 것 같습니다.
Summary by CodeRabbit
Refactor
New Features
Chores
.gitignoreto exclude all.envfiles in the frontend.Style