removed lodash imports and dependencies and wrote js equivalents #9116
Conversation
WalkthroughThe pull request involves significant modifications to various components and utilities within the codebase, primarily aimed at removing dependencies on the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/components/Patient/DiagnosesFilter.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@typescript-eslint/parser' declared in '.eslintrc.json': Cannot find module '@typescript-eslint/parser'
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (3)src/components/Patient/PatientRegister.tsx (3)
The changes successfully replace lodash imports with local utility functions. Also applies to: 85-85
The change correctly uses the local Line range hint The current implementation has several issues:
Recommend setting a reasonable delay (e.g., 300ms) and implementing the hook improvements: - const duplicateCheck = useDebounce(async (phoneNo: string) => {
+ const duplicateCheck = useDebounce(async (phoneNo: string) => {
if (
phoneNo &&
PhoneNumberValidator()(parsePhoneNumber(phoneNo) ?? "") === undefined
) {
const query = {
phone_number: parsePhoneNumber(phoneNo),
};
const { res, data } = await request(routes.searchPatient, {
query,
});
if (res?.ok && data?.results) {
const duplicateList = !id
? data.results
: data.results.filter(
(item: DupPatientModel) => item.patient_id !== id,
);
if (duplicateList.length) {
setStatusDialog({
show: true,
patientList: duplicateList,
});
}
}
}
- }, 0);
+ }, 300);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. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| if (!current[key]) current[key] = {}; | ||
| current = current[key]; | ||
| } | ||
| current[keys[keys.length - 1]] = value; |
Check warning
Code scanning / CodeQL
Prototype-polluting function Medium
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (19)
src/Utils/stringUtils.ts (2)
19-22: Add input validation to capitalize functionThe implementation is simple and focused but needs input validation.
Consider this improved implementation:
-export const capitalize = (str: string) => { +export const capitalize = (str: string): string => { + if (!str) return ''; + if (str.length === 1) return str.toUpperCase(); + return str.charAt(0).toUpperCase() + str.slice(1).toLowerCase(); };
1-22: Consider enhancing module documentation and testingWhile the utility functions provide a good foundation for replacing lodash, consider these improvements:
- Add JSDoc comments with examples for each function
- Create a comprehensive test suite covering edge cases
- Consider adding TypeScript strict type checks
- Add a module-level comment explaining the purpose of this utility file
Would you like me to help generate:
- JSDoc documentation with examples?
- A test suite for these utilities?
src/Utils/Notifications.js (1)
Line range hint
44-54: Add tests for error message formattingThe error message formatting logic uses the newly implemented string utility functions in a critical path. We should ensure the formatting behavior remains consistent after removing lodash.
Consider adding test cases for the following scenarios:
- Complex object keys that need camelCase transformation
- Various error message formats
- Edge cases like empty strings or special characters
Example test cases:
const testCases = [ { 'user_name': ['Invalid'] }, { 'COMPLEX_ERROR_KEY': ['Error 1', 'Error 2'] }, { 'nested.key.path': ['Test error'] } ];Would you like me to help create a comprehensive test suite for this functionality?
src/components/Patient/DiagnosesFilter.tsx (2)
37-53: Consider enhancing the useDebounce hook with cleanup and stronger typing.The implementation is correct and follows React hooks best practices. However, consider these improvements:
-function useDebounce(callback: (...args: any[]) => void, delay: number) { +function useDebounce<T extends (...args: any[]) => void>( + callback: T, + delay: number +) { const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); const debouncedCallback = useCallback( - (...args: any[]) => { + (...args: Parameters<T>) => { if (timeoutRef.current) { clearTimeout(timeoutRef.current); } timeoutRef.current = setTimeout(() => { callback(...args); }, delay); }, [callback, delay], ); + useEffect(() => { + return () => { + if (timeoutRef.current) { + clearTimeout(timeoutRef.current); + } + }; + }, []); return debouncedCallback; }Changes suggested:
- Add cleanup in useEffect to prevent memory leaks
- Add generic type parameter for better type inference
- Use Parameters utility type to maintain type safety of arguments
89-91: LGTM! Consider adding type safety to the query parameter.The implementation successfully replaces lodash's debounce with the custom hook while maintaining the same functionality.
- const debouncedQuery = useDebounce((query: string) => { + const debouncedQuery = useDebounce((query: string): void => { refetch({ query: { query } }); }, 300);Also applies to: 113-113
src/components/Facility/Investigations/Reports/utils.tsx (2)
15-42: Enhance type safety in data transformationThe implementation effectively uses native JS methods, but type safety could be improved by avoiding
anytypes.Consider these type improvements:
- data.map((value: any) => [ + data.map((value: InvestigationResponse[number]) => [ - (acc, value: any) => { + (acc: { [key: string]: InvestigationResponse }, value: InvestigationResponse[number]) => {
Line range hint
89-108: Improve maintainability of color index calculationThe color index calculation uses magic numbers and could benefit from better documentation and constants.
Consider these improvements:
+ const COLOR_INDICES = { + YELLOW: 1, + GREEN: 5, + RED: 7, + INVALID: -1, + BUCKET_COUNT: 3 + } as const; + + /** + * Calculates color index based on value's position relative to min/max bounds + * @returns + * - 1-2: Low range (yellow) + * - 3-5: Normal range (green) + * - 6-8: High range (red) + * - -1: Invalid input + */ export const getColorIndex = memoize( ({ max, min, value }: { min?: number; max?: number; value?: number }) => { if (!max && min && value) { - return value < min ? 1 : 5; + return value < min ? COLOR_INDICES.YELLOW : COLOR_INDICES.GREEN; } // ... apply similar changes to other conditionssrc/components/Form/Form.tsx (1)
60-65: LGTM! Successfully replaced lodash with native JavaScript.The implementation correctly replaces
lodash-es'somitBywith native JavaScript methods while maintaining the same functionality. The type assertion ensures type safety.Consider extracting the filtering logic into a utility function for better maintainability and reusability:
+const filterNonEmptyEntries = <T extends Record<string, any>>(obj: T) => + Object.fromEntries( + Object.entries(obj).filter( + ([_key, value]) => value !== "" && value !== null && value !== undefined + ) + ) as T; const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => { event.preventDefault(); event.stopPropagation(); if (validate) { - const errors = Object.fromEntries( - Object.entries(validate(state.form)).filter( - ([_key, value]) => - value !== "" && value !== null && value !== undefined, - ), - ) as FormErrors<T>; + const errors = filterNonEmptyEntries(validate(state.form)) as FormErrors<T>;src/components/Facility/Investigations/Table.tsx (1)
62-69: Add error handling for invalid pathsThe function assumes the path is always valid but should handle edge cases gracefully.
Consider adding these safety checks:
const handleValueChange = (value: any, name: string) => { + if (!name || typeof name !== 'string') { + console.error('Invalid path provided'); + return; + } const form = { ...state }; const keys = name.split("."); + if (keys.length === 0) { + console.error('Empty path provided'); + return; + } let current = form; for (let i = 0; i < keys.length - 1; i++) { const key = keys[i]; if (!current[key]) current[key] = {}; current = current[key]; } current[keys[keys.length - 1]] = value; dispatch({ type: "set_form", form }); };src/components/Facility/Investigations/ShowInvestigation.tsx (1)
159-171: LGTM! Consider minor optimization for readabilityThe native JavaScript implementation successfully replaces lodash functionality. The code is clear and maintainable.
Optional: Consider this slightly more concise version using object property shorthand:
const changedValues = Object.keys(state.initialValues).reduce( (acc: any, key: any) => { const val = state.initialValues[key]; + const value = val?.value || null; + const notes = val?.notes || null; acc[key] = { - id: val?.id, - initialValue: val?.notes || val?.value || null, - value: val?.value || null, - notes: val?.notes || null, + id: val?.id, + initialValue: notes || value || null, + value, + notes, }; return acc; }, {}, );src/components/Form/AutoCompleteAsync.tsx (3)
Line range hint
71-92: Implementation of custom debouncing looks good, but consider memory leak preventionThe replacement of lodash's debounce with native setTimeout is well implemented. However, we should ensure cleanup when the component unmounts.
Consider adding a cleanup function in useEffect to prevent potential memory leaks:
useEffect(() => { fetchDataAsync(query); + return () => { + clearTimeout(timeoutId); + }; }, [query, fetchDataAsync]);
Line range hint
78-90: Consider error handling for failed API callsThe async operation inside setTimeout should include error handling to ensure the loading state is properly reset even when the API call fails.
Add try-catch block:
timeoutId = setTimeout(async () => { + try { const data = ((await fetchData(query)) || [])?.filter((d: any) => filter ? filter(d) : true, ); if (showNOptions !== undefined) { setData(data.slice(0, showNOptions)); } else { setData(data); } + } catch (error) { + console.error('Failed to fetch autocomplete data:', error); + setData([]); + } finally { setLoading(false); + } }, debounceTime);
Line range hint
14-39: Consider strengthening TypeScript typesThe Props interface uses
anytypes in several places which reduces type safety. Consider creating more specific types for the data structure.Example improvement:
interface Option { id: string | number; label: string; [key: string]: any; // for additional properties } interface Props<T extends Option = Option> { // ... other props selected: T | T[]; fetchData: (search: string) => Promise<T[]> | undefined; onChange: (selected: T | T[] | null) => void; optionLabel?: (option: T) => string; // ... rest of the props }src/components/Patient/SampleTestCard.tsx (1)
104-104: Consider using meaningful default valuesWhile adding null checks is good, using empty strings as fallbacks might not be the most user-friendly approach. Consider using more descriptive defaults that indicate the absence of data.
- {startCase(camelCase(itemData.status || ""))} + {startCase(camelCase(itemData.status || "Not Set"))} - {startCase(camelCase(itemData.result || ""))} + {startCase(camelCase(itemData.result || "Pending"))}Also applies to: 136-136
src/components/Common/ExcelFIleDragAndDrop.tsx (2)
70-72: Consider enhancing date handling robustnessThe current date conversion logic could benefit from some improvements:
- Add validation for invalid dates
- Consider timezone handling
- Avoid direct mutation of the row object
- Extract to a separate utility function
Consider refactoring to something like this:
+ const convertExcelDate = (date: Date): string => { + if (!(date instanceof Date) || isNaN(date.getTime())) { + return ''; + } + return date.toISOString().split('T')[0]; + }; data.forEach((row: any) => { - Object.keys(row).forEach((key) => { - if (row[key] instanceof Date) { - row[key] = row[key].toISOString().split("T")[0]; - } - }); + const convertedRow = { ...row }; + Object.keys(convertedRow).forEach((key) => { + if (convertedRow[key] instanceof Date) { + convertedRow[key] = convertExcelDate(convertedRow[key]); + } + }); + return convertedRow; });
70-72: Enhance type safety by removinganytypesThe current implementation uses
anytypes which reduces type safety. Consider defining proper interfaces for the Excel data structure.Consider adding these types:
interface ExcelRow { [key: string]: string | Date | number; } interface ExcelData extends Array<ExcelRow> {}Then update the code:
- data.forEach((row: any) => { + data.forEach((row: ExcelRow) => {src/components/Facility/Investigations/Reports/index.tsx (1)
181-190: Well-implemented lodash removal with native JavaScript methods!The refactoring effectively replaces lodash methods with native JavaScript equivalents. The implementation is clean and maintains the original functionality while reducing external dependencies.
A few suggestions to make the code even more maintainable:
- const investigationList = Array.from( - data - .flatMap((i) => i?.data?.results || []) - .map((i) => ({ - ...i, - name: `${i.name} ${i.groups[0].name ? " | " + i.groups[0].name : ""}`, - })) - .reduce((map, item) => map.set(item.external_id, item), new Map()) - .values(), - ); + // Extract results and handle potential null/undefined data + const flattenedResults = data.flatMap((i) => i?.data?.results || []); + + // Create a Map for unique entries by external_id + const uniqueInvestigations = new Map( + flattenedResults.map((i) => [ + i.external_id, + { + ...i, + name: `${i.name}${i.groups[0]?.name ? ` | ${i.groups[0].name}` : ''}`, + }, + ]) + ); + + const investigationList = Array.from(uniqueInvestigations.values());Changes suggested:
- Split the chain into meaningful steps for better readability
- Use optional chaining for
groups[0]?.nameto prevent potential null reference- Use template literals more consistently
- Add comments to explain the data transformation steps
src/components/Patient/SampleDetails.tsx (1)
Line range hint
385-385: Add null checks for consistency with other string transformations.Similar to the changes in
renderFlow, consider adding null checks to other instances of string transformations to maintain consistency and prevent potential runtime errors:- {startCase(camelCase(sampleDetails.doctor_name))} + {startCase(camelCase(sampleDetails.doctor_name || ""))} - {startCase(camelCase(sampleDetails.sample_type))} + {startCase(camelCase(sampleDetails.sample_type || ""))}Also applies to: 461-461
src/components/Patient/PatientRegister.tsx (1)
762-762: MemoizeduplicateCheckto prevent unnecessary recreations.To avoid recreating the debounced function on every render, wrap
duplicateCheckwithuseCallback. This ensures thatduplicateCheckmaintains referential equality between renders unless its dependencies change.Apply this diff to memoize
duplicateCheck:- const duplicateCheck = useDebounce(async (phoneNo: string) => { + const duplicateCheck = useCallback( + useDebounce(async (phoneNo: string) => { // function body }, 300), + [] + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
package.json(0 hunks)src/Utils/Notifications.js(1 hunks)src/Utils/stringUtils.ts(1 hunks)src/components/Common/ExcelFIleDragAndDrop.tsx(1 hunks)src/components/Facility/Investigations/Reports/index.tsx(1 hunks)src/components/Facility/Investigations/Reports/utils.tsx(1 hunks)src/components/Facility/Investigations/ShowInvestigation.tsx(2 hunks)src/components/Facility/Investigations/Table.tsx(1 hunks)src/components/Form/AutoCompleteAsync.tsx(2 hunks)src/components/Form/Form.tsx(1 hunks)src/components/Patient/DiagnosesFilter.tsx(4 hunks)src/components/Patient/PatientRegister.tsx(3 hunks)src/components/Patient/SampleDetails.tsx(2 hunks)src/components/Patient/SampleTestCard.tsx(3 hunks)
💤 Files with no reviewable changes (1)
- package.json
🧰 Additional context used
🔇 Additional comments (16)
src/Utils/stringUtils.ts (1)
1-9:
Critical issues in camelCase implementation
The current implementation has several issues that need to be addressed:
- Missing input validation for null/undefined
- The final
.toLowerCase()transform contradicts camelCase format by lowercasing all characters - Implementation differs from lodash's behavior for handling special characters and multiple spaces
Consider this improved implementation:
-export const camelCase = (str: string) => {
+export const camelCase = (str: string): string => {
+ if (!str) return '';
+
return str
- .replace(/(?:^\w|[A-Z]|\b\w|\s+)/g, (match, index) =>
- index === 0 ? match.toLowerCase() : match.toUpperCase(),
- )
- .replace(/\s+/g, "")
- .replace(/([A-Z])/g, (match) => match.toLowerCase());
+ .toLowerCase()
+ .replace(/[^a-zA-Z0-9]+(.)/g, (_, char) => char.toUpperCase());
};Let's verify the behavior matches lodash:
✅ Verification successful
Based on the gathered evidence, I can now provide a final response:
Simplify camelCase implementation while maintaining compatibility
The current implementation can be improved, but the suggested changes in the review need revision. The function is being used in combination with startCase for formatting display text from snake_case API responses (e.g., sample_type → Sample Type). The final .toLowerCase() transform isn't actually an issue since it's always used with startCase.
A simpler implementation that handles the current usage patterns:
export const camelCase = (str: string): string => {
if (!str) return '';
return str
.toLowerCase()
.replace(/[^a-zA-Z0-9]+(.)/g, (_, char) => char.toUpperCase());
};This maintains compatibility with existing usage while being more concise and handling edge cases better.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find test files that might contain lodash camelCase usage examples
rg -l "\.camelCase\(" | grep -i "test\|spec"
Length of output: 46
Script:
#!/bin/bash
# Let's search for any tests or usage of camelCase function
rg -l "camelCase"
# Also search for any lodash imports to understand the context of migration
rg "from 'lodash'" -A 1
Length of output: 264
Script:
#!/bin/bash
# Let's check the actual usage of camelCase in the files to understand the context
rg "camelCase" -B 2 -A 2 scripts/setup-care-apps.js src/components/Patient/SampleTestCard.tsx src/components/Patient/SampleDetails.tsx src/Utils/Notifications.js
# Also check if there are any lodash imports with different syntax
rg "lodash" -B 1 -A 1
Length of output: 48534
Script:
#!/bin/bash
# Let's check how camelCase is actually being used in the codebase
rg -B 2 -A 2 "camelCase\(" src/
# Also check if there are any existing tests for stringUtils
fd "stringUtils" -e test.ts -e spec.ts
Length of output: 2369
src/Utils/Notifications.js (1)
4-4: Verify stringUtils implementation completeness
The import change aligns with the PR objective of removing lodash dependencies. However, we should verify that the local implementation in stringUtils matches the lodash behavior exactly.
✅ Verification successful
Let me verify the behavior of these implementations against lodash's behavior by checking how they are used in the Notifications.js file.
Implementation is suitable for the use case
The local stringUtils implementation is appropriate for its usage in Notifications.js, where it's used to format error messages by:
- Converting keys to camelCase and then to startCase (e.g., "error_message" → "errorMessage" → "Error Message")
- The implementation matches the required behavior for this specific use case
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the stringUtils implementation exists and contains required functions
# Check if stringUtils file exists in the same directory
fd -e js -e ts "stringUtils" "src/Utils"
# Check implementation of required functions
rg -A 10 "export.*(camelCase|startCase)" "src/Utils"
Length of output: 1298
Script:
#!/bin/bash
# Check how these functions are used in Notifications.js
rg -A 5 "(camelCase|startCase)" "src/Utils/Notifications.js"
# Also check if there are any tests for these string utilities
fd -e test.js -e test.ts -e spec.js -e spec.ts "stringUtils" .
Length of output: 557
src/components/Patient/DiagnosesFilter.tsx (1)
1-1: Verify complete removal of lodash dependencies.
Let's ensure there are no remaining lodash imports or dependencies in the codebase.
✅ Verification successful
Lodash dependency has been successfully removed ✅
The verification shows no traces of lodash in the codebase:
- No lodash imports found in TypeScript/JavaScript files
- No lodash requires or usage patterns detected
- No lodash dependencies in package.json
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining lodash imports and dependencies
# Check for lodash imports in TypeScript/JavaScript files
echo "Checking for lodash imports..."
rg -l "import.*lodash" -t ts -t tsx -t js
# Check package.json for lodash dependencies
echo "Checking package.json for lodash dependencies..."
if [ -f "package.json" ]; then
jq '.dependencies + .devDependencies | with_entries(select(.key | contains("lodash")))' package.json
fi
Length of output: 381
Script:
#!/bin/bash
# Description: Check for any remaining lodash imports with correct file type flags
echo "Checking for lodash imports..."
rg -l "import.*lodash" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type-add 'js:*.js' -t tsx -t ts -t js
# Check if lodash is used without import statements (e.g., require)
echo "Checking for lodash requires..."
rg -l "require.*lodash" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type-add 'js:*.js' -t tsx -t ts -t js
# Check for any remaining lodash usage patterns
echo "Checking for lodash usage patterns..."
rg -l "lodash" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' --type-add 'js:*.js' -t tsx -t ts -t js
Length of output: 546
src/components/Facility/Investigations/Reports/utils.tsx (1)
1-1: Successfully achieved lodash removal objective
The changes effectively replace lodash functions with native JavaScript implementations while maintaining functionality. The code is now more maintainable as it reduces external dependencies.
Regarding your question about testing the frontend changes: To help identify the specific route, I can analyze the codebase.
✅ Verification successful
Investigation Reports can be tested at /patient/:id/investigation_reports
The component is accessible through the patient routes at /patient/:id/investigation_reports where :id should be replaced with a valid patient ID. The component receives the patient ID as a prop.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for routes that import or use the Investigation Reports component
rg -l "Investigations/Reports" | grep -i "route"
Length of output: 83
Script:
#!/bin/bash
# Check the route configuration in PatientRoutes.tsx
rg -A 10 "Investigations/Reports" src/Routers/routes/PatientRoutes.tsx
Length of output: 645
Script:
#!/bin/bash
# Check the route path configuration for InvestigationReports
rg -B 5 -A 5 "InvestigationReports" src/Routers/routes/PatientRoutes.tsx
Length of output: 959
src/components/Form/Form.tsx (1)
60-66: Verify test coverage for the new error filtering logic.
Since this is a critical form validation logic, we should ensure proper test coverage exists.
src/components/Facility/Investigations/Table.tsx (1)
Line range hint 1-1: Verify complete removal of lodash dependencies
Let's ensure there are no remaining lodash imports or usages in this file or related components.
✅ Verification successful
Let me verify the lodash removal more thoroughly by checking the entire codebase and the specific file.
Let me try one more verification with a simpler pattern to ensure we catch any lodash usage:
Lodash dependency removal verified successfully
The verification shows that lodash has been completely removed from the Table.tsx component. The only remaining references to lodash are in package metadata files (package-lock.json) and software bill of materials files (bom.json, fe-sbom.json), which is expected and doesn't indicate actual usage in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining lodash imports in this file and related components
echo "Checking for lodash imports..."
rg -l "lodash" "src/components/Facility/Investigations"
echo "Checking package.json for lodash dependencies..."
cat package.json | jq '.dependencies | keys[] | select(contains("lodash"))'
Length of output: 309
Script:
#!/bin/bash
# Check the specific file for any lodash usage
echo "Checking Table.tsx for lodash..."
rg "lodash" "src/components/Facility/Investigations/Table.tsx"
# Check if lodash is used in imports or as _ anywhere in the file
echo -e "\nChecking for potential lodash usage as _..."
rg "import.*{.*_.*}" "src/components/Facility/Investigations/Table.tsx"
rg "import.*_ from" "src/components/Facility/Investigations/Table.tsx"
# Check if package.json exists and has lodash
echo -e "\nChecking if package.json exists and contains lodash..."
if [ -f "package.json" ]; then
echo "Content of dependencies in package.json:"
cat package.json | jq '.dependencies'
fi
# Check the entire codebase for lodash imports
echo -e "\nChecking entire codebase for lodash imports..."
rg "from ['\"]lodash['\"]|from ['\"]lodash/|require\(['\"]lodash['\"]" -l
Length of output: 2423
Script:
#!/bin/bash
# Check for any lodash-related strings in the specific file
echo "Checking Table.tsx for any lodash references..."
rg -i "lodash" "src/components/Facility/Investigations/Table.tsx"
# Check for potential lodash usage patterns in the file
echo -e "\nChecking for potential lodash usage patterns..."
rg "import.*_" "src/components/Facility/Investigations/Table.tsx"
rg "from.*lodash" "src/components/Facility/Investigations/Table.tsx"
# Simple check for lodash in the entire codebase
echo -e "\nChecking codebase for lodash..."
rg -l "lodash"
Length of output: 579
src/components/Form/AutoCompleteAsync.tsx (1)
74-77: Verify loading state behavior during rapid typing
The loading state is set immediately but only cleared after the debounced operation completes. This could lead to a flickering loading indicator during rapid typing.
Let's verify the loading behavior in similar components:
✅ Verification successful
Loading state implementation is correct and follows best practices
After examining the implementation, the loading state behavior in AutoCompleteAsync is properly implemented:
- The loading indicator is managed within a debounced operation where:
setLoading(true)is called immediately before the debounce timeoutsetLoading(false)is called after the data fetch completes- The timeout is properly cleared on each new query using
clearTimeout
- This pattern prevents unnecessary API calls during rapid typing while providing immediate visual feedback
- The loading state is appropriately displayed as a spinner icon in the UI, replacing the dropdown arrow
- The implementation is consistent with other components in the codebase (e.g., SearchInput) that use similar debounce patterns
The current implementation actually helps prevent flickering by debouncing the actual data fetching operation while providing immediate visual feedback to the user.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other async components with loading states
rg -l "setLoading.*true.*setTimeout" --type tsx
Length of output: 77
Script:
#!/bin/bash
# Search for loading state patterns in TypeScript/React files with broader file extensions
rg -l "setLoading\(true\)" --type ts --type tsx --type jsx
# Look for debounce patterns with loading states
rg "setLoading.*setTimeout|setTimeout.*setLoading" --type ts --type tsx --type jsx -A 5
# Search for similar autocomplete components
rg "AutoComplete.*async" --type ts --type tsx --type jsx -A 5
Length of output: 298
Script:
#!/bin/bash
# Search for loading state patterns in all files
rg -l "setLoading\(true\)"
# Look for debounce patterns with loading states
rg "setLoading.*setTimeout|setTimeout.*setLoading" -A 5
# Search for the current component implementation
rg "class.*AutoCompleteAsync|function.*AutoCompleteAsync" -A 10
# Find any debounce implementations
rg "clearTimeout.*timeoutId" -A 5
Length of output: 1655
Script:
#!/bin/bash
# Let's examine the complete implementation of AutoCompleteAsync to understand the loading state management
cat src/components/Form/AutoCompleteAsync.tsx
# Also check SearchInput implementation for comparison of debounce pattern
cat src/components/Form/SearchInput.tsx
Length of output: 10365
src/components/Patient/SampleTestCard.tsx (2)
Line range hint 1-213: Overall implementation looks good
The changes successfully remove the lodash dependency while maintaining the component's functionality. The implementation includes:
- Proper TypeScript types and interfaces
- Responsive UI design
- Robust error handling
- Clear status management
15-15: Verify the local stringUtils implementation
The import change aligns with the PR objective to remove lodash dependencies. However, we should verify the implementation of these utility functions.
✅ Verification successful
Local stringUtils implementation is adequate and correctly used
The implementation of camelCase and startCase in stringUtils.ts is appropriate for the use case:
- Both functions are properly implemented with string manipulation
- They are used correctly in SampleTestCard.tsx for formatting status and result fields
- The usage includes null checks with the
||operator, preventing potential issues with undefined values
While there are no dedicated test files found, the implementation is straightforward and follows standard string manipulation patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of camelCase and startCase in stringUtils
# Check if the utility functions exist and their implementation
fd --type f stringUtils.ts --exec cat {} \;
# Look for tests of these utilities
fd --type f "stringUtils.test" --exec cat {} \;
Length of output: 882
Script:
#!/bin/bash
# Let's verify if these utility functions are being used correctly in the component
rg -A 2 "camelCase|startCase" "src/components/Patient/SampleTestCard.tsx"
# Also check if there are any tests for stringUtils in other test file formats
fd --type f "stringUtils" --extension test.ts --extension test.tsx --extension spec.ts --extension spec.tsx --exec cat {} \;
Length of output: 508
src/components/Common/ExcelFIleDragAndDrop.tsx (1)
70-72: LGTM! Successful replacement of lodash dependency.
The replacement of lodash-es's forIn with native Object.keys().forEach is a good change that:
- Removes external dependency while maintaining the same functionality
- Uses modern JavaScript features that are well-supported
- Potentially improves bundle size by removing lodash
Let's verify there are no remaining lodash imports or usages in this file:
✅ Verification successful
Confirmed: No lodash dependencies remain in the file
After reviewing the complete file content, I can verify that:
- There are no lodash imports present
- No lodash methods (including
_.forIn) are being used - The native
Object.keys().forEachimplementation is correctly handling the date conversion
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining lodash imports or usage
rg "lodash|_\." "src/components/Common/ExcelFIleDragAndDrop.tsx"
Length of output: 64
Script:
#!/bin/bash
# Let's verify the file content to ensure no lodash dependencies remain
cat "src/components/Common/ExcelFIleDragAndDrop.tsx"
Length of output: 9536
src/components/Facility/Investigations/Reports/index.tsx (1)
181-190: Verify data structure assumptions
Please ensure that the data structure assumptions remain valid after removing lodash:
dataarray contains valid response objectsresultsproperty exists in the response- Each investigation object has required properties (
external_id,name,groups)
✅ Verification successful
Based on the shell script results, I can now verify the data structure assumptions. Let me analyze the code in src/components/Facility/Investigations/Reports/index.tsx and related files:
-
The
Investigationinterface insrc/components/Facility/Investigations/Reports/types.tsconfirms that the required properties exist:external_idis present ininvestigation_objectnameis present ininvestigation_objectgroupsarray is defined inInvestigationType
-
The data transformation in the code:
const investigationList = Array.from( data .flatMap((i) => i?.data?.results || []) .map((i) => ({ ...i, name: `${i.name} ${i.groups[0].name ? " | " + i.groups[0].name : ""}`, })) .reduce((map, item) => map.set(item.external_id, item), new Map()) .values(), );
- Safely handles undefined data with
i?.data?.results || [] - Properly accesses
groups[0].namewith optional chaining - Uses Map to deduplicate by
external_id
- Safely handles undefined data with
Data structure assumptions are valid ✓
The code handles all edge cases appropriately and the type definitions confirm the required properties are present. The removal of lodash has not impacted the data structure integrity.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the structure of investigation data across the codebase
# Check for investigation type definitions
echo "Checking investigation type definitions..."
rg -t typescript "type.*Investigation.*=.*{" -A 10
# Check for investigation object usage
echo "Checking investigation object usage..."
rg -t typescript "investigation.*\.external_id|investigation.*\.name|investigation.*\.groups"
Length of output: 479
Script:
#!/bin/bash
# Search for type definitions related to investigations
echo "Searching for investigation type definitions..."
rg "type\s+Investigation" -A 10
# Search for interfaces related to investigations
echo "Searching for investigation interfaces..."
rg "interface\s+Investigation" -A 10
# Check usage of investigation properties
echo "Checking investigation property usage..."
rg "\.external_id|\.name|\.groups\[0\]"
# Look for investigation data transformations
echo "Checking data transformations..."
rg "flatMap.*results|\.reduce.*Map.*external_id"
Length of output: 73526
src/components/Patient/SampleDetails.tsx (3)
17-17: LGTM! Import change aligns with PR objectives.
Successfully migrated from lodash-es to local string utilities.
273-273: LGTM! Improved null-safety with defensive checks.
The addition of null checks (|| "") for both flow.status and flow.notes prevents potential runtime errors when these values are undefined or null.
Also applies to: 277-277
Line range hint 1-500: Verify complete removal of lodash dependencies.
Let's verify that all lodash imports have been properly replaced in this file and related components.
✅ Verification successful
Migration from lodash to local utilities is complete and correct
The verification confirms:
- No lodash imports remain in SampleDetails.tsx
- Local string utility functions (camelCase, startCase, capitalize) are properly implemented in src/Utils/stringUtils.ts
- The usage pattern is consistent across Patient components, with multiple files using the same utility functions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining lodash imports in the codebase
# and verify the consistency of string utility usage
# Check for any remaining lodash imports in this file
echo "Checking for remaining lodash imports in SampleDetails.tsx..."
rg "from ['\"]lodash" "src/components/Patient/SampleDetails.tsx"
# Check for similar string utility usage patterns in related files
echo "Checking for similar string utility usage patterns..."
rg "startCase\(camelCase\(" "src/components/Patient/"
# Verify the implementation of local string utilities
echo "Verifying local string utility implementations..."
cat "src/Utils/stringUtils.ts"
Length of output: 1802
src/components/Patient/PatientRegister.tsx (2)
77-77: Import statement of startCase is correct.
The startCase function is correctly imported from @/Utils/stringUtils, replacing the lodash-es dependency.
629-629: Proper usage of startCase on patient name.
The code correctly formats the patient's name by converting it to lowercase and then applying startCase to capitalize it appropriately.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
src/components/Facility/Investigations/Reports/utils.tsx (1)
Line range hint
25-96: Improve type safety and code organization in data transformation.While the implementation is efficient, there's heavy usage of
anytypes which reduces type safety. Consider breaking down the complex transformation into smaller, well-typed functions.Here's a suggested approach:
interface SessionObject { session_external_id: string; session_created_date: string; facility_name?: string; facility_id?: string; [key: string]: any; } interface InvestigationValue { session_object: SessionObject; consultation_object?: { facility_name?: string; facility?: string; }; investigation_object: { external_id: string; min_value?: number; max_value?: number; }; value?: number; notes?: string; } const createSessionMap = (data: InvestigationValue[]) => { return Array.from( new Map( data.map((value) => [ value.session_object.session_external_id, { ...value.session_object, facility_name: value.consultation_object?.facility_name, facility_id: value.consultation_object?.facility, }, ]), ).values(), ).sort( (a, b) => new Date(b.session_created_date).getTime() - new Date(a.session_created_date).getTime(), ); };src/components/Facility/Investigations/Table.tsx (1)
62-80: Consider type safety and documentation improvementsWhile the implementation successfully replaces lodash.set, consider these enhancements:
- Replace
value: anywith a more specific type- Add JSDoc documentation describing the function's purpose and parameters
- Consider extracting this to a reusable utility function since other components might need similar lodash.set replacements
+/** + * Updates a nested object property using dot notation + * @param value - The value to set + * @param name - The property path in dot notation (e.g., "a.b.c") + */ -const handleValueChange = (value: any, name: string) => { +const handleValueChange = (value: string | number, name: string) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
src/Utils/stringUtils.ts(1 hunks)src/components/Facility/Investigations/Reports/utils.tsx(2 hunks)src/components/Facility/Investigations/ShowInvestigation.tsx(2 hunks)src/components/Facility/Investigations/Table.tsx(1 hunks)src/components/Patient/PatientRegister.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Facility/Investigations/ShowInvestigation.tsx
🧰 Additional context used
🔇 Additional comments (8)
src/Utils/stringUtils.ts (2)
11-22: LGTM! Well-implemented startCase function
The implementation is robust and handles all edge cases correctly:
- Empty string validation
- Multiple space normalization
- Proper word capitalization
- Empty word filtering
Note: This implementation already addresses the concerns raised in the previous review comment.
1-27: Verify lodash replacement completeness
Let's ensure all lodash string utility imports are properly replaced with these new implementations.
✅ Verification successful
Let me verify the usage of these functions in the actual files to ensure we haven't missed any replacements.
Lodash string utilities successfully replaced
The verification shows that:
- All identified files have been updated to use the new string utility functions from
@/Utils/stringUtils - No remaining lodash imports for these string utilities exist
- The functions are being actively used in the components with the same patterns as before
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining lodash string utility imports and compare behavior
# Check for any remaining lodash string utility imports
echo "Checking for remaining lodash string utility imports..."
rg -l "import.*\{.*(camelCase|startCase|capitalize).*\}.*from.*lodash"
# Find all files that previously used these lodash functions
echo "Files previously using these lodash functions..."
git log -p | rg -B 2 "import.*\{.*(camelCase|startCase|capitalize).*\}.*from.*lodash"
Length of output: 1436
Script:
#!/bin/bash
# Check current usage in the identified files
echo "Checking SampleDetails.tsx..."
rg -A 2 "(camelCase|startCase|capitalize)" "src/components/Patient/SampleDetails.tsx" || echo "File not found"
echo -e "\nChecking SampleTestCard.tsx..."
rg -A 2 "(camelCase|startCase|capitalize)" "src/components/Patient/SampleTestCard.tsx" || echo "File not found"
# Also check for any current imports of these functions from lodash
echo -e "\nChecking current lodash imports in these files..."
rg "import.*lodash" "src/components/Patient/SampleDetails.tsx" "src/components/Patient/SampleTestCard.tsx" || echo "No lodash imports found"
# Check if these files are using our new utility functions
echo -e "\nChecking for imports from stringUtils..."
rg "import.*stringUtils" "src/components/Patient/SampleDetails.tsx" "src/components/Patient/SampleTestCard.tsx" || echo "No stringUtils imports found"
Length of output: 2738
src/components/Facility/Investigations/Reports/utils.tsx (2)
55-62: LGTM! Efficient session lookup implementation.
The implementation follows the previous review suggestion of using a Map for O(1) lookup performance.
Line range hint 98-99: LGTM! Appropriate use of memoization.
The getColorIndex function is now properly memoized while maintaining its original logic.
src/components/Facility/Investigations/Table.tsx (1)
64-72: Security validation looks good!
The implementation correctly prevents prototype pollution by validating object keys. This addresses the security concern raised in the previous review.
src/components/Patient/PatientRegister.tsx (3)
77-77: LGTM: Import change aligns with PR objectives.
The change to import startCase from local utilities instead of lodash aligns with the PR's goal of removing lodash dependencies.
188-204: LGTM: useDebounce implementation follows best practices.
The implementation correctly incorporates the suggested improvements from previous reviews.
647-647: LGTM: Improved name case handling.
Converting the name to lowercase before applying startCase ensures consistent capitalization regardless of input format.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/Utils/stringUtils.ts (3)
1-1: Enhance function documentation with examplesConsider adding examples to make the function's behavior more clear:
-// Converts a string to camelCase format, first word - lowercase and each subsequent word - uppercase letter, with no spaces. +// Converts a string to camelCase format (e.g., "hello world" → "helloWorld", "foo-bar" → "fooBar", "FOO_BAR" → "fooBar")
19-19: Optimize word capitalization logicThe current word capitalization can be optimized by avoiding array access and using charAt:
- .map((word) => (word ? word[0].toUpperCase() + word.slice(1) : "")) + .map((word) => (word ? word.charAt(0).toUpperCase() + word.slice(1) : ""))
23-28: LGTM! Consider adding an example to the documentationThe implementation is robust and efficient. As a minor enhancement, consider adding an example to the documentation:
-// Capitalize the first letter of a string +// Capitalize the first letter of a string (e.g., "hello" → "Hello")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/Utils/stringUtils.ts(1 hunks)src/components/Facility/Investigations/Reports/utils.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Facility/Investigations/Reports/utils.tsx
🔇 Additional comments (1)
src/Utils/stringUtils.ts (1)
1-28: Verify compatibility with lodash implementations
Let's verify that these utilities handle all edge cases similarly to the lodash implementations they're replacing:
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
src/Utils/Notifications.js(1 hunks)src/Utils/utils.ts(2 hunks)src/components/Patient/DiagnosesFilter.tsx(4 hunks)src/components/Patient/PatientRegister.tsx(3 hunks)src/components/Patient/SampleDetails.tsx(4 hunks)src/components/Patient/SampleTestCard.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/Utils/Notifications.js
- src/components/Patient/DiagnosesFilter.tsx
- src/components/Patient/SampleDetails.tsx
- src/components/Patient/SampleTestCard.tsx
🔇 Additional comments (3)
src/components/Patient/PatientRegister.tsx (3)
77-77: LGTM! Import changes align with PR objectives.
The changes correctly import startCase and useDebounce from local utility files, successfully removing lodash dependencies.
Also applies to: 85-85
630-630: LGTM! Name formatting maintains consistent behavior.
The change correctly uses the local startCase utility while maintaining proper name formatting by converting to lowercase first.
Line range hint 750-773: LGTM! Debounce implementation follows best practices.
The code now correctly uses the imported useDebounce hook instead of defining it inside the component, which addresses the previous review comments about hook redefinition. The duplicate check functionality is properly maintained with appropriate debouncing.
| export const useDebounce = ( | ||
| callback: (...args: string[]) => void, | ||
| delay: number, | ||
| ) => { | ||
| const callbackRef = useRef(callback); | ||
| useEffect(() => { | ||
| callbackRef.current = callback; | ||
| }, [callback]); | ||
|
|
||
| const timeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null); | ||
| const debouncedCallback = (...args: string[]) => { | ||
| if (timeoutRef.current) { | ||
| clearTimeout(timeoutRef.current); | ||
| } | ||
| timeoutRef.current = setTimeout(() => { | ||
| callbackRef.current(...args); | ||
| }, delay); | ||
| }; | ||
| return debouncedCallback; | ||
| }; |
There was a problem hiding this comment.
this is a hook so move it to src/hooks
| @@ -1,6 +1,7 @@ | |||
| import { Stack, alert, defaultModules } from "@pnotify/core"; | |||
| import * as PNotifyMobile from "@pnotify/mobile"; | |||
| import { camelCase, startCase } from "lodash-es"; | |||
|
|
|||
| import { camelCase, startCase } from "./utils"; | |||
| let current = changedFields; | ||
| for (let i = 0; i < keys.length - 1; i++) { | ||
| const key = keys[i]; | ||
|
|
||
| // Protect against prototype pollution by skipping unsafe keys - crai | ||
| if (key === "__proto__" || key === "constructor" || key === "prototype") { | ||
| continue; | ||
| } | ||
|
|
||
| // Use Object.create(null) to prevent accidental inheritance from Object prototype - coderabbit | ||
| current[key] = current[key] || Object.create(null); | ||
| current = current[key]; | ||
| } | ||
|
|
||
| const lastKey = keys[keys.length - 1]; | ||
|
|
||
| // Final key assignment, ensuring no prototype pollution vulnerability - coderabbit | ||
| if ( | ||
| lastKey !== "__proto__" && | ||
| lastKey !== "constructor" && | ||
| lastKey !== "prototype" | ||
| ) { | ||
| current[lastKey] = value; | ||
| } |
There was a problem hiding this comment.
extract this function to utils and reuse for ./Table component as well
There was a problem hiding this comment.
the issue with using the css transform capitalize it is just changes the visual representation of the text, but if the user tries to copy it it will be copied in its orignal form, instead of this we should make a function to convert text directly to title case and use that
cc: @rithviknishad
|
👋 Hi, @SwanandBhuskute, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Proposed Changes
i was unable to check this code on the frontend , coz i am not getting on which route the output displays
kindly help
this was generated after build
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
useDebouncehook for improved debouncing in various components.startCaseandcamelCasefor string manipulation.Enhancements
PatientRegisterandShowInvestigation.ExcelFileDragAndDropcomponent for better error reporting during file selection.Bug Fixes
These updates enhance performance, maintainability, and user experience across the application.