Revert "Fix linter errors" by eloiberlinger1 · Pull Request #21 · setrsoft/setrsoft_app

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

No description provided.

gemini-code-assist

Code Review

This pull request refactors the editor features by removing several TypeScript interfaces and replacing specific types with "any", while also replacing "useMemo" and "useCallback" hooks with direct render logic or "useEffect" synchronizations. The review identifies several critical issues: calculating derived data in the render body of "EditorApp.tsx" causes performance degradation and unsafe object mutations; missing dependencies in "MainCanvas.tsx" hooks risk stale closures; and the removal of safety filters in "Sidebar.tsx" could lead to runtime crashes. Furthermore, using "useEffect" to sync state with props in "SidebarHoldsSection.tsx" is noted as an anti-pattern that may result in inconsistent UI behavior.

gemini-code-assist · 2026-04-20T20:10:41Z

+  const holdModels: Array<Record<string, HoldInstance>> = [];
+  const holdModelsGLBURL: string[] = [];
+
  useEffect(() => {
    const glbUrl = session_data?.related_wall?.glb_url;
    setWallModels(glbUrl ? [glbUrl] : []);
  }, [session_data?.related_wall?.glb_url]);

-  const { holdModels, holdModelsGLBURL } = useMemo(() => {
-    const holdModels: SessionHoldInstance[] = [];
-    const holdModelsGLBURL: string[] = [];
-    if (session_data?.related_holds_collection) {
-      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);
-        }
-      });
-    }
-    return { holdModels, holdModelsGLBURL };
-  }, [session_data?.related_holds_collection, session_data?.holds_collection_instances]);
+  if (session_data?.related_holds_collection) {
+    session_data.holds_collection_instances?.forEach((hold: any) => {
+      hold.hold_instance_id = hold.id;
+      holdModels.push(hold);
+      if (hold.hold_type?.glb_url) {
+        holdModelsGLBURL.push(hold.hold_type.glb_url);
+      }
+    });
+  }


Calculating holdModels and holdModelsGLBURL directly in the render body introduces several issues:

Performance: These arrays are recreated on every render. Since holdModelsGLBURL is a dependency for the useEffect on line 110 (preloading), that effect will now trigger on every render cycle.

Side Effects in Render: The line hold.hold_instance_id = hold.id mutates the session_data object during rendering. If session_data is managed by a state library or cache (like TanStack Query), this mutation can lead to unpredictable UI bugs.

Type Mismatch: The type Array<Record<string, HoldInstance>> on line 87 does not match the usage (it's being populated as a flat array of objects).

Wrapping this logic in useMemo is highly recommended.

Suggested change

const holdModels: Array<Record<string, HoldInstance>> = [];

const holdModelsGLBURL: string[] = [];

useEffect(() => {

const glbUrl = session_data?.related_wall?.glb_url;

setWallModels(glbUrl ? [glbUrl] : []);

}, [session_data?.related_wall?.glb_url]);

const { holdModels, holdModelsGLBURL } = useMemo(() => {

const holdModels: SessionHoldInstance[] = [];

const holdModelsGLBURL: string[] = [];

if (session_data?.related_holds_collection) {

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);

}

});

}

return { holdModels, holdModelsGLBURL };

}, [session_data?.related_holds_collection, session_data?.holds_collection_instances]);

if (session_data?.related_holds_collection) {

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

hold.hold_instance_id = hold.id;

holdModels.push(hold);

if (hold.hold_type?.glb_url) {

holdModelsGLBURL.push(hold.hold_type.glb_url);

}

});

}

const { holdModels, holdModelsGLBURL } = useMemo(() => {

const models: any[] = [];

const urls: string[] = [];

if (session_data?.related_holds_collection) {

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

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

models.push(holdWithId);

if (hold.hold_type?.glb_url) {

urls.push(hold.hold_type.glb_url);

}

});

}

return { holdModels: models, holdModelsGLBURL: urls };

}, [session_data?.related_holds_collection, session_data?.holds_collection_instances]);

useEffect(() => {

const glbUrl = session_data?.related_wall?.glb_url;

setWallModels(glbUrl ? [glbUrl] : []);

}, [session_data?.related_wall?.glb_url]);

gemini-code-assist · 2026-04-20T20:10:41Z

    wallColors,
    holdColors,
    coloredTexture,
-    addObject,
-    endDrag,
-    selectObject,
-    selectedObjId,
  ]);


The useEffect hook is missing critical dependencies: addObject, endDrag, selectObject, and selectedObjId. These are used within the handleDrop function defined inside this effect. Removing them from the dependency array will cause handleDrop to capture stale values from the initial render, leading to bugs when these functions or values change.

wallColors, holdColors, coloredTexture, addObject, endDrag, selectObject, selectedObjId, ]);

gemini-code-assist · 2026-04-20T20:10:41Z

+  const processedHoldModels: HoldModel[] = holdModels.map((hold) => ({
+    name: hold.hold_type.manufacturer_ref,
+    file: hold.hold_type.cdn_ref,
+    hold_type: hold.hold_type,
+    hold_instance_id: hold.id,
+    id: hold.id,
+  }));


Removing the .filter((hold) => !!hold.hold_type) check introduces a potential runtime crash. If any hold object in holdModels lacks a hold_type property, accessing hold.hold_type.manufacturer_ref will throw a TypeError. It is safer to restore the filter or use optional chaining.

const processedHoldModels: HoldModel[] = holdModels .filter((hold) => !!hold.hold_type) .map((hold) => ({ name: hold.hold_type.manufacturer_ref ?? "", file: hold.hold_type.cdn_ref ?? "", hold_type: hold.hold_type, hold_instance_id: hold.id, id: hold.id, }));

gemini-code-assist · 2026-04-20T20:10:41Z

@@ -1,12 +1,11 @@
-import { useEffect, useMemo, useRef, useState } from "react";
+import { useEffect, useRef, useState } from "react";


The useMemo hook is required for the optimizations suggested in the component body to ensure reference stability and prevent unnecessary re-renders.

Suggested change

import { useEffect, useRef, useState } from "react";

import { useEffect, useMemo, useRef, useState } from "react";

gemini-code-assist · 2026-04-20T20:10:41Z

+  useEffect(() => {
+    setModels((prevModels) => {
+      const locallyAddedHolds = prevModels.filter(
+        (prevModel) =>
+          !holdModels.some(
+            (holdModel) =>
+              holdModel.hold_instance_id === prevModel.hold_instance_id
+          )
+      );
+      return [...holdModels, ...locallyAddedHolds];
+    });
+  }, [holdModels]);


Using useEffect to synchronize the models state with the holdModels prop is an anti-pattern that causes unnecessary extra render cycles. Furthermore, the current implementation of handleDelete (line 95) only updates the local models state. If the holdModels prop subsequently changes, this useEffect will re-add the deleted items because they still exist in the holdModels array. Consider deriving models using useMemo by combining the server-provided holdModels with a separate state for local additions/deletions.

Copilot

Pull request overview

This PR reverts a prior “Fix linter errors” change set in the frontend editor/auth code, primarily by removing/relaxing TypeScript typings and adjusting some editor UI logic (holds sidebar, drag preview, and wall loading).

Changes:

Removes several shared TS types and replaces typed parameters/locals with any / inline types across editor utilities and components.
Refactors holds sidebar state handling (local add/delete + current holds wiring) and updates AddHoldModal / Sidebar interactions.
Tweaks drag preview + canvas logic (rotation calculations, drag/drop, wall loading overlay), plus some small auth/editor component lint-related edits.

Reviewed changes

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

Show a summary per file

File	Description
frontend/src/shared/auth/AuthProvider.tsx	Removes a react-refresh eslint disable comment before `useAuth` export.
frontend/src/features/editor/utils/useHoldAvailabilityValidation.tsx	Drops local TS types and uses `any`; simplifies manufacturer extraction.
frontend/src/features/editor/utils/HandleLoadSession.tsx	Removes `SessionData` typing and layout object typing in favor of `any`.
frontend/src/features/editor/utils/HandleAddHold.tsx	Replaces imports with inline interfaces and loosens the added-hold model typing.
frontend/src/features/editor/stubs/Hold360.tsx	Removes eslint disable and replaces typed `hold` with `unknown` + cast.
frontend/src/features/editor/store.ts	Removes exported hold/session model interfaces (keeps Zustand store types).
frontend/src/features/editor/components/useDragPreview.ts	Adjusts rotation/drag logic and relaxes typing (introduces lint concerns).
frontend/src/features/editor/components/SidebarHoldsSection.tsx	Refactors local holds state handling and changes drag/delete logic (introduces lint concerns).
frontend/src/features/editor/components/Sidebar.tsx	Removes typed editor models and changes how current holds are passed to the modal.
frontend/src/features/editor/components/ModelViewer.tsx	Uses `any` casts to detect material maps.
frontend/src/features/editor/components/MainCanvas.tsx	Refactors drag/drop and wall loading state; adds derived dragged object state (introduces lint concerns).
frontend/src/features/editor/components/HoldInspector.tsx	Switches event listener casts to `any`.
frontend/src/features/editor/components/FileManager.tsx	Loosens session typing and simplifies blob URL stripping logic with `any`.
frontend/src/features/editor/components/AddHoldModal.tsx	Drops typed stock model and loosens types; changes `addedHoldIds` set typing.
frontend/src/features/editor/EditorApp.tsx	Removes memoization of derived hold models and relaxes types (introduces repeated-preload concern).

Comments suppressed due to low confidence (1)

frontend/src/features/editor/EditorApp.tsx:110

holdModelsGLBURL is recreated every render, so the preload useEffect dependency [... , holdModelsGLBURL] will change on every render and re-run preloading unnecessarily. Compute holdModels/holdModelsGLBURL with useMemo (as before) or derive the URL list inside the effect based on session_data so the effect only runs when the underlying data changes.

  const holdModels: Array<Record<string, HoldInstance>> = [];
  const holdModelsGLBURL: string[] = [];

  useEffect(() => {
    const glbUrl = session_data?.related_wall?.glb_url;
    setWallModels(glbUrl ? [glbUrl] : []);
  }, [session_data?.related_wall?.glb_url]);

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

  const { preload } = useGLTF;
  useEffect(() => {
    [...wallModels, ...holdModelsGLBURL].forEach((url) => {
      preload(url);
    });
  }, [preload, wallModels, holdModelsGLBURL]);

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

Copilot · 2026-04-20T20:15:16Z

+      let finalQ = q.clone();
      let customAngle = 0;
-      if (model && 'customRotation' in model && typeof model.customRotation === "number") {
-        customAngle = model.customRotation;
+      if (model && typeof (model as any).customRotation === "number") {
+        customAngle = (model as any).customRotation;
      }


finalQ is never reassigned (only mutated via .multiply()), so let will trigger the prefer-const lint error. Use const finalQ = q.clone() instead.

Copilot · 2026-04-20T20:15:17Z

          if (parentObj) {
            // Combine base rotation and customRotation for parent
-            const parentQ = new THREE.Quaternion(...parentObj.rotation);
+            let parentQ = new THREE.Quaternion(...parentObj.rotation);


parentQ is never reassigned (only mutated), so let violates prefer-const (eslint error). Use const parentQ = new THREE.Quaternion(...).

Suggested change

let parentQ = new THREE.Quaternion(...parentObj.rotation);

const parentQ = new THREE.Quaternion(...parentObj.rotation);

Copilot · 2026-04-20T20:15:17Z

  const handleDelete = async (hold: HoldModel) => {
-    setDeletedHoldTypeIds((prev) => new Set([...prev, String(hold.hold_type?.id)]));
-    setLocallyAddedHolds((prev) => prev.filter((m) => m.hold_type?.id !== hold.hold_type?.id));
+    setModels(models.filter((m) => m.hold_type.id !== hold.hold_type.id));


setModels(models.filter(...)) captures models from the current render; if multiple deletes happen quickly, it can apply a stale value. Prefer the functional form setModels(prev => prev.filter(...)) for correctness with batched state updates.

Suggested change

setModels(models.filter((m) => m.hold_type.id !== hold.hold_type.id));

setModels((prevModels) =>

prevModels.filter((m) => m.hold_type.id !== hold.hold_type.id)

);

Copilot · 2026-04-20T20:15:17Z

 ): [number, number, number] {
  try {
-    const faceNormal = normal.clone();
+    let faceNormal = normal.clone();


faceNormal is never reassigned; using let violates the repo's prefer-const eslint rule (configured as error) and will fail lint. Change this back to const.

Suggested change

let faceNormal = normal.clone();

const faceNormal = normal.clone();

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

          holdName = match ? match[1] : model.url;
        }
-        const newId = model.id || uuidv4();
+        let newId = model.id || uuidv4();


newId is never reassigned; using let will fail the prefer-const lint rule. Change this back to const newId = ....

Suggested change

let newId = model.id || uuidv4();

const newId = model.id || uuidv4();

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

@@ -158,10 +159,6 @@ function DragPreview() {
    wallColors,
    holdColors,
    coloredTexture,


This effect uses addObject, endDrag, selectedObjId, and selectObject but they are missing from the dependency array. With react-hooks/exhaustive-deps enabled, this will fail lint and can also lead to stale closures at runtime. Include the missing dependencies or refactor (e.g., wrap handleDrop in useCallback with the right deps).

Suggested change

coloredTexture,

coloredTexture,

addObject,

endDrag,

selectedObjId,

selectObject,

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

  useEffect(() => {
    if (onHoldDragState) onHoldDragState(!!selectedHold);
-  }, [selectedHold, onHoldDragState]);
+  }, [selectedHold]);


onHoldDragState is referenced inside this effect but omitted from the dependency list; react-hooks/exhaustive-deps will flag this as an error and it can call a stale callback. Add onHoldDragState to the dependencies (or guard with useCallback upstream).

Suggested change

}, [selectedHold]);

}, [onHoldDragState, selectedHold]);

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

  const { user, authenticatedFetch } = useEditorAuth();
  const API_URL = import.meta.env.VITE_API_BASE;
-  const [, setCurrentDownloadUrl] = useState<string>();
+  const [_currentDownloadUrl, setCurrentDownloadUrl] = useState<string>();


_currentDownloadUrl is declared but never used. With @typescript-eslint/no-unused-vars configured as error (and no varsIgnorePattern), this will fail lint; keep the original const [, setCurrentDownloadUrl] = ... pattern if the value is intentionally ignored.

Suggested change

const [_currentDownloadUrl, setCurrentDownloadUrl] = useState<string>();

const [, setCurrentDownloadUrl] = useState<string>();

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

    finalRotation.setFromRotationMatrix(finalRotationMatrix);
    return [finalRotation.x, finalRotation.y, finalRotation.z];
-  } catch {
+  } catch (error) {


The catch (error) variable is unused, which violates @typescript-eslint/no-unused-vars (configured as error). Use catch { ... } or rename to _error if you intend to ignore it.

Suggested change

} catch (error) {

} catch {

Revert "Fix linter errors"

3cf82e6

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

eloiberlinger1 closed this Apr 20, 2026

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

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

gemini-code-assist Bot reviewed Apr 20, 2026

View reviewed changes

Copilot AI reviewed Apr 20, 2026

View reviewed changes

		@@ -1,12 +1,11 @@
		import { useEffect, useMemo, useRef, useState } from "react";
		import { useEffect, useRef, useState } from "react";

	let parentQ = new THREE.Quaternion(...parentObj.rotation);
	const parentQ = new THREE.Quaternion(...parentObj.rotation);

	let faceNormal = normal.clone();
	const faceNormal = normal.clone();

	let newId = model.id \|\| uuidv4();
	const newId = model.id \|\| uuidv4();

	const [_currentDownloadUrl, setCurrentDownloadUrl] = useState<string>();
	const [, setCurrentDownloadUrl] = useState<string>();

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!

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!

gemini-code-assist Bot Apr 20, 2026

Choose a reason for hiding this comment

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!

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