noUncheckedIndexedAccess in tsconfig #9880
Conversation
WalkthroughThis pull request focuses on enhancing code robustness across multiple components and files by implementing optional chaining, nullish coalescing, and adding safeguards against undefined values. The changes span various parts of the application, including routing, component logic, hooks, and utility functions. The primary goal is to improve error handling and prevent potential runtime errors by ensuring that code can gracefully handle scenarios where data might be undefined or null. Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (3)
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: 9
🧹 Nitpick comments (16)
src/Routers/routes/PatientRoutes.tsx (1)
13-13: Consider using nullish coalescing (??) instead of logical OR (||)While adding default values for route parameters is correct, using the logical OR operator (
||) will convert any falsy value (including empty string) to the default. Consider using the nullish coalescing operator (??) which only defaults fornullorundefined.Example fix:
- facilityId={facilityId || ""} + facilityId={facilityId ?? ""}Also applies to: 19-19, 21-21, 23-23, 26-26, 33-33, 38-38, 45-46
src/service-worker.ts (1)
38-38: Consider adding error handling for postMessageWhile the optional chaining operator correctly handles undefined clients, consider adding error handling for the
postMessageoperation itself.- clients[0]?.postMessage(data); + try { + clients[0]?.postMessage(data); + } catch (error) { + console.error('Failed to post message to client:', error); + }src/components/Common/Avatar.tsx (1)
33-33: The default color might be unnecessaryThe modulo operation
stringToInt(name) % colors.lengthensures the index is always within bounds of the non-emptycolorsarray. The default color would only be used if thecolorsarray is empty. Consider adding an assertion or check for non-empty colors array instead.- const backgroundColor = colors[index] || "#FFFFFF"; // Default to white if undefined + if (colors.length === 0) { + throw new Error("Colors array must not be empty"); + } + const backgroundColor = colors[index];src/Routers/routes/ConsultationRoutes.tsx (1)
16-17: Consider using nullish coalescing for route parameters.While using
||works, the nullish coalescing operator (??) would be more appropriate here as it only replacesnullorundefinedvalues, not all falsy values like empty strings.Example refactor:
- facilityId={facilityId || ""} + facilityId={facilityId ?? ""}Also applies to: 26-27, 36-37, 44-47, 55-56, 63-65, 71-74, 80-81, 87-89, 98-99
src/components/Common/AudioPlayer.tsx (1)
94-99: Simplify the conditional checks.The current implementation has duplicate conditions checking the same value. This can be simplified while maintaining type safety.
- if (value[0] !== undefined) { - audioRef.current.currentTime = value[0]; - } - if (value[0] !== undefined) { - setCurrentTime(value[0]); - } + const time = value[0]; + if (time !== undefined) { + audioRef.current.currentTime = time; + setCurrentTime(time); + }src/components/Common/Export.tsx (1)
65-73: Remove unnecessary non-null assertion and improve code organization.The non-null assertion (
!) onitem.routeis redundant since we've already checked its existence. Also, consider extracting the action logic for better readability.- let action = item?.action; - if (item?.route) { - action = async () => { - const { data } = await request(item.route!); - return data ?? null; - }; - } - if (action) { - exportFile(action, item?.filePrefix, item?.type, item?.parse); - } + const getDataFromRoute = async (route: Route<string | { results: object[] }, unknown>) => { + const { data } = await request(route); + return data ?? null; + }; + + const exportAction = item?.route + ? () => getDataFromRoute(item.route) + : item?.action; + + if (exportAction) { + exportFile(exportAction, item?.filePrefix, item?.type, item?.parse); + }src/common/schemaParser.ts (2)
94-97: Improve type safety and make the fallback more explicit.The empty object fallback could be more explicit using nullish coalescing, and type safety could be improved.
- const { value, error } = - validateAndParse(key, item[key], schema[key] as SingleKeySchema)[key] || - {}; + const result = validateAndParse(key, item[key], schema[key] as SingleKeySchema)[key] ?? { + value: undefined, + error: undefined, + }; + const { value, error } = result;
186-189: Remove unnecessary optional chaining and improve type safety.The optional chaining on
Object.values()is unnecessary as it always returns an array. Consider adding type guards instead.- return ( - dataWithErrors[index] && - !Object.values(dataWithErrors[index])?.some((item) => item.error) - ); + const record = dataWithErrors[index]; + return record ? !Object.values(record).some((item) => item.error) : false;src/Utils/AutoSave.tsx (2)
184-186: Simplify the draft selection logic.The draft selection logic can be simplified by using array indexing.
- (draftStarted - ? drafts[0]?.draft - : drafts[drafts.length - 1]?.draft) || {}, + drafts[draftStarted ? 0 : drafts.length - 1]?.draft ?? {},
196-197: Simplify the timestamp selection logic.The timestamp selection logic can be simplified similar to the draft selection.
- ? drafts[0]?.timestamp - : drafts[drafts.length - 1]?.timestamp, + drafts[draftStarted ? 0 : drafts.length - 1]?.timestamp,src/components/Common/Charts/ObservationChart.tsx (1)
354-357: LGTM! Good use of optional chaining.The addition of optional chaining operators for accessing
details.enteredByimproves the code's robustness by safely handling cases whereobservations[0]might be undefined.Consider extracting the first observation's details into a constant for better readability:
+ const firstObservation = observations[0]?.details; <Avatar - name={observations[0]?.details.enteredBy} + name={firstObservation?.enteredBy} className="h-6 w-6" /> - <span>{observations[0]?.details.enteredBy}</span> + <span>{firstObservation?.enteredBy}</span>src/components/Questionnaire/QuestionnaireForm.tsx (1)
150-156: LGTM! Improved error handling for form updates.The changes enhance the robustness of error handling by safely accessing
form?.errorsand conditionally updating the form array.Consider using nullish coalescing to provide a default empty array for errors:
- form?.errors.push({ + (form?.errors ?? []).push({ question_id: error.question_id, error: error.error ?? error.msg, } as QuestionValidationError);src/Utils/utils.ts (1)
313-316: LGTM! Safe country code handling.Good use of optional chaining and nullish coalescing for handling potentially undefined country codes and their properties.
Consider extracting the country code access into a helper function to reduce repetition:
const getCountryCodeFromPhoneCodes = (code: string) => phoneCodes[code as string]?.code.replaceAll("-", "") ?? "";tsconfig.json (1)
25-26: LGTM! Important type safety enhancement.The addition of
noUncheckedIndexedAccessis a valuable improvement that will help catch potential runtime errors by making array and object property access stricter.This change will require careful handling of array indices and object properties throughout the codebase, as TypeScript will now treat them as potentially undefined. This aligns well with defensive programming practices.
src/pages/Encounters/EncounterShow.tsx (1)
169-169: Consider logging invalid tab selectionsWhile the fallback to
EncounterUpdatesTabprevents runtime errors, it might be helpful to log when invalid tabs are selected to track potential routing or navigation issues.Consider adding error logging:
- const SelectedTab = tabs[props.tab] || EncounterUpdatesTab; + const SelectedTab = tabs[props.tab] || (() => { + console.warn(`Invalid tab selected: ${props.tab}`); + return EncounterUpdatesTab; + })();src/hooks/useFileManager.tsx (1)
84-86: LGTM! Robust handling of optional array access.The changes properly handle potential undefined values when accessing array indices, which aligns with the
noUncheckedIndexedAccessTypeScript option. The implementation includes:
- Fallback to empty array if split fails
- Optional chaining for array access
- Nullish coalescing to provide empty string default
However, consider extracting the extension logic into a reusable utility function since URL parsing is a common operation.
+// In src/Utils/files.ts +export const getFileExtension = (url: string): string => { + const div1 = url.split("?")?.[0]?.split(".") ?? []; + return div1.length > 0 ? (div1[div1.length - 1]?.toLowerCase() ?? "") : ""; +}; -const getExtension = (url: string) => { - const div1 = url.split("?")[0]?.split(".") || []; - const ext: string = - div1.length > 0 ? (div1[div1.length - 1]?.toLowerCase() ?? "") : ""; - return ext; -}; +const getExtension = getFileExtension;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
src/CAREUI/interactive/HumanChart.tsx(1 hunks)src/Providers/PatientUserProvider.tsx(1 hunks)src/Routers/AppRouter.tsx(1 hunks)src/Routers/routes/ConsultationRoutes.tsx(3 hunks)src/Routers/routes/FacilityRoutes.tsx(1 hunks)src/Routers/routes/OrganizationRoutes.tsx(1 hunks)src/Routers/routes/PatientRoutes.tsx(1 hunks)src/Routers/routes/ResourceRoutes.tsx(1 hunks)src/Routers/routes/UserRoutes.tsx(1 hunks)src/Routers/routes/questionnaireRoutes.tsx(1 hunks)src/Utils/AutoSave.tsx(3 hunks)src/Utils/utils.ts(2 hunks)src/common/schemaParser.ts(2 hunks)src/common/utils.tsx(1 hunks)src/components/Common/AudioPlayer.tsx(1 hunks)src/components/Common/Avatar.tsx(1 hunks)src/components/Common/Charts/ObservationChart.tsx(1 hunks)src/components/Common/DateTextInput.tsx(1 hunks)src/components/Common/ExcelFIleDragAndDrop.tsx(3 hunks)src/components/Common/Export.tsx(1 hunks)src/components/Common/GLocationPicker.tsx(3 hunks)src/components/Common/SearchByMultipleFields.tsx(6 hunks)src/components/Facility/DuplicatePatientDialog.tsx(1 hunks)src/components/Form/FormFields/PhoneNumberFormField.tsx(2 hunks)src/components/Patient/PatientRegistration.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/FollowUpAppointmentQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(2 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Questionnaire/structured/handlers.ts(1 hunks)src/components/Schedule/Appointments/utils.ts(1 hunks)src/components/Schedule/ScheduleTemplatesList.tsx(1 hunks)src/components/Schedule/routes.tsx(1 hunks)src/components/Users/UserAvailabilityTab.tsx(2 hunks)src/components/ui/chart.tsx(1 hunks)src/components/ui/input-otp.tsx(1 hunks)src/hooks/useAppHistory.ts(1 hunks)src/hooks/useFileManager.tsx(1 hunks)src/hooks/useFileUpload.tsx(1 hunks)src/hooks/useFilters.tsx(2 hunks)src/pages/Encounters/EncounterShow.tsx(1 hunks)src/pages/Encounters/PrintPrescription.tsx(1 hunks)src/pages/Encounters/tabs/EncounterNotesTab.tsx(1 hunks)src/pages/Encounters/tabs/EncounterPlotsTab.tsx(1 hunks)src/pages/FacilityOrganization/components/FacilityOrganizationSelector.tsx(1 hunks)src/service-worker.ts(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx
[error] 60-60: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
src/components/Patient/PatientRegistration.tsx
[error] 474-474: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (40)
src/Routers/routes/questionnaireRoutes.tsx (1)
9-10: LGTM! Consistent handling of route parameters.The addition of
|| ""fallback ensures type safety by preventing undefined values from being passed to the components, aligning with thenoUncheckedIndexedAccessrequirements.src/Routers/routes/UserRoutes.tsx (1)
13-15: LGTM! Consistent handling of tab parameters.The addition of
|| ""fallback ensures type safety by preventing undefined values from being passed to theUserHomecomponent, aligning with thenoUncheckedIndexedAccessrequirements.Also applies to: 20-20, 22-22
src/Routers/routes/ResourceRoutes.tsx (1)
17-18: LGTM! Consistent handling of resource IDs.The addition of
|| ""fallback ensures type safety by preventing undefined values from being passed to theResourceDetailsandResourceDetailsUpdatecomponents, aligning with thenoUncheckedIndexedAccessrequirements.src/common/utils.tsx (1)
39-39: Verify all callers handle undefined return valueThe return type change from
stringtostring | undefinedis correct fornoUncheckedIndexedAccess, but could break existing code that expects a string.src/pages/Encounters/tabs/EncounterPlotsTab.tsx (1)
33-34: LGTM! Consider additional type safety improvementsThe optional chaining operator for
data[0]?.idcorrectly handles the case whendatais empty. However, consider adding type assertions or runtime checks for the entire data structure to ensure type safety.✅ Verification successful
The code is already type-safe and well-structured
The codebase shows proper type safety measures:
- Null checking with
if (isLoading || !data)- Optional chaining with
data[0]?.id- Handling undefined
find()result- Safe array mapping in JSX
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any other array accesses in the file that might need similar treatment rg -U '\[\d+\][^?]' src/pages/Encounters/tabs/EncounterPlotsTab.tsxLength of output: 69
Script:
#!/bin/bash # Get the complete file content cat src/pages/Encounters/tabs/EncounterPlotsTab.tsx # Also search for any array operations on data rg -U "data\." src/pages/Encounters/tabs/EncounterPlotsTab.tsxLength of output: 2105
src/Routers/AppRouter.tsx (1)
68-69: LGTM! Good error handling.The conditional rendering with fallback to ErrorPage is a clean way to handle undefined route parameters.
src/components/Schedule/ScheduleTemplatesList.tsx (1)
110-111: Extract magic strings as named constants.The default time values should be defined as constants at the module level for better maintainability and reusability.
+const DEFAULT_START_TIME = "00:00"; +const DEFAULT_END_TIME = "23:59"; + // ... rest of the code ... - slot.availability[0]?.start_time ?? "00:00", - slot.availability[0]?.end_time ?? "23:59", + slot.availability[0]?.start_time ?? DEFAULT_START_TIME, + slot.availability[0]?.end_time ?? DEFAULT_END_TIME,Also, verify if these default values are appropriate for your use case, as they span the entire day.
src/pages/FacilityOrganization/components/FacilityOrganizationSelector.tsx (1)
103-108: LGTM! Proper null checks added.The added null checks for
lastOrgandlastOrg.idimprove type safety by preventing potential undefined access.src/components/Common/DateTextInput.tsx (1)
65-65: LGTM! Added safe array access.The nullish coalescing operator ensures safe array access by providing a default value of 0 when
maxMap[index]is undefined.src/components/Questionnaire/QuestionTypes/FollowUpAppointmentQuestion.tsx (1)
53-56: LGTM! Added proper default values.The nullish coalescing operator ensures that
valueis always a validFollowUpAppointmentRequestobject with default values, preventing potential undefined access.src/components/Common/GLocationPicker.tsx (2)
140-140: LGTM! Safe access to map controls.The optional chaining operators ensure safe access to Google Maps controls, which may be undefined during initialization.
Also applies to: 171-171, 179-179
159-159: LGTM! Safe access to places data.The optional chaining operator ensures safe access to potentially undefined places data, preventing runtime errors.
src/hooks/useFilters.tsx (2)
97-99: LGTM! Safe handling of potentially undefined values.The addition of
|| ""ensures that the value will always be a string, preventing potential undefined values from propagating.
133-133: LGTM! Safe array index access.Adding
|| ""as a fallback when accessing array indices aligns with thenoUncheckedIndexedAccessTypeScript option.src/components/Common/SearchByMultipleFields.tsx (3)
72-72: LGTM! Safe initialization and type checking.The addition of optional chaining (
?.) ensures safe access toselectedOptionproperties during initialization and type checking.Also applies to: 79-80
98-106: LGTM! Robust error handling in handleOptionChange.The changes ensure safe handling of potentially undefined values:
- Added fallbacks for option values
- Added null check before calling onFieldChange
162-165: LGTM! Consistent null-safe property access.Optional chaining has been added consistently across the component for accessing selectedOption properties.
Also applies to: 183-183, 188-188, 202-202
src/hooks/useFileUpload.tsx (1)
108-108: LGTM! Safe file extension extraction.Adding
|| ""as a fallback ensures that the extension will always be a string, even if the filename doesn't contain a period.src/components/Form/FormFields/PhoneNumberFormField.tsx (2)
107-113: LGTM! Robust country code handling.The changes add proper null checks when looking up country data:
- Check if countryCode exists
- Check if countryData exists for the code
195-195: LGTM! Safe phone code conditioning.Adding
|| ""ensures that the code is always a string before attempting to split it.src/components/Common/ExcelFIleDragAndDrop.tsx (4)
66-71: LGTM! Good error handling.The addition of the worksheet existence check with proper error handling improves the robustness of the code.
127-127: LGTM! Safe property access.The addition of optional chaining for accessing the file type property improves type safety.
132-134: LGTM! Proper null check.The addition of the null check before calling
onSelectFileprevents potential runtime errors.
189-191: LGTM! Safe file handling.The addition of the null check before calling
onSelectFileensures safe file handling.src/components/ui/chart.tsx (1)
140-140: LGTM! Safe property access.The addition of optional chaining for accessing
dataKeyandnameproperties improves type safety.src/components/Users/UserAvailabilityTab.tsx (3)
203-204: LGTM! Safe time handling with defaults.The addition of optional chaining and default values for time properties ensures safe access and valid time values.
302-302: LGTM! Safe start time handling.The addition of optional chaining and default value for
start_timeensures safe access and a valid time value.
303-303: LGTM! Safe end time handling.The addition of optional chaining and default value for
end_timeensures safe access and a valid time value.src/pages/Encounters/PrintPrescription.tsx (2)
197-197: LGTM! Safe code property access.The addition of optional chaining for accessing the
codeproperty improves type safety.
200-200: LGTM! Safe meaning property access.The addition of optional chaining for accessing the
meaningproperty improves type safety.src/components/Schedule/Appointments/utils.ts (1)
55-58: LGTM! Good error handling for sorting.The addition of optional chaining (
?.) and nullish coalescing (??) operators improves the code's robustness by providing a sensible default value when sorting slots with potentially undefined values.src/Utils/utils.ts (2)
290-292: LGTM! Safe property access in phone formatting.Good use of optional chaining and nullish coalescing to handle potentially undefined country codes.
307-309: LGTM! Improved phone code validation.The changes enhance the robustness of phone code validation by safely handling undefined values and providing an empty string fallback.
src/Providers/PatientUserProvider.tsx (1)
53-55: LGTM! Proper null check added.The addition of the null check before setting the selected patient is a good defensive programming practice that aligns with the new TypeScript configuration.
src/pages/Encounters/tabs/EncounterNotesTab.tsx (1)
334-334: LGTM! Safe array access implemented.The use of optional chaining and nullish coalescing (
threadsData?.results[0]?.id ?? null) is the correct way to handle potentially undefined values when accessing array elements.src/components/Questionnaire/QuestionnaireEditor.tsx (2)
134-136: LGTM! Safe array manipulation in drag handler.The null check before splicing the reordered item is a good safety measure that aligns with the new TypeScript configuration.
1154-1156: LGTM! Consistent null checking pattern.The same safety pattern is correctly applied to the sub-questions drag handler, maintaining consistency throughout the codebase.
src/components/ui/input-otp.tsx (1)
36-37: LGTM! Improved null safety handlingThe changes enhance type safety by safely accessing and destructuring the slot object with a fallback to an empty object.
src/CAREUI/interactive/HumanChart.tsx (1)
39-39: LGTM! Added safety check for RegExp constructionThe addition of the
|| ""fallback prevents potential runtime errors when constructing a RegExp with an undefined value.src/components/Questionnaire/structured/handlers.ts (1)
152-152: LGTM! Enhanced null safety in follow-up appointment handlingThe changes improve robustness by safely destructuring values with appropriate defaults, preventing potential runtime errors when handling empty or undefined follow-up appointments.
src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Facility/DuplicatePatientDialog.tsx (1)
42-45: Simplify null check and add default value.The current implementation has redundant null checking. Since TypeScript's optional chaining (
?.) already handles null/undefined values, the additional&&operator is unnecessary.Consider this improvement:
- <span className="font-bold"> - {patientList[0] && patientList[0]?.phone_number} - </span> + <span className="font-bold"> + {patientList[0]?.phone_number ?? '-'} + </span>This change:
- Simplifies the null check using optional chaining
- Adds a default value for better user experience when no phone number is available
🧰 Tools
🪛 Biome (1.9.4)
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/Routers/routes/ConsultationRoutes.tsx(3 hunks)src/Routers/routes/FacilityRoutes.tsx(1 hunks)src/Routers/routes/PatientRoutes.tsx(1 hunks)src/Routers/routes/ResourceRoutes.tsx(1 hunks)src/Routers/routes/UserRoutes.tsx(1 hunks)src/Routers/routes/questionnaireRoutes.tsx(1 hunks)src/components/Facility/DuplicatePatientDialog.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx(1 hunks)src/components/Schedule/routes.tsx(1 hunks)src/hooks/useAppHistory.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx
- src/hooks/useAppHistory.ts
- src/Routers/routes/questionnaireRoutes.tsx
- src/components/Schedule/routes.tsx
- src/Routers/routes/PatientRoutes.tsx
- src/Routers/routes/ConsultationRoutes.tsx
- src/Routers/routes/ResourceRoutes.tsx
- src/Routers/routes/UserRoutes.tsx
- src/Routers/routes/FacilityRoutes.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Facility/DuplicatePatientDialog.tsx
[error] 43-43: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: OSSAR-Scan
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/common/utils.tsx (1)
40-49: Consider optimizing the array operations.The implementation is correct and safely handles undefined values with the nullish coalescing operator. However, we can optimize the array operations by combining filter and map into a single reduce operation.
Consider this optimization:
return ( humanizeStrings( - options - .filter((option) => { - return id instanceof Array - ? id.map((i) => String(i)).includes(String(option.id)) - : String(option.id) === String(id); - }) - .map((option) => option.text), + options.reduce((acc: string[], option) => { + const shouldInclude = Array.isArray(id) + ? id.map(String).includes(String(option.id)) + : String(option.id) === String(id); + return shouldInclude ? [...acc, option.text] : acc; + }, []), ) ?? "" );🧰 Tools
🪛 Biome (1.9.4)
[error] 44-44: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.(lint/suspicious/useIsArray)
src/common/schemaParser.ts (2)
101-101: Remove redundant variable assignment.The
propvariable is only used once and duplicates the expression from line 97. Consider removing this assignment and using the expression directly where needed.- const prop = schema[key]?.prop ?? key; if (schema[key]?.parent) { const indexKey = schema[key].parent || key; acc[indexKey] = acc[indexKey] || {}; - acc[indexKey][prop] = { value, error }; + acc[indexKey][schema[key]?.prop ?? key] = { value, error };
186-189: Remove unnecessary null checks.The current implementation has redundant safety checks:
- The
dataWithErrors[index]check is unnecessary asdataWithErrorsis created from the same array asparsedData, so the indices will always match.- The optional chaining on
Object.values()is redundant as it always returns an array with thesome()method.return ( - dataWithErrors[index] && - !Object.values(dataWithErrors[index])?.some((item) => item.error) + !Object.values(dataWithErrors[index]).some((item) => item.error) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/CAREUI/interactive/HumanChart.tsx(1 hunks)src/Routers/routes/FacilityRoutes.tsx(1 hunks)src/Utils/AutoSave.tsx(3 hunks)src/common/schemaParser.ts(2 hunks)src/common/utils.tsx(1 hunks)src/components/Common/AudioPlayer.tsx(1 hunks)src/components/Common/Avatar.tsx(1 hunks)src/components/Common/DateTextInput.tsx(1 hunks)src/components/Common/ExcelFIleDragAndDrop.tsx(3 hunks)src/components/Common/SearchByMultipleFields.tsx(6 hunks)src/components/Form/FormFields/PhoneNumberFormField.tsx(2 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Questionnaire/structured/handlers.ts(1 hunks)src/components/Schedule/routes.tsx(1 hunks)src/components/Users/UserAvailabilityTab.tsx(2 hunks)src/components/ui/input-otp.tsx(1 hunks)src/hooks/useAppHistory.ts(1 hunks)src/hooks/useFileUpload.tsx(1 hunks)src/hooks/useFilters.tsx(2 hunks)src/pages/FacilityOrganization/components/FacilityOrganizationSelector.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (18)
- src/components/Schedule/routes.tsx
- src/hooks/useAppHistory.ts
- src/components/ui/input-otp.tsx
- src/CAREUI/interactive/HumanChart.tsx
- src/hooks/useFileUpload.tsx
- src/Utils/AutoSave.tsx
- src/components/Common/Avatar.tsx
- src/components/Questionnaire/structured/handlers.ts
- src/components/Common/AudioPlayer.tsx
- src/components/Common/ExcelFIleDragAndDrop.tsx
- src/pages/FacilityOrganization/components/FacilityOrganizationSelector.tsx
- src/components/Common/DateTextInput.tsx
- src/hooks/useFilters.tsx
- src/components/Questionnaire/QuestionnaireForm.tsx
- src/components/Users/UserAvailabilityTab.tsx
- src/Routers/routes/FacilityRoutes.tsx
- src/components/Common/SearchByMultipleFields.tsx
- src/components/Form/FormFields/PhoneNumberFormField.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/common/utils.tsx
[error] 44-44: Use Array.isArray() instead of instanceof Array.
instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.
(lint/suspicious/useIsArray)
🔇 Additional comments (1)
src/common/schemaParser.ts (1)
94-96: LGTM! Good use of optional chaining and nullish coalescing.The changes improve type safety by properly handling potentially undefined values from
validateAndParse.
|
👋 Hi, @Rishith25, 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
🧹 Nitpick comments (2)
src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx (1)
53-56: Consider these type-safety and maintainability improvements.While the current changes are good, consider these enhancements:
- Extract default values to a named constant for reusability
- Add JSDoc to document the initialization
- Improve the type casting above this code
+ /** Default values for a new appointment question */ + const DEFAULT_APPOINTMENT: CreateAppointmentQuestion = { + reason_for_visit: "", + slot_id: "", + } as const; - const values = - (questionnaireResponse.values?.[0] - ?.value as unknown as CreateAppointmentQuestion[]) || []; + const values = questionnaireResponse.values?.[0]?.value as + | CreateAppointmentQuestion[] + | undefined + ?? []; - const value: CreateAppointmentQuestion = values[0] ?? { - reason_for_visit: "", - slot_id: "", - }; + const value: CreateAppointmentQuestion = values[0] ?? DEFAULT_APPOINTMENT;src/pages/Appointments/utils.ts (1)
55-58: Consider adding validation for empty arrays before sorting.While the current changes handle undefined array access safely, consider adding a guard clause to handle empty arrays explicitly. This would make the code more maintainable and prevent potential edge cases.
result.sort((a, b) => + // Handle empty arrays + !a.slots.length || !b.slots.length + ? 0 + : compareAsc( a.slots[0]?.start_datetime ?? "00:00", b.slots[0]?.start_datetime ?? "00:00", ), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/Routers/AppRouter.tsx(1 hunks)src/Routers/routes/ScheduleRoutes.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(2 hunks)src/components/Questionnaire/structured/handlers.ts(1 hunks)src/components/Users/UserAvailabilityTab.tsx(2 hunks)src/pages/Appointments/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/Questionnaire/structured/handlers.ts
- src/components/Questionnaire/QuestionnaireEditor.tsx
- src/Routers/AppRouter.tsx
- src/components/Users/UserAvailabilityTab.tsx
🔇 Additional comments (2)
src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx (1)
53-56: LGTM! Good type safety improvements.The explicit type annotation and default values effectively address the
noUncheckedIndexedAccessrequirements while preventing potential runtime errors.src/Routers/routes/ScheduleRoutes.tsx (1)
39-40: Maintain consistency in default value handling across components.The same feedback applies here. Consider implementing a consistent strategy for handling undefined route parameters across all components.
|
Hi @amjithtitus09 @rithviknishad @Jacobjeevan |
| }) => ( | ||
| <PrintPrescription | ||
| facilityId={facilityId ?? ""} | ||
| encounterId={encounterId ?? ""} | ||
| /> | ||
| ), |
There was a problem hiding this comment.
would the component work with "" being passed to the id fields?
i mean, the whole reason for adding this strict check is so that it's super safe. i don't see much advantage in enabling this check when we are anyways gonna fallback to empty string.
There was a problem hiding this comment.
@rithviknishad you are right. Falling back to an empty string defeats the purpose of strict checks.
I’m planning to display the ErrorPage when the parameters are undefined or invalid:
"/facility/:facilityId/encounter/:encounterId/prescriptions/print": ({
facilityId,
encounterId,
}) => (
facilityId && encounterId ? (
<PrintPrescription facilityId={facilityId} encounterId={encounterId} />
) : (
<ErrorPage />
)
),Does this approach align with what you had in mind?
There was a problem hiding this comment.
the root issue seems to be with the implementation of AppRoutes type itself. It should've inferred type as string automatically instead, hence these checks wouldn't be needed in the first place.
There was a problem hiding this comment.
what we have now:
what's expected:
There was a problem hiding this comment.
We won't need these checks with #9890 i believe
There was a problem hiding this comment.
i did check after enabling this config in that branch, it still gives string | undefined after enabling it. (atleast it's not never)
There was a problem hiding this comment.
"/facility/:facilityId/encounter/:encounterId/prescriptions/print": ({ facilityId, encounterId, }) => ( facilityId && encounterId ? ( <PrintPrescription facilityId={facilityId} encounterId={encounterId} /> ) : ( <ErrorPage /> ) ),
@rithviknishad So should i follow this approach to handle the undefined right?
There was a problem hiding this comment.
we'll need a better solution than that 😅 this approach is not maintainable. with too many routes, repeating this logic is quite cumbersome.
There was a problem hiding this comment.
(there's a reason for the issue being 6 story points) 🙂
There was a problem hiding this comment.
@rithviknishad I have found two methods
- Using the non-null assertion operator (!)
/facility/:facilityId/encounter/:encounterId/prescriptions/print": ({
facilityId,
encounterId,
}) => (
<PrintPrescription facilityId={facilityId!} encounterId={encounterId!} />
),- I have modified AppRoutes type in AppRoute.tsx
export type AppRoutes = {
[K in keyof RouteFunction<K>]: RouteFunction<K>;
};by 2nd method params are of type any and if we declare the param types as string
"/facility/:facilityId/encounter/:encounterId/prescriptions/print": ({
facilityId,
encounterId,
}: {
facilityId: string;
encounterId: string;
}) => <PrintPrescription facilityId={facilityId} encounterId={encounterId} />,
Currently trying to finding more methods to infer params as directly string instead of string | undefind or never
src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx
Outdated
Show resolved
Hide resolved
rithviknishad
removed
Deploy-Failed
|
@rithviknishad I have done all suggested changes and more relevant changes. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx (1)
62-62:⚠️ Potential issueReplace
isNaNwithNumber.isNaNfor safer type checkingThe global
isNaNperforms type coercion which can lead to unexpected results.Apply this diff to improve type safety:
- if (isNaN(hours) || isNaN(minutes)) { + if (Number.isNaN(hours) || Number.isNaN(minutes)) {🧰 Tools
🪛 Biome (1.9.4)
[error] 62-62: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
[error] 62-62: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
🧹 Nitpick comments (2)
src/pages/Scheduling/components/EditScheduleTemplateSheet.tsx (1)
437-454: Optimize the null checks for better readability.While the implementation is safe, the double optional chaining after the null check is redundant.
Consider simplifying the code:
{Object.entries(availabilitiesByDay).map(([day, times]) => { const dayofWeek = DayOfWeek[parseInt(day)]; if (!dayofWeek) return; - const formattedDay = - dayofWeek?.charAt(0) + dayofWeek?.slice(1).toLowerCase(); + const formattedDay = + dayofWeek.charAt(0) + dayofWeek.slice(1).toLowerCase(); return ( <p key={day} className="flex items-center gap-2 text-sm">src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx (1)
58-64: Consider adding error feedback for invalid time inputCurrently, the function silently returns on invalid input. Consider providing feedback to the user when they enter invalid time values.
+ const showError = (message: string) => { + // You can integrate this with your existing error handling system + console.error(message); + }; + const parts = event.target.value.split(":"); - if (parts.length !== 2) return; + if (parts.length !== 2) { + showError("Invalid time format"); + return; + } const [hoursStr, minutesStr] = parts; const hours = Number(hoursStr); const minutes = Number(minutesStr); if (Number.isNaN(hours) || Number.isNaN(minutes)) { + showError("Invalid time values"); return; } if (hours < 0 || hours > 23 || minutes < 0 || minutes > 59) { + showError("Time values out of range"); return; }🧰 Tools
🪛 Biome (1.9.4)
[error] 62-62: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
[error] 62-62: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.(lint/suspicious/noGlobalIsNan)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
src/CAREUI/interactive/HumanChart.tsx(0 hunks)src/Utils/AutoSave.tsx(2 hunks)src/Utils/utils.ts(2 hunks)src/common/schemaParser.ts(2 hunks)src/components/Common/Avatar.tsx(2 hunks)src/components/Common/DateTextInput.tsx(1 hunks)src/components/Common/GLocationPicker.tsx(3 hunks)src/components/Common/SearchByMultipleFields.tsx(6 hunks)src/components/Facility/DuplicatePatientDialog.tsx(1 hunks)src/components/Patient/PatientRegistration.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Questionnaire/structured/handlers.ts(1 hunks)src/components/Users/UserAvailabilityTab.tsx(2 hunks)src/components/ui/input-otp.tsx(1 hunks)src/hooks/useAppHistory.ts(1 hunks)src/hooks/useFilters.tsx(2 hunks)src/pages/Appointments/utils.ts(1 hunks)src/pages/Encounters/tabs/EncounterNotesTab.tsx(1 hunks)src/pages/Scheduling/ScheduleTemplates.tsx(1 hunks)src/pages/Scheduling/components/EditScheduleTemplateSheet.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- src/CAREUI/interactive/HumanChart.tsx
🚧 Files skipped from review as they are similar to previous changes (18)
- src/components/ui/input-otp.tsx
- src/components/Questionnaire/QuestionTypes/AppointmentQuestion.tsx
- src/Utils/utils.ts
- src/components/Questionnaire/QuestionnaireForm.tsx
- src/components/Common/Avatar.tsx
- src/components/Common/GLocationPicker.tsx
- src/components/Patient/PatientRegistration.tsx
- src/components/Users/UserAvailabilityTab.tsx
- src/pages/Encounters/tabs/EncounterNotesTab.tsx
- src/hooks/useAppHistory.ts
- src/components/Facility/DuplicatePatientDialog.tsx
- src/components/Questionnaire/structured/handlers.ts
- src/common/schemaParser.ts
- src/pages/Appointments/utils.ts
- src/components/Common/DateTextInput.tsx
- src/Utils/AutoSave.tsx
- src/hooks/useFilters.tsx
- src/components/Common/SearchByMultipleFields.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx
[error] 62-62: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
[error] 62-62: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
🔇 Additional comments (3)
src/pages/Scheduling/ScheduleTemplates.tsx (1)
128-135: LGTM! Robust null-safety implementation.The addition of optional chaining and nullish coalescing operators effectively prevents potential runtime errors when accessing array elements and calculating slots.
src/pages/Scheduling/components/EditScheduleTemplateSheet.tsx (2)
333-341: LGTM! Consistent null-safety pattern.The implementation maintains consistency with ScheduleTemplates.tsx in handling potential null values during slot calculations.
Line range hint
128-135: Well-structured implementation of null safety.The consistent pattern of null checks and optional chaining across components demonstrates a thorough approach to preventing runtime errors and improving type safety.
Also applies to: 333-341, 437-454
src/components/Questionnaire/QuestionTypes/DateTimeQuestion.tsx
Outdated
Show resolved
Hide resolved
|
👋 Hi, @Rishith25, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
github-actions
bot
added
merge conflict
|
👋 Hi, @Rishith25, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Proposed Changes
noUncheckedIndexedAccessin tsconfig@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the release notes:
Release Notes
Robustness Improvements
Type Safety Enhancements
Routing and Navigation
Performance and Stability
These changes collectively improve the application's reliability and type safety, reducing potential runtime errors and enhancing overall user experience.