Fix linter errors by eloiberlinger1 · Pull Request #20 · setrsoft/setrsoft_app

eloiberlinger1 · 2026-04-20T20:03:58Z

No description provided.

…nd optimize state management for drag and wall loading logic

…ent download URL

…agement and improve performance with useMemo and useCallback

…tore.ts. replacing any across all files

…ove manufacturer handling

gemini-code-assist

Code Review

This pull request focuses on improving type safety and state management across the editor feature by replacing 'any' types with explicit interfaces and refactoring several components to use React hooks like useMemo and useCallback. Key changes include the introduction of centralized interfaces for session and hold data in the store, as well as more robust loading logic for wall models in the MainCanvas. Feedback is provided regarding a direct object mutation within a useMemo hook in EditorApp.tsx, which should be replaced with an immutable update to avoid side effects on cached data. Additionally, an unnecessary type cast in AddHoldModal.tsx was identified that could lead to bugs if IDs are strings.

gemini-code-assist · 2026-04-20T20:05:29Z

+      session_data.holds_collection_instances?.forEach((hold: SessionHoldInstance) => {
+        hold.hold_instance_id = hold.id;
+        holdModels.push(hold);
+        if (hold.hold_type?.glb_url) {
+          holdModelsGLBURL.push(hold.hold_type.glb_url);
+        }
+      });


Mutating the hold object directly inside useMemo is a side effect that should be avoided. Since session_data likely comes from a cache (e.g., TanStack Query), mutating its contents can lead to unexpected behavior in other parts of the application or when the data is re-read from the cache. It is better to create a new object with the added property to maintain immutability.

Suggested change

session_data.holds_collection_instances?.forEach((hold: SessionHoldInstance) => {

hold.hold_instance_id = hold.id;

holdModels.push(hold);

if (hold.hold_type?.glb_url) {

holdModelsGLBURL.push(hold.hold_type.glb_url);

}

});

session_data.holds_collection_instances?.forEach((hold: SessionHoldInstance) => {

const holdWithId = { ...hold, hold_instance_id: hold.id };

holdModels.push(holdWithId);

if (holdWithId.hold_type?.glb_url) {

holdModelsGLBURL.push(holdWithId.hold_type.glb_url);

}

});

gemini-code-assist · 2026-04-20T20:05:30Z

+        (hold: StockItem) =>
          !currentHoldTypeIds.has(hold.hold_type.id) &&
-          !addedHoldIds.has(hold.hold_type.id)
+          !addedHoldIds.has(hold.hold_type.id as number)


The type cast as number is unnecessary and potentially incorrect. hold.hold_type.id is defined as string | number, and addedHoldIds is a Set<string | number>. Casting to number will cause issues if the ID is actually a string.

Suggested change

!addedHoldIds.has(hold.hold_type.id as number)

!addedHoldIds.has(hold.hold_type.id)

Copilot

Pull request overview

This PR addresses frontend lint/TypeScript issues in the editor feature by tightening types, removing any usage, and adjusting a few React/Three.js patterns to satisfy lint rules.

Changes:

Centralizes and reuses editor model types (HoldType, HoldModel, SessionData, etc.) from frontend/src/features/editor/store.ts.
Replaces multiple any usages with explicit types and safer property access across editor utils/components.
Makes targeted lint-driven refactors (e.g., catch without unused vars, const correctness, react-refresh lint suppression).

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
frontend/src/shared/auth/AuthProvider.tsx	Adds react-refresh lint suppression for exported hook in same module.
frontend/src/features/editor/utils/useHoldAvailabilityValidation.tsx	Adds local types, removes `any`, and improves manufacturer handling.
frontend/src/features/editor/utils/HandleLoadSession.tsx	Introduces `SessionData` typing and typed layout objects.
frontend/src/features/editor/utils/HandleAddHold.tsx	Switches to shared store types and returns a typed `HoldModel`.
frontend/src/features/editor/stubs/Hold360.tsx	Adds react-refresh lint suppression and types `hold` access.
frontend/src/features/editor/store.ts	Introduces shared editor interfaces/types for holds and sessions.
frontend/src/features/editor/components/useDragPreview.ts	Cleans up const usage, removes unused catch var, improves typing.
frontend/src/features/editor/components/SidebarHoldsSection.tsx	Refactors local state handling (add/delete), improves typing, memoization/callbacks.
frontend/src/features/editor/components/Sidebar.tsx	Uses shared types and changes how `currentHolds` is derived for the modal.
frontend/src/features/editor/components/ModelViewer.tsx	Replaces `any` material access with `MeshStandardMaterial`-typed access.
frontend/src/features/editor/components/MainCanvas.tsx	Refactors wall loading state logic and removes some derived state effects.
frontend/src/features/editor/components/HoldInspector.tsx	Replaces `as any` listener casts with typed `EventListener` assertions.
frontend/src/features/editor/components/FileManager.tsx	Types `session_data` and removes `any` from object cleaning logic.
frontend/src/features/editor/components/AddHoldModal.tsx	Adds `SessionData` typing + `StockItem` type and reduces `any` usage.
frontend/src/features/editor/EditorApp.tsx	Replaces mutable in-render arrays with a `useMemo`-derived model list.

Comments suppressed due to low confidence (1)

frontend/src/features/editor/utils/useHoldAvailabilityValidation.tsx:92

current_location uses `Wall ${stockHold.last_used_wall?.wall_id}` as a fallback value, but that template string is always truthy (even when wall_id is undefined), which prevents the final "Unknown location" fallback from ever being used and can yield "Wall undefined". Only use the wall-id string when wall_id is present; otherwise fall through to the final fallback.

              })(),
              required_count: requiredCount,
              available_count: availableCount,
              current_location:
                stockHold.last_used_wall?.wall_name ||

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-20T20:09:18Z

          onClose={() => setAddHoldModalOpen(false)}
          session_data={session_data}
          onHoldAdded={handleHoldAddedFromModal}
-          currentHolds={
-            holdsSectionRef.current?.getCurrentHolds() || processedHoldModels
-          }
+          currentHolds={[...processedHoldModels, ...locallyAddedHolds]}
        />


currentHolds is derived from processedHoldModels plus locallyAddedHolds, but it doesn't reflect deletions performed inside SidebarHoldsSection (which tracks deletions locally). That can cause AddHoldModal to incorrectly treat deleted hold types as still present and hide them from the stock list. Consider lifting the single source of truth for current holds to the parent (e.g., have SidebarHoldsSection notify on delete/add), or revert to using holdsSectionRef.current?.getCurrentHolds() when opening/feeding the modal so it reflects the actual sidebar state.

Copilot · 2026-04-20T20:09:19Z


    const currentHoldData = stockData?.holds?.find(
-      (stock: any) => stock.id === hold.id
+      (stock) => stock.id === hold.id


The stock lookup compares stock.id === hold.id using strict equality, but stock.id is typed as string | number while hold.id is a string in this flow. If the API returns numeric IDs, this will fail and show incorrect availability/usage counts. Normalize both sides (e.g., compare stringified IDs) before comparing.

Suggested change

(stock) => stock.id === hold.id

(stock) => String(stock.id) === String(hold.id)

Copilot · 2026-04-20T20:09:19Z

+      hold_instance_id: String(selectedHold.id),
+      id: String(selectedHold.id),
+      name: (selectedHold.hold_type.manufacturer_ref as string | undefined)
+        ?? `${(selectedHold.hold_type.manufacturer as string | undefined) ?? ''} ${selectedHold.hold_type.model ?? ''}`.trim(),


manufacturer can be an object (per HoldType), but the fallback name construction casts it to string and interpolates it, which can produce "[object Object]" at runtime when manufacturer_ref is missing. Extract the manufacturer name when it is an object (e.g., manufacturer.name) before building the name string.

Suggested change

?? `${(selectedHold.hold_type.manufacturer as string | undefined) ?? ''} ${selectedHold.hold_type.model ?? ''}`.trim(),

?? `${typeof selectedHold.hold_type.manufacturer === 'string'

? selectedHold.hold_type.manufacturer

: typeof selectedHold.hold_type.manufacturer === 'object'

&& selectedHold.hold_type.manufacturer !== null

&& 'name' in selectedHold.hold_type.manufacturer

&& typeof selectedHold.hold_type.manufacturer.name === 'string'

? selectedHold.hold_type.manufacturer.name

: ''} ${selectedHold.hold_type.model ?? ''}`.trim(),

Copilot · 2026-04-20T20:09:20Z

+  // Sync batch size with current wall count — React's recommended pattern for resetting
+  // state when derived data changes (avoids setState-in-effect anti-pattern).
+  if (wallBatchSize !== wallObjects.length) {
+    setWallBatchSize(wallObjects.length);
+    setWallLoadCount(0);
+    setForceLoaded(false);
+  }


State resets (setWallBatchSize / setWallLoadCount / setForceLoaded) are being triggered during render when wallObjects.length changes. Render-phase state updates are an anti-pattern and can cause extra renders or warnings under StrictMode. Move this reset logic into a useEffect keyed on wallObjects.length (or derive wallBatchSize without storing it) to avoid updating state during render.

Suggested change

// Sync batch size with current wall count — React's recommended pattern for resetting

// state when derived data changes (avoids setState-in-effect anti-pattern).

if (wallBatchSize !== wallObjects.length) {

setWallBatchSize(wallObjects.length);

setWallLoadCount(0);

setForceLoaded(false);

}

// Reset wall-loading state when the number of wall objects changes.

useEffect(() => {

setWallBatchSize(wallObjects.length);

setWallLoadCount(0);

setForceLoaded(false);

}, [wallObjects.length]);

Copilot · 2026-04-20T20:09:20Z

+  const [deletedHoldTypeIds, setDeletedHoldTypeIds] = useState<Set<string>>(new Set());
  const [showHolds, setShowHolds] = useState(true);
  const [showVolumes, setShowVolumes] = useState(true);
  const scrollRef = useRef<HTMLDivElement>(null);


scrollRef is declared as useRef<HTMLDivElement>(null) but is passed both as the HoldScrollContext value (typed as RefObject<HTMLElement | null>) and assigned to a <section> via a cast. This hides a real type mismatch. Prefer typing the ref as useRef<HTMLElement | null>(null) (or use a div consistently) so you don't need casts and the context/ref stay aligned.

Suggested change

const scrollRef = useRef<HTMLDivElement>(null);

const scrollRef = useRef<HTMLElement | null>(null);

eloiberlinger1 added 5 commits April 17, 2026 16:43

refactor(MainCanvas, useDragPreview): improve variable declarations a…

d8303db

…nd optimize state management for drag and wall loading logic

refactor(SidebarHoldsSection): simplify state initialization for curr…

7ad403d

…ent download URL

refactor(EditorApp, Sidebar, SidebarHoldsSection): optimize state man…

1100f5b

…agement and improve performance with useMemo and useCallback

Introduced HoldType, HoldModel, SessionData, SessionHoldInstance in s…

e1cbd28

…tore.ts. replacing any across all files

refactor(useHoldAvailabilityValidation): enhance type safety and impr…

c742c26

…ove manufacturer handling

Copilot AI review requested due to automatic review settings April 20, 2026 20:03

Copilot started reviewing on behalf of eloiberlinger1 April 20, 2026 20:04 View session

gemini-code-assist Bot reviewed Apr 20, 2026

View reviewed changes

eloiberlinger1 merged commit 8b74a56 into main Apr 20, 2026
3 of 4 checks passed

Copilot AI reviewed Apr 20, 2026

View reviewed changes

eloiberlinger1 deleted the fix-linter-errors branch April 20, 2026 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix linter errors#20

Fix linter errors#20
eloiberlinger1 merged 5 commits into
mainfrom
fix-linter-errors

eloiberlinger1 commented Apr 20, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Apr 20, 2026

Uh oh!

gemini-code-assist Bot Apr 20, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

Copilot AI Apr 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	!addedHoldIds.has(hold.hold_type.id as number)
	!addedHoldIds.has(hold.hold_type.id)

	(stock) => stock.id === hold.id
	(stock) => String(stock.id) === String(hold.id)

-        ?? `${(selectedHold.hold_type.manufacturer as string | undefined) ?? ''} ${selectedHold.hold_type.model ?? ''}`.trim(),
+        ?? `${typeof selectedHold.hold_type.manufacturer === 'string'
+          ? selectedHold.hold_type.manufacturer
+          : typeof selectedHold.hold_type.manufacturer === 'object'
+            && selectedHold.hold_type.manufacturer !== null
+            && 'name' in selectedHold.hold_type.manufacturer
+            && typeof selectedHold.hold_type.manufacturer.name === 'string'
+            ? selectedHold.hold_type.manufacturer.name
+            : ''} ${selectedHold.hold_type.model ?? ''}`.trim(),

	const scrollRef = useRef<HTMLDivElement>(null);
	const scrollRef = useRef<HTMLElement \| null>(null);

Conversation

eloiberlinger1 commented Apr 20, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants