Feat: Disallow Relative import #10009
Conversation
WalkthroughThis pull request introduces a comprehensive update to import paths across the project, transitioning from relative to absolute import paths. The changes are primarily driven by the addition of a new ESLint rule ( Changes
Assessment against linked issues
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
Finishing Touches
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 (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/Patient/MedicationStatementList.tsx (1)
Line range hint
93-93: Add missing key prop to TableRow component.When mapping over an array to create elements, each element needs a unique key prop for React's reconciliation process.
- <TableRow> + <TableRow key={statement.id ?? `medication-${index}`}>
🧹 Nitpick comments (4)
src/components/Patient/MedicationStatementList.tsx (2)
Line range hint
147-156: Standardize null value handling between trigger and tooltip content.The trigger shows "-" for null values while the tooltip content shows "Not Specified". This inconsistency might confuse users. Consider standardizing the fallback text.
<Tooltip> <TooltipTrigger asChild className="max-w-60 truncate"> - <p>{statement.reason ?? "-"}</p> + <p>{statement.reason ?? "Not Specified"}</p> </TooltipTrigger> <TooltipContent> <p>{statement.reason ?? "Not Specified"}</p> </TooltipContent> </Tooltip>Also applies to: 166-175
Line range hint
94-156: Enhance tooltip accessibility.Consider adding
aria-labelto tooltip triggers to improve screen reader experience. Also, ensure the truncated text is clearly indicated to users.Example for medication name:
<Tooltip> <TooltipTrigger asChild className="max-w-60 truncate" + aria-label={`Medication name: ${statement.medication.display ?? statement.medication.code}`} >src/types/questionnaire/questionnaireApi.ts (1)
Line range hint
77-83: Use consistent type definition pattern for setOrganizations endpoint.While the endpoint implementation is correct, the request body type definition uses a different pattern from other endpoints. For consistency and better type safety, consider using the
Typeutility.setOrganizations: { path: "/api/v1/questionnaire/{id}/set_organizations/", method: HttpMethod.POST, TRes: Type<PaginatedResponse<Organization>>(), - TBody: {} as { organizations: string[] }, + TBody: Type<{ organizations: string[] }>(), },package.json (1)
Line range hint
1-102: Consider a phased migration approach for relative imports.To ensure a smooth transition to absolute imports:
- Consider adding a script to automate the conversion of relative imports to absolute paths
- Update the CI pipeline to:
- Initially run the rule in warning mode
- After a grace period, enforce as errors
- Document the new import convention in the project's coding guidelines
This will help manage the migration process and reduce developer friction.
Would you like me to help create a migration script to automate the conversion of relative imports to absolute paths?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (35)
.eslintrc.json(3 hunks)package.json(1 hunks)src/Routers/routes/questionnaireRoutes.tsx(1 hunks)src/components/Common/Charts/ObservationChart.tsx(1 hunks)src/components/Common/Charts/ObservationHistoryTable.tsx(1 hunks)src/components/Facility/DuplicatePatientDialog.tsx(1 hunks)src/components/Facility/EncounterCard.tsx(1 hunks)src/components/Files/AudioCaptureDialog.tsx(1 hunks)src/components/Patient/MedicationStatementList.tsx(1 hunks)src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx(1 hunks)src/components/Patient/PatientRegistration.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(1 hunks)src/components/Questionnaire/index.tsx(1 hunks)src/components/Questionnaire/show.tsx(1 hunks)src/components/Resource/ResourceCommentSection.tsx(1 hunks)src/components/Resource/ResourceDetails.tsx(1 hunks)src/components/Users/UserSummary.tsx(1 hunks)src/components/ui/sidebar/patient-switcher.tsx(1 hunks)src/pages/Facility/FacilitiesPage.tsx(1 hunks)src/pages/Facility/components/FacilityCard.tsx(1 hunks)src/pages/PublicAppointments/PatientRegistration.tsx(1 hunks)src/types/emr/allergyIntolerance/allergyIntolerance.ts(1 hunks)src/types/emr/diagnosis/diagnosis.ts(1 hunks)src/types/emr/encounter.ts(1 hunks)src/types/emr/newPatient.ts(1 hunks)src/types/emr/observation.ts(1 hunks)src/types/emr/symptom/symptom.ts(1 hunks)src/types/facility/facility.ts(1 hunks)src/types/facilityOrganization/facilityOrganization.ts(1 hunks)src/types/organization/organization.ts(1 hunks)src/types/organization/organizationApi.ts(1 hunks)src/types/questionnaire/questionnaireApi.ts(1 hunks)src/types/questionnaire/questionnaireResponse.ts(1 hunks)src/types/resourceRequest/resourceRequest.ts(1 hunks)
✅ Files skipped from review due to trivial changes (29)
- src/types/facilityOrganization/facilityOrganization.ts
- src/Routers/routes/questionnaireRoutes.tsx
- src/components/Users/UserSummary.tsx
- src/types/facility/facility.ts
- src/components/Resource/ResourceCommentSection.tsx
- src/components/Facility/EncounterCard.tsx
- src/types/resourceRequest/resourceRequest.ts
- src/components/Common/Charts/ObservationHistoryTable.tsx
- src/types/emr/observation.ts
- src/components/ui/sidebar/patient-switcher.tsx
- src/components/Common/Charts/ObservationChart.tsx
- src/components/Patient/PatientRegistration.tsx
- src/types/emr/newPatient.ts
- src/components/Questionnaire/show.tsx
- src/types/questionnaire/questionnaireResponse.ts
- src/components/Questionnaire/index.tsx
- src/types/organization/organization.ts
- src/components/Resource/ResourceDetails.tsx
- src/components/Questionnaire/QuestionnaireEditor.tsx
- src/pages/Facility/components/FacilityCard.tsx
- src/types/emr/allergyIntolerance/allergyIntolerance.ts
- src/components/Files/AudioCaptureDialog.tsx
- src/types/emr/diagnosis/diagnosis.ts
- src/components/Facility/DuplicatePatientDialog.tsx
- src/types/emr/encounter.ts
- src/pages/Facility/FacilitiesPage.tsx
- src/types/emr/symptom/symptom.ts
- src/pages/PublicAppointments/PatientRegistration.tsx
- src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx
🔇 Additional comments (8)
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx (1)
13-13: LGTM! Import path standardization looks good.The update from relative path to absolute path using
@/prefix aligns with the PR objective of standardizing imports across the codebase.src/types/organization/organizationApi.ts (1)
3-3: LGTM! Import path updated to use absolute path.The change from relative to absolute import path aligns with the PR objective and maintains consistency with other imports in the file.
Let's verify the TypeScript path alias configuration:
✅ Verification successful
Import path configuration verified successfully
The TypeScript path alias configuration correctly maps
@/*to./src/*, and the target file exists at the expected pathsrc/types/emr/newPatient.ts. The absolute import path is properly configured and will resolve correctly.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify TypeScript path alias configuration for absolute imports # Check tsconfig.json for path alias configuration echo "Checking tsconfig.json for '@' path alias..." cat tsconfig.json | jq '.compilerOptions.paths' # Verify that the target file exists at the expected path echo "Verifying the imported file exists..." fd "newPatient.ts" --type fLength of output: 405
src/components/Patient/MedicationStatementList.tsx (1)
17-21: LGTM! Import changes align with the standardization effort.The new tooltip component imports follow the absolute path pattern using
@/prefix, maintaining consistency with the project's import standardization initiative.src/types/questionnaire/questionnaireApi.ts (2)
3-3: LGTM! Import path standardization looks good.The change to use absolute import path with
@/prefix aligns with the PR objective of standardizing imports across the codebase.
Line range hint
71-83: Verify path parameter naming consistency across API endpoints.The new endpoints use
{id}as the path parameter, which matches other endpoints in this file. Let's verify this is consistent across the codebase.✅ Verification successful
Path parameter naming is consistent with the system design
The different parameter names (
{id}vs{slug}) in the questionnaire API endpoints are intentionally designed this way, as the tags-related endpoints consistently use slugs while other questionnaire operations use IDs.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for API endpoint definitions to verify path parameter naming consistency rg -g '*.ts' -g '*.tsx' '/api/v\d+/[^"]+/\{[^}]+\}/' --no-headingLength of output: 8372
.eslintrc.json (2)
45-46: LGTM! Plugin addition looks good.The
no-relative-import-pathsplugin is correctly added to the plugins array.
82-87: Verify the rule configuration impact.The rule configuration looks good with appropriate settings:
- Error level enforcement ensures strict adoption
allowSameFolder: trueprovides practical flexibilityprefix: "@"standardizes absolute importsHowever, let's verify the potential impact on the codebase.
✅ Verification successful
ESLint rule configuration is safe to implement ✅
The rule configuration perfectly aligns with your existing TypeScript path aliases and current codebase practices. No relative imports were found that would break under this rule, indicating zero migration effort needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for existing relative imports that would break under the new rule # This helps assess the migration effort needed # Search for relative imports (../ or ./) echo "Checking for relative imports that need to be updated:" rg "from ['\"]\.\.?/" --type ts --type tsx # Count total occurrences echo -e "\nTotal count of files with relative imports:" rg "from ['\"]\.\.?/" --type ts --type tsx --count | wc -lLength of output: 389
Script:
#!/bin/bash # Check for relative imports in JS/TS files echo "Files with relative imports:" rg "from ['\"]\.\.?/" -t js -t jsx -t ts -t mjs -t cjs -C 1 # Check for tsconfig.json and package.json echo -e "\nChecking for TypeScript configuration:" fd tsconfig.json echo -e "\nChecking package.json:" cat package.json | jq '.dependencies, .devDependencies'Length of output: 4725
Script:
#!/bin/bash # Search for relative imports with simpler pattern and correct syntax echo "Checking for relative imports:" rg "from '\.\." --type=js --type=ts echo -e "\nChecking root tsconfig.json for path aliases:" cat tsconfig.json | jq '.compilerOptions.paths'Length of output: 378
package.json (1)
158-158: LGTM! Dependency addition is correct.The ESLint plugin is:
- Added to the correct section (devDependencies)
- Uses appropriate version range (^1.6.1)
- Properly sorted alphabetically
|
👋 Hi, @AdityaJ2305, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/types/questionnaire/questionnaireApi.ts (1)
Line range hint
74-84: Use Type utility for request body type definition.The
setOrganizationsendpoint uses a runtime type assertion forTBody. This should be consistent with other endpoints by using theTypeutility.Apply this change:
setOrganizations: { path: "/api/v1/questionnaire/{id}/set_organizations/", method: HttpMethod.POST, TRes: Type<PaginatedResponse<Organization>>(), - TBody: {} as { organizations: string[] }, + TBody: Type<{ organizations: string[] }>(), },
🧹 Nitpick comments (3)
src/pages/Encounters/EncounterShow.tsx (2)
Line range hint
52-117: Remove commented-out code blocks.Large blocks of commented code make the file harder to maintain and understand. If this code might be needed in the future, it's better tracked through version control. Consider:
- Removing all commented code blocks
- If some commented code represents pending work, replace with TODO comments referencing specific issue numbers
Also applies to: 171-196
Line range hint
143-151: Consolidate error checks.The check for
!encounterDatais redundant as it's already handled in the loading condition above. Consider consolidating these checks:- if (!props.tab) { - return <ErrorPage />; - } - - if (!encounterData) { - return <ErrorPage />; - } + if (!props.tab) { + return <ErrorPage />; + }.eslintrc.json (1)
64-68: Consider establishing clear module boundaries with barrel exports.While enforcing absolute imports, consider:
- Creating barrel exports (index.ts) for major feature modules
- Establishing a clear public API for each module
- Documenting the import convention in the project README
This will help maintain a clean architecture and make the codebase more maintainable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
.eslintrc.json(3 hunks)src/App.tsx(1 hunks)src/Routers/AppRouter.tsx(1 hunks)src/Routers/PatientRouter.tsx(1 hunks)src/Utils/request/useQuery.ts(1 hunks)src/components/Common/Charts/ObservationChart.tsx(1 hunks)src/components/Common/UserAutocompleteFormField.tsx(1 hunks)src/components/Patient/PatientDetailsTab/index.tsx(1 hunks)src/components/Patient/PatientInfoCard.tsx(1 hunks)src/components/Patient/diagnosis/list.tsx(1 hunks)src/components/Patient/symptoms/list.tsx(1 hunks)src/components/Questionnaire/QuestionRenderer.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/QuestionInput.tsx(1 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(1 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Questionnaire/show.tsx(1 hunks)src/components/Users/UserBanner.tsx(1 hunks)src/pages/Encounters/EncounterShow.tsx(1 hunks)src/pages/Facility/FacilitiesPage.tsx(1 hunks)src/pages/Facility/FacilityDetailsPage.tsx(1 hunks)src/pages/FacilityOrganization/FacilityOrganizationIndex.tsx(1 hunks)src/pages/FacilityOrganization/FacilityOrganizationUsers.tsx(1 hunks)src/pages/FacilityOrganization/FacilityOrganizationView.tsx(1 hunks)src/pages/Organization/OrganizationFacilities.tsx(1 hunks)src/pages/Organization/OrganizationPatients.tsx(1 hunks)src/pages/Organization/OrganizationUsers.tsx(1 hunks)src/pages/Organization/OrganizationView.tsx(1 hunks)src/pages/Patient/index.tsx(1 hunks)src/pluginTypes.ts(1 hunks)src/types/emr/allergyIntolerance/allergyIntoleranceApi.ts(1 hunks)src/types/emr/diagnosis/diagnosisApi.ts(1 hunks)src/types/emr/symptom/symptomApi.ts(1 hunks)src/types/facility/facilityApi.ts(1 hunks)src/types/organization/organizationApi.ts(1 hunks)src/types/questionnaire/question.ts(1 hunks)src/types/questionnaire/questionnaire.ts(1 hunks)src/types/questionnaire/questionnaireApi.ts(1 hunks)src/types/questionnaire/questionnaireResponse.ts(1 hunks)
✅ Files skipped from review due to trivial changes (26)
- src/Utils/request/useQuery.ts
- src/components/Patient/diagnosis/list.tsx
- src/pages/FacilityOrganization/FacilityOrganizationUsers.tsx
- src/pages/Organization/OrganizationView.tsx
- src/types/facility/facilityApi.ts
- src/types/emr/allergyIntolerance/allergyIntoleranceApi.ts
- src/types/questionnaire/question.ts
- src/components/Questionnaire/QuestionTypes/QuestionGroup.tsx
- src/pages/Organization/OrganizationUsers.tsx
- src/pages/Organization/OrganizationPatients.tsx
- src/components/Patient/PatientInfoCard.tsx
- src/types/emr/symptom/symptomApi.ts
- src/components/Users/UserBanner.tsx
- src/pages/Organization/OrganizationFacilities.tsx
- src/types/emr/diagnosis/diagnosisApi.ts
- src/components/Common/UserAutocompleteFormField.tsx
- src/Routers/PatientRouter.tsx
- src/pages/Patient/index.tsx
- src/components/Patient/symptoms/list.tsx
- src/pages/FacilityOrganization/FacilityOrganizationView.tsx
- src/pages/Facility/FacilityDetailsPage.tsx
- src/pages/FacilityOrganization/FacilityOrganizationIndex.tsx
- src/components/Questionnaire/QuestionRenderer.tsx
- src/components/Questionnaire/QuestionnaireForm.tsx
- src/types/questionnaire/questionnaire.ts
- src/components/Questionnaire/QuestionTypes/QuestionInput.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/components/Common/Charts/ObservationChart.tsx
- src/pages/Facility/FacilitiesPage.tsx
- src/types/organization/organizationApi.ts
- src/types/questionnaire/questionnaireResponse.ts
- src/components/Questionnaire/show.tsx
- src/components/Questionnaire/QuestionnaireEditor.tsx
🔇 Additional comments (13)
src/App.tsx (1)
20-20: LGTM! Import path updated correctly.The change from relative to absolute import path aligns with the PR objective and maintains consistency with other imports in the file.
Let's verify the path alias configuration:
✅ Verification successful
Path alias configuration verified successfully
The
@/import path is properly configured in the TypeScript configuration, mapping@/*to./src/*. The import statement follows the correct pattern.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for path alias configuration in common config files # Check for path alias in tsconfig.json echo "Checking tsconfig.json for path aliases..." fd -t f "tsconfig.*json" -x cat {} | jq -r '.compilerOptions.paths | select(. != null)' # Check for path alias in vite.config.ts/js echo "Checking vite config for path aliases..." fd -t f "vite.config.(ts|js)" -x cat {}Length of output: 433
src/pages/Encounters/EncounterShow.tsx (2)
19-19: LGTM! Import path follows the new convention.The import statement correctly uses an absolute path with the
@/prefix, which aligns with the PR objective to disallow relative imports.
Line range hint
31-36: LGTM! EncounterNotesTab integration looks good.The
noteskey indefaultTabscorrectly references the importedEncounterNotesTabcomponent, maintaining consistency with other tab components.src/types/questionnaire/questionnaireApi.ts (2)
3-11: LGTM! Import paths successfully updated to absolute paths.The changes align with the PR objective of transitioning from relative to absolute imports, maintaining consistent use of the '@/' prefix.
Line range hint
74-84: Verify API endpoint response types with backend.Please ensure that both new endpoints' response types (
PaginatedResponse<Organization>) match the backend implementation.Run this script to check the API documentation or implementation:
src/components/Patient/PatientDetailsTab/index.tsx (2)
Line range hint
1-40: LGTM! The changes align with the PR objective.The conversion from relative to absolute imports is consistent with the PR's goal to disallow relative imports. The component structure and tab configuration remain unchanged, maintaining the existing functionality.
3-8: Verify that all imported components exist at the specified paths.The import paths have been updated to use absolute paths with the
@/prefix. Let's verify these components exist at the specified locations.✅ Verification successful
All component imports are valid and correctly mapped to their source files.
The following components are properly exported from their respective files in the
src/components/Patient/PatientDetailsTabdirectory:
AppointmentsfromAppointments.tsxDemographyfromDemography.tsxPatientFilesTabfromPatientFiles.tsxPatientUsersfromPatientUsers.tsxResourceRequestsfromResourceRequests.tsxUpdatesfrompatientUpdates.tsx🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all imported components exist at the specified paths # Check if the component files exist components=( "components/Patient/PatientDetailsTab/Appointments" "components/Patient/PatientDetailsTab/PatientFiles" "components/Patient/PatientDetailsTab/PatientUsers" "components/Patient/PatientDetailsTab/ResourceRequests" ) for component in "${components[@]}"; do echo "Checking for $component..." fd -t f "^index\.(tsx?|jsx?)$|^[^/]+\.(tsx?|jsx?)$" "src/$component" doneLength of output: 1954
Script:
#!/bin/bash # Search for each component definition using ripgrep components=("Appointments" "Demography" "PatientFilesTab" "PatientUsers" "ResourceRequests" "Updates") for component in "${components[@]}"; do echo "=== Searching for component: $component ===" # Search for component definition rg -l --type ts --type tsx "export.*${component}.*" src/ # Also search with type/interface definitions to catch potential issues rg -l --type ts --type tsx "type.*${component}.*|interface.*${component}.*" src/ done # Additionally, let's verify the specific PatientDetailsTab directory structure echo -e "\n=== Checking PatientDetailsTab directory ===" fd . "src/components/Patient/PatientDetailsTab"Length of output: 2810
Script:
#!/bin/bash # Check the actual exports from the component files files=( "src/components/Patient/PatientDetailsTab/PatientFiles.tsx" "src/components/Patient/PatientDetailsTab/patientUpdates.tsx" ) for file in "${files[@]}"; do echo "=== Checking exports in $file ===" ast-grep --pattern 'export $_' "$file" doneLength of output: 737
Script:
#!/bin/bash # Check component exports with more specific patterns files=( "Appointments.tsx" "Demography.tsx" "PatientFiles.tsx" "PatientUsers.tsx" "ResourceRequests.tsx" "patientUpdates.tsx" ) for file in "${files[@]}"; do echo "=== Checking exports in $file ===" ast-grep --pattern 'export const $_ = $_' "src/components/Patient/PatientDetailsTab/$file" ast-grep --pattern 'export function $_($_) { $$$ }' "src/components/Patient/PatientDetailsTab/$file" doneLength of output: 77802
.eslintrc.json (4)
22-23: LGTM! Appropriate MDX configuration.The MDX file configuration is well-structured and follows recommended practices.
38-38: LGTM! Plugin addition aligns with PR objective.The addition of the
no-relative-import-pathsplugin is necessary to support the new absolute import path requirements.
56-57: Review the permissive exclude patterns in i18next configuration.The use of
".*"in exclude patterns effectively bypasses all i18next checks. This could lead to untranslated strings making their way into the application. Consider:
- Using more specific exclude patterns
- Removing the exclude patterns if they're not necessary
Let's verify the impact:
Also applies to: 60-60
64-68: Verify the impact of strict relative import restrictions.While the rule configuration aligns with the objective, let's verify that this won't cause issues with:
- Test files that might benefit from relative imports
- Deeply nested component structures
- Package exports
Let's analyze the codebase structure:
✅ Verification successful
Consider adding allowance patterns for test files
The codebase structure is compatible with the strict relative import restriction, but consider:
- Adding
allowPatternsto the rule config for test files (.test., .spec.) to maintain flexibility in testing- Including a comment in the config explaining the '@' prefix choice for team reference
Example:
{ "prefix": "@", "allowPatterns": ["**/*.test.*", "**/*.spec.*"] }🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential problematic import patterns echo "Analyzing directory structure..." find src -type d -mindepth 3 | sort -r | head -n 5 echo "Checking test file imports..." rg -t tsx -t jsx "from '\.\." --glob "*.test.*" --glob "*.spec.*" echo "Checking package exports..." find src -name "index.ts" -o -name "index.tsx" | while read -r file; do echo "=== $file ===" rg "export .* from '\.\." "$file" || true doneLength of output: 1840
Script:
#!/bin/bash # Retry test file analysis with correct parameters echo "Checking test file imports..." rg "from '\.\." --type=typescript --type=javascriptreact --type=typescriptreact --glob "*.test.*" --glob "*.spec.*" echo "Checking existing absolute import patterns..." rg "from '@/" --type=typescript --type=javascriptreact --type=typescriptreact | head -n 5Length of output: 455
Script:
#!/bin/bash # Check imports using file extensions echo "Checking test file imports..." rg "from '\.\." --glob "*.ts" --glob "*.tsx" --glob "*.jsx" --glob "*.js" --glob "*.test.*" --glob "*.spec.*" echo "Checking existing absolute import patterns..." rg "from '@/" --glob "*.ts" --glob "*.tsx" --glob "*.jsx" --glob "*.js" | head -n 5Length of output: 373
src/pluginTypes.ts (1)
4-4: LGTM! Verify path resolution for absolute imports.The conversion from relative to absolute imports is correct and aligns with the standardization objective.
Let's verify that the import paths resolve correctly:
Also applies to: 7-7, 9-9
src/Routers/AppRouter.tsx (1)
18-18: Consider path structure consistency.While the conversion to absolute imports is correct, the path structure has been made longer than necessary:
- Current:
@/Routers/routes/...- Could be:
@/routes/...This doesn't affect functionality but might impact maintainability if the directory structure changes.
Let's verify the path resolution and check for consistency across the codebase:
Also applies to: 23-23
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/Utils/request/types.ts (1)
Line range hint
10-16: Fix inconsistent usage of TBody type parameter.The TBody type parameter is used inconsistently in the interface. It's defined as a generic parameter but then redeclared as an optional property, which could lead to type safety issues.
export interface ApiRoute<TData, TBody = unknown> { method?: "GET" | "POST" | "PUT" | "PATCH" | "DELETE"; - TBody?: TBody; path: string; TRes: TData; noAuth?: boolean; }
🧹 Nitpick comments (3)
src/Utils/request/types.ts (3)
Line range hint
1-7: Consider using more targeted ESLint disable directives.Instead of disabling the rule for the entire file, consider using inline directives for specific variables or declarations that trigger the warning. This helps maintain better code quality by ensuring unused variables are caught elsewhere in the file.
-/* eslint-disable @typescript-eslint/no-unused-vars */ +// Add inline directives where needed, e.g.: +// eslint-disable-next-line @typescript-eslint/no-unused-vars
Line range hint
18-32: Enhance deprecation notices with migration guidance.While the deprecation notices indicate the preferred alternatives, they could be more helpful by including:
- Migration instructions
- Links to documentation
- Target removal version/date
Also, consider using the more type-safe
StructuredErrortype for the error property inRequestResult./** - * @deprecated in favor of useQuery/useMutation/callApi + * @deprecated Since version X.Y.Z. Use useQuery/useMutation/callApi instead. + * @see {link to migration guide} + * @example + * // Old way + * const result = await makeRequest(); + * // New way + * const result = await callApi(); */ export interface RequestResult<TData> { res: Response | undefined; data: TData | undefined; - error: undefined | Record<string, unknown>; + error: undefined | StructuredError; }
Line range hint
44-51: Consider tightening the type constraint on Route.The current constraint
Route extends ApiRoute<unknown, unknown>is very permissive. Consider constraining it to ensure the route has the expected shape.-export interface ApiCallOptions<Route extends ApiRoute<unknown, unknown>> { +export interface ApiCallOptions<Route extends ApiRoute<any, any>> { pathParams?: PathParams<Route["path"]>; queryParams?: QueryParams; body?: Route["TBody"]; silent?: boolean | ((response: Response) => boolean); signal?: AbortSignal; headers?: HeadersInit; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Routers/AppRouter.tsx(2 hunks)src/Utils/request/types.ts(1 hunks)src/Utils/types.ts(1 hunks)src/components/ui/chart.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/Utils/types.ts
- src/components/ui/chart.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Routers/AppRouter.tsx
🔇 Additional comments (3)
src/Utils/request/types.ts (3)
Line range hint
34-42: Well-implemented route parameter extraction!The types make excellent use of TypeScript's type system to provide compile-time safety for route parameters.
Line range hint
53-77: Excellent error handling implementation!The error types and HTTPError class are well-structured and provide good type safety for error handling.
Line range hint
79-90: Clean implementation of pagination types!The module augmentation and pagination interface are well-designed and provide good type safety.
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable @typescript-eslint/no-unused-vars */ | |||
There was a problem hiding this comment.
Avoid using eslint-disable for an entire file, as it can unintentionally bypass important linting rules and introduce issues. Instead, disable ESLint only for specific lines or blocks of code where necessary, ensuring minimal impact.
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable @typescript-eslint/no-unused-vars */ | |||
There was a problem hiding this comment.
Avoid using eslint-disable for an entire file, as it can unintentionally bypass important linting rules and introduce issues. Instead, disable ESLint only for specific lines or blocks of code where necessary, ensuring minimal impact.
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Chores
@/prefix.no-relative-import-pathsto enforce import path conventions.eslint-plugin-no-relative-import-paths.New Features
getOrganizationsandsetOrganizationsto manage organization-related data.MedicationStatementListcomponent for enhanced user interface.Documentation