Skip to content

[ENG-565] switch unimported to knip (resolves vulnerabilities related to simply-git)#16457

Open
rithviknishad wants to merge 4 commits into
developfrom
security/simple-git
Open

[ENG-565] switch unimported to knip (resolves vulnerabilities related to simply-git)#16457
rithviknishad wants to merge 4 commits into
developfrom
security/simple-git

Conversation

@rithviknishad

@rithviknishad rithviknishad commented Jun 16, 2026

Copy link
Copy Markdown
Member

This PR uninstalls unimported package in favor of knip. We are dropping this package from the project as the package has been deprecated and no longer maintained for more than 2 years as of now.

simple-git a transitive dependency of unimported had several vulnerabilities. This should resolve those:

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The PR migrates from unimported to knip for unused code analysis, standardizes component and hook export shapes to use single export styles (either named or default), adds public API documentation markers, refactors medication administration helpers to support grouped medication management, removes unused utilities and constants, and adjusts permissions and dependency management.

Changes

Knip tooling migration and codebase cleanup

Layer / File(s) Summary
Knip tooling integration and CI migration
package.json, knip.json, .github/workflows/linter.yml, .github/copilot-instructions.md
The unimported script is updated to use npx --yes unimported@1.31.1, the unimported devDependency is removed, and knip (6.17.1) is added as a devDependency with a knip.json configuration file introduced to define Knip schema, entry points, exclusions, and issue-ignore rules. GitHub workflows and documentation are updated to run knip instead of unimported.
Dependency and environment updates
package.json, src/vite-env.d.ts
@radix-ui/react-visually-hidden is added to dependencies; html-to-image is removed; zod is updated from ^3.23.8 to ^3.24.2; ts-node is removed from devDependencies; @fontsource/* ambient module declaration is added for TypeScript recognition.
Public API documentation annotations
src/Utils/schema/extensionSchema.ts, src/components/Questionnaire/QuestionTypes/ServiceRequestQuestion.tsx, src/hooks/useExtensions.tsx, src/i18n.ts
JSDoc @public annotations are added to exported APIs: convertJsonSchemaToZod, validateServiceRequestQuestion, useExtensions and withExtensions hooks, and the default i18n export.
Medication administration refactoring
src/components/Medicine/MedicationAdministration/utils.ts
Time-slot and date helpers (getAdministrationsForTimeSlot, getCurrentTimeSlotIndex, getEarliestAuthoredDate) are replaced with a new medication grouping API: groupMedicationsByProduct groups requests by product ID with active-request and PRN tracking, getLatestActiveRequest retrieves the most recent active request in a group, and getGroupAdministrationsForTimeSlot filters administrations by group and time slot.
Component export normalization to default-only
src/components/Common/*, src/components/Scan/*, src/pages/Facility/billing/account/*, src/pages/Facility/billing/invoice/*, src/pages/Facility/billing/paymentReconciliation/*, src/pages/Patient/History/index.tsx
Named export declarations are removed from LanguageSelector, LanguageSelectorLogin, PinPageDialog, PrintFooter, GenericQRScanDialog, SpecimenIDScanDialog, and billing/invoice/payment reconciliation components (AccountSheet, AccountShow, BedChargeItemsTable, ChargeItemsTable, InvoiceList, InvoiceShow, PaymentReconciliationList, PaymentReconciliationShow, PrintPaymentReconciliation, and ClinicalHistoryPage), leaving only default exports.
Component and provider export normalization to named-only
src/components/Common/ResourceSubTypePicker.tsx, src/components/Scan/SpecimenIDScanSuccessDialog.tsx, src/pages/Facility/overview.tsx, src/lib/override/OverrideProvider.tsx, src/pages/Facility/billing/account/components/CreateInvoiceSheet.tsx
Default exports are removed, leaving only named exports for ResourceSubTypePicker, SpecimenIDScanSuccessDialog, FacilityOverview, OverrideProvider, and CreateInvoiceSheet.
Hook export API reshaping
src/hooks/useExtensions.tsx, src/hooks/useUserPreferences.ts, src/types/emr/tagConfig/useTagConfig.ts
useExtensions removes default export and adds named re-exports for ExtensionEntityType and useExtensionSchemas; useUserPreferences becomes default-export-only; tag config hook replaces single-tag useTagConfig with multi-tag useTagConfigs using useQueries for batch queries with disabled control per tag.
Permission system and reports integration updates
src/common/Permissions.tsx, src/pages/Encounters/tabs/overview/summary-panel-reports-tab.tsx
PERMISSION_CREATE_CHARGE_ITEM_DEFINITION is replaced with PERMISSION_SET_CHARGE_ITEM_DEFINITION; summary-panel-reports-tab switches from usePermissions to useHasPermission for template-listing authorization gating.
Utility helper and re-export removals
src/CAREUI/interactive/Zoom.tsx, src/Utils/utils.ts, src/components/Facility/FacilityHome.tsx, src/components/Medicine/MedicationTimingSelect.tsx, src/components/Medicine/utils.ts, src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx, src/pages/Facility/locations/utils/useCurrentLocation.ts, src/pages/Facility/services/utils/useCurrentService.ts
Removed exports: ZoomControls component, saveElementAsImage/getWeeklyIntervalsFromTodayTill/conditionalArrayAttribute from utils, getFacilityFeatureIcon from FacilityHome, reverseFrequencyOption from MedicationTimingSelect, formatMedicationLine from Medicine/utils, reverseFrequencyOption re-export from MedicationRequestQuestion, and silent error wrappers useCurrentLocationSilently and useCurrentServiceSilently. Unused date-fns and lucide-react imports are cleaned up.
Domain type and constant cleanup
src/common/constants.tsx, src/types/base/qualifiedRange/qualifiedRange.ts, src/types/emr/diagnosis/diagnosis.ts, src/types/emr/medicationRequest/medicationRequest.ts, src/types/emr/prescription/prescription.ts, src/types/emr/serviceRequest/serviceRequest.ts, src/types/patient/patientIdentifierConfig/patientIdentifierConfig.ts, src/types/questionnaire/validation.ts, tests/admin/valueset/valuesetConstants.ts, tests/facility/patient/encounter/serviceRequests/serviceRequest.ts
Unused exports removed: SOCIOECONOMIC_STATUS_CHOICES, DOMESTIC_HEALTHCARE_SUPPORT_CHOICES, OCCUPATION_TYPES, PATIENT_NOTES_THREADS, RATION_CARD_CATEGORY, and encounterIcons from constants; COLOR_OPTIONS from qualifiedRange; INACTIVE_DIAGNOSIS_CLINICAL_STATUS from diagnosis; MEDICATION_REQUEST_PRIORITY_COLORS, MEDICATION_PRIORITY_COLORS, and isUniformMan from medicationRequest; groupMedicationsByPrescription from prescription; toServiceRequestUpdateSpec from serviceRequest; extractPatientIdentifierConfigsFromBatchResponse from patientIdentifierConfig; createFieldKeys and createValidationError from questionnaire validation. Test constants VALID_OPERATORS, STATUS_OPTIONS, and OBSERVATION_DEFINITIONS are removed; ACTIVITY_DEFINITIONS is reduced to three items (Urinalysis, Lipid Panel, Fasting Blood Glucose).

Possibly related PRs

  • ohcnetwork/care_fe#16229: Both PRs modify src/lib/override/OverrideProvider.tsx, with this PR removing the default export while the related PR adds/defines OverrideProvider with a default export.
  • ohcnetwork/care_fe#16276: Both PRs modify src/types/base/monetaryComponent/monetaryComponent.ts, with this PR removing PriceBreakdown and getPriceBreakdown, while the related PR refactors monetary component discriminated union and adds isDiscountComponent.

Suggested reviewers

  • nihal467
  • Jacobjeevan
  • gigincg
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The PR description lacks structure; it misses several template sections including Merge Checklist items, specs, documentation updates, I18n considerations, and QA confirmations. Add the missing Merge Checklist section with checkboxes and update description to match the template structure with all required sections completed or explicitly marked as N/A.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: switching from unimported to knip and resolving security vulnerabilities related to simple-git. This is concise, specific, and accurately reflects the primary objective of the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/simple-git

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR removes the deprecated unimported devDependency (and its vulnerable transitive dependency chain including simple-git) and updates CI/dev workflows to invoke unimported via npx instead.

Changes:

  • Replaced the unimported npm script to run via npx instead of the locally installed package.
  • Removed unimported from devDependencies and pruned its transitive dependencies from package-lock.json (including simple-git).

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
package.json Drops the unimported dependency and switches the script to invoke it via npx.
package-lock.json Removes unimported and related transitive packages (including simple-git) from the lockfile.

Comment thread package.json Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 16, 2026 07:27
@greptile-apps

greptile-apps Bot commented Jun 16, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the deprecated unimported package (which carried simple-git vulnerabilities) with knip, and removes all dead code that knip identifies across 59 files.

  • Tooling swap: .unimportedrc.json is deleted, knip.json is added with equivalent entry points plus plugin-API awareness (@public tag, ignoreIssues for src/components/ui/**); the CI workflow and package.json scripts are updated accordingly.
  • Dead code removal: Unused exports (functions, constants, components, types) confirmed to have zero importers in the codebase are deleted — including the entire AdminOrganizationSelector.tsx component, several billing/medicine utility functions, and duplicate default exports.
  • Package cleanup: Removes html-to-image, ts-node, @eslint/eslintrc, stale @types/* packages, and deduplicates a zod entry that was listed in both dependencies and devDependencies; adds @radix-ui/react-visually-hidden (already consumed by sidebar.tsx).

Confidence Score: 5/5

Safe to merge — all deleted code was verified to have no importers in the repo, and the tooling change is a direct like-for-like swap.

Every removed export was confirmed unused via grep before deletion. The @public JSDoc mechanism correctly preserves intentional plugin-API surface. Package removals correspond to code that no longer exists. No functional logic was altered.

No files require special attention.

Important Files Changed

Filename Overview
knip.json New knip configuration: correctly sets up entry points, ignores plugin-boundary exports (ui/**), uses @public tag for preserving intentional API exports, and suppresses enumMembers/types checks that were creating noise.
package.json Removes unimported/html-to-image/ts-node/duplicate zod devDep; adds knip@6.17.1 and @radix-ui/react-visually-hidden (already consumed by sidebar.tsx); zod deduped to a single entry in dependencies at ^3.24.2.
.github/workflows/linter.yml CI step updated from npm run unimported to npm run knip; straightforward one-line swap.
src/pages/Admin/organizations/components/AdminOrganizationSelector.tsx 403-line component deleted; was previously suppressed in .unimportedrc.json ignoreUnimported and confirmed to have zero imports in the codebase.
src/common/constants.tsx Removes several unused exports (OCCUPATION_TYPES, encounterIcons, RATION_CARD_CATEGORY, etc.); all verified to have no remaining imports.
src/hooks/useExtensions.tsx Removes redundant default export; adds @public JSDoc annotations on useExtensions and withExtensions so knip treats them as intentional plugin API exports.
src/vite-env.d.ts Adds declare module "@fontsource/" wildcard declaration; enables TypeScript to resolve @fontsource/ imports (used in CSS and SpecimenIDScanSuccessDialog.tsx) without per-package type stubs.
src/Utils/utils.ts Removes saveElementAsImage (html-to-image dep gone), conditionalArrayAttribute, and getWeeklyIntervalsFromTodayTill — all confirmed unused by grep.
src/pages/Encounters/tabs/overview/summary-panel-reports-tab.tsx Switches from usePermissions + destructure to the more direct useHasPermission helper; functionally equivalent.

Reviews (3): Last reviewed commit: "supported browsers" | Re-trigger Greptile

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 16, 2026

Copy link
Copy Markdown

Deploying care-preview with  Cloudflare Pages  Cloudflare Pages

Latest commit: 93f16d1
Status: ✅  Deploy successful!
Preview URL: https://938f66cf.care-preview-a7w.pages.dev
Branch Preview URL: https://security-simple-git.care-preview-a7w.pages.dev

View logs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

Comment thread package.json Outdated
@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown

🎭 Playwright Test Results

Status: ❌ Failed
Test Shards: 3

Metric Count
Total Tests 321
✅ Passed 320
❌ Failed 1
⏭️ Skipped 0

📊 Detailed results are available in the playwright-final-report artifact.

Run: #9559

@rithviknishad rithviknishad requested a review from a team as a code owner June 16, 2026 10:32
@github-actions github-actions Bot added the Type Changes Contains changes in typescript types label Jun 16, 2026
@rithviknishad rithviknishad changed the title uninstall unimported package, switch to using npx to run it; resolve vulnerabilities related to simple-git [ENG-565] uninstall unimported package, switch to using npx to run it; resolve vulnerabilities related to simple-git Jun 16, 2026
@rithviknishad rithviknishad changed the title [ENG-565] uninstall unimported package, switch to using npx to run it; resolve vulnerabilities related to simple-git [ENG-565] switch unimported to knip (resolves vulnerabilities related to simply-git) Jun 16, 2026
Copilot AI review requested due to automatic review settings June 16, 2026 10:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 57 out of 59 changed files in this pull request and generated 1 comment.

Comment thread package.json

@nihal467 nihal467 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removing the need testing label, since @rithviknishad mentioned no need to test it, since its been merged to another package update PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants