Skip to content

Audit React Best Practices #999

@carlosthe19916

Description

@carlosthe19916

React Best Practices — Fix Plan

Audit based on Vercel Engineering's React Best Practices (70 rules, 8 categories).


Phase 1: Trivial Fixes (Barrel Imports)

Effort: ~10 minutes | Impact: CRITICAL (bundle size)

Fix 3 files importing from the @patternfly/react-icons barrel file instead of direct ESM paths.
The rest of the codebase already uses direct paths — these are the only exceptions.

1.1 — client/src/app/api/model-utils.ts

- import { CircleIcon } from "@patternfly/react-icons";
+ import CircleIcon from "@patternfly/react-icons/dist/esm/icons/circle-icon";

1.2 — client/src/app/components/Autocomplete/Autocomplete.tsx

- import { ExclamationCircleIcon } from "@patternfly/react-icons";
+ import ExclamationCircleIcon from "@patternfly/react-icons/dist/esm/icons/exclamation-circle-icon";

1.3 — client/src/app/components/FilterToolbar/MultiselectFilterControl.tsx

- import { TimesIcon } from "@patternfly/react-icons";
+ import TimesIcon from "@patternfly/react-icons/dist/esm/icons/times-icon";

Verification

npm run check && npm run build

Phase 2: Parallelize Mutation Invalidations

Effort: ~30 minutes | Impact: CRITICAL (eliminates waterfalls)

Replace sequential await invalidateQueries() calls with Promise.all() in mutation callbacks.

2.1 — client/src/app/queries/sbom-groups.ts

useDeleteSbomGroupMutation onSuccess (lines 167-175):

  onSuccess: async (_res, payload) => {
    onSuccess(payload);
-   await queryClient.invalidateQueries({
-     queryKey: [SBOMGroupsQueryKey],
-   });
-   await queryClient.invalidateQueries({
-     queryKey: [SBOMsQueryKey, payload.id],
-   });
+   await Promise.all([
+     queryClient.invalidateQueries({ queryKey: [SBOMGroupsQueryKey] }),
+     queryClient.invalidateQueries({ queryKey: [SBOMsQueryKey, payload.id] }),
+   ]);
  },

useDeleteSbomGroupMutation onError (lines 177-185) — same fix with Promise.all().

2.2 — client/src/app/queries/sboms.ts

useDeleteSbomMutation onSuccess (lines 131-135):

  onSuccess: async (_, sbom) => {
    onSuccess(sbom);
-   await queryClient.invalidateQueries({ queryKey: [SBOMsQueryKey] });
-   queryClient.removeQueries({ queryKey: [SBOMsQueryKey, sbom.id] });
+   await Promise.all([
+     queryClient.invalidateQueries({ queryKey: [SBOMsQueryKey] }),
+     queryClient.removeQueries({ queryKey: [SBOMsQueryKey, sbom.id] }),
+   ]);
  },

2.3 — client/src/app/queries/advisories.ts

Single invalidation per callback — no parallelization needed. Already fine.

2.4 — client/src/app/queries/importers.ts

Single invalidation per callback — no parallelization needed. Already fine.

Verification

npm run check && npm test

Then manually test: delete an SBOM, delete an SBOM group, and confirm cache refreshes correctly.


Phase 3: Replace O(n^2) Deduplication with Map

Effort: ~1 hour | Impact: MEDIUM (algorithmic improvement)

7 locations use .reduce() + prev.find() for deduplication. Replace with Map for O(n) lookups.

Pattern

// Before — O(n^2)
items.reduce((prev, current) => {
  const existing = prev.find((item) => item.id === current.id);
  if (!existing) {
    prev.push(current);
  }
  return prev;
}, [] as Item[]);

// After — O(n)
const seen = new Map<string, Item>();
for (const item of items) {
  if (!seen.has(item.id)) {
    seen.set(item.id, item);
  }
}
return Array.from(seen.values());

3.1 — Simple key dedup (3 files)

File Line Key
pages/vulnerability-details/sboms-by-vulnerability.tsx 63 uuid
pages/sbom-details/vulnerabilities-by-sbom.tsx 102 id
hooks/domain-controls/useVulnerabilitiesOfPackage.ts 88 composite

3.2 — Composite key dedup (3 files)

These use composite keys (e.g., vulnerability.identifier + status). Build a string key for the Map.

File Line Key
hooks/domain-controls/useVulnerabilitiesOfSbom.ts 80 vulnerability.identifier + status
hooks/domain-controls/useSbomsOfVulnerability.ts 70 composite
pages/sbom-scan/hooks/useVulnerabilitiesOfSbom.ts 73 vulnerability.identifier + status

3.3 — Merge-on-duplicate dedup (1 file)

pages/sbom-scan/hooks/useVulnerabilitiesOfSbom.ts (line 73-86) merges data into existing entries when a duplicate is found. Use Map.get() + merge instead of prev.find() + merge.

3.4 — Replace indexOf with Set

pages/sbom-scan/components/VulnerabilityTable.tsx (line 291):

+ const importerSet = new Set(filterValues.importer);
  // inside .filter():
- filterValues.importer.indexOf(extractImporterNameFromAdvisory(...)) !== -1
+ importerSet.has(extractImporterNameFromAdvisory(...))

Verification

npm run check && npm test

Then test the vulnerability scan page, SBOM details vulnerabilities tab, and vulnerability details SBOMs tab to confirm data displays correctly.


Phase 4: Eliminate Derived State in useEffect

Effort: ~1.5 hours | Impact: MEDIUM (removes extra render cycles)

9 filter components sync props into state via useEffect. Replace with direct computation or useMemo.

4.1 — SelectOptions sync (4 files, straightforward)

These mirror category.selectOptions into local state for no reason:

File Lines
FilterPanel/CheckboxFilterControl.tsx 23-31
FilterPanel/AutocompleteLabelFilterControl.tsx 23-34
FilterToolbar/AutocompleteLabelFilterControl.tsx 37-41
FilterPanel/AsyncMultiselectFilterControl.tsx 34-42
FilterToolbar/AsyncMultiselectFilterControl.tsx 42-48
- const [selectOptions, setSelectOptions] = React.useState<FilterSelectOptionProps[]>(
-   Array.isArray(category.selectOptions) ? category.selectOptions : []
- );
- React.useEffect(() => {
-   setSelectOptions(
-     Array.isArray(category.selectOptions) ? category.selectOptions : []
-   );
- }, [category.selectOptions]);

+ const selectOptions = React.useMemo(
+   () => Array.isArray(category.selectOptions) ? category.selectOptions : [],
+   [category.selectOptions]
+ );

4.2 — Input value sync (2 files, evaluate carefully)

These maintain an internal editable inputValue synced from filterValue:

File Lines
FilterPanel/SearchFilterControl.tsx 32-36
FilterToolbar/SearchFilterControl.tsx 37-39

These are "controlled with internal state" patterns for debounced input. The useEffect sync is intentional here — it resets the input when the external filter changes (e.g., user clears filters). Evaluate if the component truly needs editable local state. If the input is not debounced, derive directly. If debounced, keep the effect but document why.

4.3 — Date range sync (2 files, evaluate carefully)

File Lines
FilterPanel/DateRangeFilter.tsx 36-49
FilterToolbar/DateRangeFilter.tsx 53-62

These parse a string interval into from/to Date objects. The useEffect is needed if the user edits dates locally before committing. If dates are always derived from filterValue, use useMemo with parseInterval.

Verification

npm run check && npm test

Then test all filter types: checkbox filters, search filters, date range filters, and autocomplete filters across advisory list and SBOM list pages.


Phase 5: Use .toSorted() for Immutable Sorting

Effort: ~45 minutes | Impact: LOW-MEDIUM (immutability, minor perf)

Replace 13 mutable .sort() calls with .toSorted() (ES2023).

Prerequisite

Verify ES2023 support in tsconfig.json:

grep -i "lib" client/tsconfig.json

If ES2023 is not in the lib array, add it. Rsbuild will handle transpilation for older targets.

5.1 — Replace [...arr].sort() with arr.toSorted()

File Line Current
hooks/table-controls/sorting/getLocalSortDerivedState.ts 53 [...items].sort(fn)
api/model-utils.ts 183 [...scores].sort(fn)[0]

5.2 — Replace .sort() on fresh arrays (already safe, but cleaner)

File Line
hooks/table-controls/getHubRequestParams.ts 106
hooks/table-controls/filtering/getFilterHubRequestParams.ts 247
hooks/useUrlParams.ts 153
pages/sbom-groups/sbom-group-labels.tsx 26
components/LabelsAsList.tsx 21
components/EditLabelsForm.tsx 92
pages/search/components/SearchMenu.tsx 128
pages/sbom-scan/components/VulnerabilityTable.tsx 298
pages/sbom-scan/hooks/useVulnerabilitiesOfSbom.ts 128

5.3 — Replace .sort().reverse() with .toSorted().toReversed()

File Line
components/VulnerabilityGallery.tsx 36-41
components/SbomVulnerabilitiesDonutChart.tsx 29

5.4 — Skip auto-generated files

client/src/app/client/core/queryKeySerializer.gen.ts — auto-generated, do not modify.

Verification

npm run check && npm run build && npm test

Phase 6: Migrate useContext to React 19 use()

Effort: ~1 hour | Impact: MEDIUM (React 19 modernization)

The project uses React 19 and has zero forwardRef usage (already clean), but still uses useContext in 43 places across ~30 files. React 19's use() is the modern replacement — it can be called conditionally and also unwraps promises.

Pattern

- import { useContext } from "react";
+ import { use } from "react";

- const value = useContext(MyContext);
+ const value = use(MyContext);

// or for React.useContext:
- const value = React.useContext(MyContext);
+ const value = React.use(MyContext);

6.1 — Core components (high traffic)

File Lines Context
components/Notifications.tsx 11 NotificationsContext
components/ErrorFallback.tsx 32 NotificationsContext
components/PageDrawerContext.tsx 124 PageDrawerContext
hooks/useNotifyErrorCallback.ts 10 NotificationsContext

6.2 — Page-level context consumers

File Lines Context
pages/advisory-list/advisory-toolbar.tsx 29 AdvisoryContext
pages/advisory-list/advisory-table.tsx 48, 51 AdvisoryContext
pages/advisory-list/components/AdvisoryEditLabelsForm.tsx 22 AdvisoryContext
pages/advisory-details/advisory-details.tsx 52 PageDrawerContext
pages/sbom-list/sbom-toolbar.tsx 54 SbomContext
pages/sbom-list/sbom-table.tsx 44, 55 SbomContext
pages/sbom-list/components/SBOMEditLabelsForm.tsx 23 SbomContext
pages/sbom-details/sbom-details.tsx 54 PageDrawerContext
pages/sbom-groups/sbom-groups.tsx 36 SbomGroupsContext
pages/sbom-groups/sbom-groups-toolbar.tsx 17 SbomGroupsContext
pages/sbom-groups/sbom-groups-table.tsx 33, 41, 136 SbomGroupsContext
pages/vulnerability-list/vulnerability-table.tsx 32 VulnerabilityContext
pages/vulnerability-list/vulnerability-toolbar.tsx 17 VulnerabilityContext
pages/package-list/package-table.tsx 30 PackageContext
pages/package-list/package-toolbar.tsx 17 PackageContext
pages/license-list/license-table.tsx 25 LicenseContext
pages/importer-list/importer-list.tsx 85 NotificationsContext

6.3 — Search page (8 context calls in one file)

File Lines Context
pages/search/search.tsx 40-80 SbomSearchContext, PackageSearchContext, VulnerabilitySearchContext, AdvisorySearchContext
pages/search/components/SearchMenu.tsx 166 search context

6.4 — Home page

File Lines Context
pages/home/watched-sboms-context.tsx 32 WatchedSbomsContext
pages/home/components/WatchedSbom.tsx 45 WatchedSbomsContext
pages/home/components/WatchedSbomsSection.tsx 18 WatchedSbomsContext

6.5 — Form hooks (import useContext directly)

File Lines
pages/sbom-groups/components/group-form/useGroupFormData.ts 1, 19
pages/importer-list/components/importer-form.tsx 2, 114
pages/sbom-list/components/add-to-group-form/useAddToGroupFormData.ts 1, 16

Verification

npm run check && npm run build && npm test

Composition Patterns — No Action Needed

The following patterns were audited and found to be clean:

Pattern Status Notes
forwardRef usage 0 instances Already React 19 compliant
Boolean prop proliferation 2-3 components All justified (state wrappers like ConditionalTableBody)
Render props ~8 instances All justified (react-hook-form's Controller API requires them)
State lifting Excellent Providers used correctly (WatchedSbomsContext, PageDrawerContext)

Summary

Phase Category Severity Effort Files
1 Barrel imports CRITICAL ~10 min 3
2 Parallelize mutations CRITICAL ~30 min 2
3 O(n^2) dedup to Map MEDIUM ~1 hr 8
4 Derived state in effect MEDIUM ~1.5 hr 9
5 .toSorted() immutability LOW-MEDIUM ~45 min 13
6 useContext to use() MEDIUM ~1 hr ~30

Total estimated effort: ~5 hours

Each phase is independently deployable and testable. Run npm run check && npm test && npm run build after each phase before proceeding to the next.

Metadata

Metadata

Assignees

No one assigned

    Labels

    needs-kindIndicates an issue or PR lacks a `kind/foo` label and requires one.needs-priorityIndicates an issue or PR lacks a `priority/foo` label and requires one.needs-triageIndicates an issue or PR lacks a `triage/foo` label and requires one.

    Type

    No type

    Projects

    Status

    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions