Browse Source

fix: address PR #64 review — blob leak, error handling, tests, docs

Move @types/three to devDependencies, fix blob URL cleanup to track
changes (not just unmount), add try/catch to capture and scene disposal,
remove GLBViewerNode from barrel export to preserve lazy-loading, fix
JSDoc, update CLAUDE.md with glbViewer node and ? shortcut, add
connectedInputs and nodeDefaults tests for glbViewer.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
handoff-20260429-1057
shrimbly 5 months ago
parent
commit
d76a8ada71
  1. 2
      CLAUDE.md
  2. 2
      package.json
  3. 34
      src/components/nodes/GLBViewerNode.tsx
  4. 1
      src/components/nodes/index.ts
  5. 20
      src/store/utils/__tests__/connectedInputs.test.ts
  6. 9
      src/store/utils/__tests__/nodeDefaults.test.ts

2
CLAUDE.md

@ -82,6 +82,7 @@ LLM models:
| `nanoBanana` | AI image generation | image, text | image | | `nanoBanana` | AI image generation | image, text | image |
| `llmGenerate` | AI text generation | text, image | text | | `llmGenerate` | AI text generation | text, image | text |
| `splitGrid` | Split image into grid cells | image | reference | | `splitGrid` | Split image into grid cells | image | reference |
| `glbViewer` | Load/display 3D GLB models | none | image |
| `output` | Display final result | image | none | | `output` | Display final result | image | none |
## Node Connection System ## Node Connection System
@ -125,6 +126,7 @@ Returns `{ images: string[], text: string | null }`.
- `H` - Stack selected nodes horizontally - `H` - Stack selected nodes horizontally
- `V` - Stack selected nodes vertically - `V` - Stack selected nodes vertically
- `G` - Arrange selected nodes in grid - `G` - Arrange selected nodes in grid
- `?` - Show keyboard shortcuts
## Adding New Node Types ## Adding New Node Types

2
package.json

@ -18,7 +18,6 @@
"@react-three/drei": "^10.7.7", "@react-three/drei": "^10.7.7",
"@react-three/fiber": "^9.5.0", "@react-three/fiber": "^9.5.0",
"@tailwindcss/postcss": "^4.1.17", "@tailwindcss/postcss": "^4.1.17",
"@types/three": "^0.182.0",
"@xyflow/react": "^12.9.3", "@xyflow/react": "^12.9.3",
"ai": "^6.0.49", "ai": "^6.0.49",
"autoprefixer": "^10.4.22", "autoprefixer": "^10.4.22",
@ -41,6 +40,7 @@
"@testing-library/react": "^16.3.1", "@testing-library/react": "^16.3.1",
"@types/jszip": "^3.4.0", "@types/jszip": "^3.4.0",
"@types/node": "^24.10.1", "@types/node": "^24.10.1",
"@types/three": "^0.182.0",
"@types/react": "^19.2.7", "@types/react": "^19.2.7",
"@types/react-dom": "^19.2.3", "@types/react-dom": "^19.2.3",
"@vitejs/plugin-react": "^4.7.0", "@vitejs/plugin-react": "^4.7.0",

34
src/components/nodes/GLBViewerNode.tsx

@ -80,12 +80,14 @@ function Model({ url, onError }: { url: string; onError?: () => void }) {
if (sceneRef.current) { if (sceneRef.current) {
sceneRef.current.traverse((obj) => { sceneRef.current.traverse((obj) => {
if (obj instanceof THREE.Mesh) { if (obj instanceof THREE.Mesh) {
obj.geometry?.dispose(); try { obj.geometry?.dispose(); } catch (e) { console.warn("GLB geometry dispose failed:", e); }
try {
if (Array.isArray(obj.material)) { if (Array.isArray(obj.material)) {
obj.material.forEach((m) => m.dispose()); obj.material.forEach((m) => m.dispose());
} else { } else {
obj.material?.dispose(); obj.material?.dispose();
} }
} catch (e) { console.warn("GLB material dispose failed:", e); }
} }
}); });
sceneRef.current = null; sceneRef.current = null;
@ -140,7 +142,7 @@ function CaptureHelper({
captureRef, captureRef,
envGroupRef, envGroupRef,
}: { }: {
captureRef: React.MutableRefObject<(() => string) | null>; captureRef: React.MutableRefObject<(() => string | null) | null>;
envGroupRef: React.RefObject<THREE.Group | null>; envGroupRef: React.RefObject<THREE.Group | null>;
}) { }) {
const { gl, scene, camera } = useThree(); const { gl, scene, camera } = useThree();
@ -148,6 +150,7 @@ function CaptureHelper({
// Update the capture function every frame so it always has current gl/scene/camera // Update the capture function every frame so it always has current gl/scene/camera
useFrame(() => { useFrame(() => {
captureRef.current = () => { captureRef.current = () => {
try {
// Directly hide environment objects in the Three.js scene graph (synchronous) // Directly hide environment objects in the Three.js scene graph (synchronous)
if (envGroupRef.current) { if (envGroupRef.current) {
envGroupRef.current.visible = false; envGroupRef.current.visible = false;
@ -163,6 +166,14 @@ function CaptureHelper({
} }
return dataUrl; return dataUrl;
} catch (err) {
console.warn("GLB capture failed:", err);
// Restore environment objects on failure
if (envGroupRef.current) {
envGroupRef.current.visible = true;
}
return null;
}
}; };
}); });
@ -190,7 +201,7 @@ function AutoRotate({ enabled }: { enabled: boolean }) {
} }
/** /**
* Loading spinner shown while GLB is parsing. * Loading indicator (wireframe sphere) shown while GLB is parsing.
*/ */
function LoadingIndicator() { function LoadingIndicator() {
return ( return (
@ -207,7 +218,7 @@ export function GLBViewerNode({ id, data, selected }: NodeProps<GLBViewerNodeTyp
const updateNodeData = useWorkflowStore((state) => state.updateNodeData); const updateNodeData = useWorkflowStore((state) => state.updateNodeData);
const { setNodes, getNodes } = useReactFlow(); const { setNodes, getNodes } = useReactFlow();
const fileInputRef = useRef<HTMLInputElement>(null); const fileInputRef = useRef<HTMLInputElement>(null);
const captureRef = useRef<(() => string) | null>(null); const captureRef = useRef<(() => string | null) | null>(null);
const viewportRef = useRef<HTMLDivElement>(null); const viewportRef = useRef<HTMLDivElement>(null);
const [autoRotate, setAutoRotate] = useState(false); const [autoRotate, setAutoRotate] = useState(false);
const [isInteracting, setIsInteracting] = useState(false); const [isInteracting, setIsInteracting] = useState(false);
@ -256,15 +267,16 @@ export function GLBViewerNode({ id, data, selected }: NodeProps<GLBViewerNodeTyp
}); });
}, [id, nodeData.capturedImage, getNodes, setNodes]); }, [id, nodeData.capturedImage, getNodes, setNodes]);
// Revoke blob URL when node is unmounted (e.g. deleted from canvas) // Revoke blob URL when it changes or when node is unmounted
const glbUrlRef = useRef<string | null>(nodeData.glbUrl);
useEffect(() => { useEffect(() => {
glbUrlRef.current = nodeData.glbUrl;
return () => { return () => {
if (nodeData.glbUrl) { if (glbUrlRef.current) {
URL.revokeObjectURL(nodeData.glbUrl); URL.revokeObjectURL(glbUrlRef.current);
} }
}; };
// eslint-disable-next-line react-hooks/exhaustive-deps -- only revoke on unmount }, [nodeData.glbUrl]);
}, []);
// Shared file processing logic for both click-to-upload and drag-and-drop // Shared file processing logic for both click-to-upload and drag-and-drop
const processFile = useCallback( const processFile = useCallback(
@ -339,7 +351,11 @@ export function GLBViewerNode({ id, data, selected }: NodeProps<GLBViewerNodeTyp
const handleCapture = useCallback(() => { const handleCapture = useCallback(() => {
if (captureRef.current) { if (captureRef.current) {
const base64 = captureRef.current(); const base64 = captureRef.current();
if (base64) {
updateNodeData(id, { capturedImage: base64 }); updateNodeData(id, { capturedImage: base64 });
} else {
useToast.getState().show("Failed to capture 3D view", "error");
}
} }
}, [id, updateNodeData]); }, [id, updateNodeData]);

1
src/components/nodes/index.ts

@ -12,5 +12,4 @@ export { OutputGalleryNode } from "./OutputGalleryNode";
export { ImageCompareNode } from "./ImageCompareNode"; export { ImageCompareNode } from "./ImageCompareNode";
export { VideoStitchNode } from "./VideoStitchNode"; export { VideoStitchNode } from "./VideoStitchNode";
export { EaseCurveNode } from "./EaseCurveNode"; export { EaseCurveNode } from "./EaseCurveNode";
export { GLBViewerNode } from "./GLBViewerNode";
export { GroupNode } from "./GroupNode"; export { GroupNode } from "./GroupNode";

20
src/store/utils/__tests__/connectedInputs.test.ts

@ -167,6 +167,26 @@ describe("getConnectedInputsPure", () => {
}); });
}); });
it("should extract capturedImage from glbViewer source", () => {
const nodes = [
makeNode("glb", "glbViewer", { capturedImage: "data:image/png;base64,snap" }),
makeNode("gen", "nanoBanana"),
];
const edges = [makeEdge("glb", "gen", "image")];
const result = getConnectedInputsPure("gen", nodes, edges);
expect(result.images).toEqual(["data:image/png;base64,snap"]);
});
it("should return empty images when glbViewer has no capture", () => {
const nodes = [
makeNode("glb", "glbViewer", { capturedImage: null }),
makeNode("gen", "nanoBanana"),
];
const edges = [makeEdge("glb", "gen", "image")];
const result = getConnectedInputsPure("gen", nodes, edges);
expect(result.images).toEqual([]);
});
it("should extract audio from audioInput source", () => { it("should extract audio from audioInput source", () => {
const nodes = [ const nodes = [
makeNode("aud", "audioInput", { audioFile: "data:audio/wav;base64,abc" }), makeNode("aud", "audioInput", { audioFile: "data:audio/wav;base64,abc" }),

9
src/store/utils/__tests__/nodeDefaults.test.ts

@ -41,6 +41,7 @@ describe("nodeDefaults utilities", () => {
"generateVideo", "generateVideo",
"llmGenerate", "llmGenerate",
"splitGrid", "splitGrid",
"glbViewer",
"output", "output",
]; ];
@ -176,6 +177,14 @@ describe("nodeDefaults utilities", () => {
expect(data).toHaveProperty("error", null); expect(data).toHaveProperty("error", null);
}); });
it("creates correct structure for glbViewer", () => {
const data = createDefaultNodeData("glbViewer");
expect(data).toHaveProperty("glbUrl", null);
expect(data).toHaveProperty("filename", null);
expect(data).toHaveProperty("capturedImage", null);
});
it("creates correct structure for output", () => { it("creates correct structure for output", () => {
const data = createDefaultNodeData("output"); const data = createDefaultNodeData("output");

Loading…
Cancel
Save