-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add FetchClient and related types for improved API handling #33
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
Conversation
✅ Deploy Preview for js-react-kit ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View your CI Pipeline Execution ↗ for commit decd9fa
☁️ Nx Cloud last updated this comment at |
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.
Pull Request Overview
This PR introduces a comprehensive FetchClient system to replace the existing fetchClient utility, improving API handling with interceptors, enhanced error handling, and TypeScript support. The changes modernize the HTTP client architecture and align with React best practices.
- Replaces the old fetchClient utility with a robust FetchClient class that supports interceptors and improved error handling
- Adds comprehensive HTTP error handling with status code constants and OIDC authentication integration
- Updates component interfaces and imports to follow TypeScript best practices
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| react-kit/src/lib/utils/fetchClient.ts | Removes the old fetchClient implementation |
| react-kit/src/lib/constants/HttpConstants.ts | Adds HTTP status code constants for consistent error handling |
| react-kit/src/lib/config/fetch/FetchInterceptor.ts | Implements request/response interceptors with OIDC integration and comprehensive error handling |
| react-kit/src/lib/config/fetch/FetchClientTypes.ts | Defines TypeScript interfaces for request/response structures |
| react-kit/src/lib/config/fetch/FetchClient.ts | Implements the new FetchClient class with interceptor support and HTTP methods |
| react-kit/src/lib/components/buttons/EditIconButton.tsx | Updates import order and adds Readonly to props |
| react-kit/src/lib/components/OpenInNewIconLink.tsx | Renames interface from OpenInNewIconLinkProps to Props and adds Readonly |
| react-kit/src/lib/components/NextLink.tsx | Extracts inline interface to named Props interface and adds Readonly |
| react-kit/src/index.ts | Updates exports to include new FetchClient and reorganizes export order |
| package.json | Adds @axa-fr/react-oidc dependency |
| .github/copilot-instructions.md | Adds coding guidelines document for React/TypeScript best practices |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
|
|
||
| // Handle 401 Unauthorized - redirect to login | ||
| if (statusCode === HTTP_401) { | ||
| login().then((r) => r); |
Copilot
AI
Aug 13, 2025
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.
The login promise is not awaited and the result is ignored. This should either be awaited or the return value should be handled properly.
|
|
||
| console.error(`HTTP Error (${statusCode}):`, message); | ||
| return Promise.reject(formattedError); | ||
| })(); |
Copilot
AI
Aug 13, 2025
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.
The immediately invoked async function (IIFE) is not awaited, which means errors thrown inside it won't be properly handled by the interceptor. The function should either return the promise or be properly awaited.
| } | ||
|
|
||
| return processedResponse; | ||
| } catch (error) { |
Copilot
AI
Aug 13, 2025
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.
The catch block only handles network errors but doesn't handle HTTP error responses (4xx, 5xx). The response status should be checked before parsing data, and appropriate FetchError should be thrown for non-2xx responses.
|
🎉 This PR is included in version 5.12.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No description provided.