jan-henrik/respira
1
0
Fork 0
mirror of https://github.com/jhbruhn/respira.git synced 2026-01-27 02:13:41 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: Revert pattern validation to local hook to prevent infinite re-renders

Browse source 
The selectPatternValidation selector doesn't work well with Zustand because:
1. It requires machineInfo parameter from a different store
2. Passing machineInfo creates new object references on every render
3. This breaks Zustand's memoization and causes infinite loops

Solution:
- Reverted usePatternValidation.ts to original implementation with useMemo
- Removed selectPatternValidation and usePatternValidationFromStore
- Removed PatternValidationResult type (not needed)
- Updated tests to remove validation selector tests (12 tests remain)

The store selectors (selectPatternCenter, selectRotatedBounds, etc) are
still useful for components that only need those specific values, but
validation logic that depends on external state should stay local.

Co-authored-by: jhbruhn <1036566+jhbruhn@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot] 2025-12-28 08:17:20 +00:00
parent bcb5ea1786
commit 5296590a45
3 changed files with 64 additions and 225 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

92
src/hooks/domain/usePatternValidation.ts
View file

@ -1,7 +1,8 @@
import { useMemo } from "react";
import type { PesPatternData } from "../../formats/import/pesImporter";
import type { MachineInfo } from "../../types/machine";
import { usePatternValidationFromStore } from "../../stores/usePatternStore";
import { calculateRotatedBounds } from "../../utils/rotationUtils";
import { calculatePatternCenter } from "../../components/PatternCanvas/patternCanvasHelpers";
export interface PatternBoundsCheckResult {
fits: boolean;
@ -11,10 +12,8 @@ export interface PatternBoundsCheckResult {
export interface UsePatternValidationParams {
pesData: PesPatternData | null;
machineInfo: MachineInfo | null;
// Note: patternOffset and patternRotation are read from the store
// These params are kept for backward compatibility but are not used
patternOffset?: { x: number; y: number };
patternRotation?: number;
patternOffset: { x: number; y: number };
patternRotation: number;
}
/**
@ -23,39 +22,76 @@ export interface UsePatternValidationParams {
* Checks if the pattern (with rotation and offset applied) fits within
* the machine's hoop bounds and provides detailed error messages if not.
*
* This hook now uses the computed selector from the pattern store for
* consistent validation logic across the application.
*
* @param params - Pattern and machine configuration
* @returns Bounds check result with fit status and error message
*/
export function usePatternValidation({
pesData,
machineInfo,
patternOffset,
patternRotation,
}: UsePatternValidationParams): PatternBoundsCheckResult {
// Use the computed selector from the store for validation
// The store selector uses the current state (patternOffset, patternRotation)
const validationFromStore = usePatternValidationFromStore(
machineInfo
? { maxWidth: machineInfo.maxWidth, maxHeight: machineInfo.maxHeight }
: null,
);
// Memoize the result to avoid unnecessary recalculations
// Memoize the bounds check calculation to avoid unnecessary recalculations
return useMemo((): PatternBoundsCheckResult => {
if (!pesData || !machineInfo) {
return { fits: true, error: null };
}
// Use the validation from store which already has all the logic
return {
fits: validationFromStore.fits,
error: validationFromStore.error,
};
}, [
pesData,
machineInfo,
validationFromStore.fits,
validationFromStore.error,
]);
// Calculate rotated bounds if rotation is applied
let bounds = pesData.bounds;
if (patternRotation && patternRotation !== 0) {
bounds = calculateRotatedBounds(pesData.bounds, patternRotation);
}
const { maxWidth, maxHeight } = machineInfo;
// The patternOffset represents the pattern's CENTER position (due to offsetX/offsetY in canvas)
// So we need to calculate bounds relative to the center
const center = calculatePatternCenter(bounds);
// Calculate actual bounds in world coordinates
const patternMinX = patternOffset.x - center.x + bounds.minX;
const patternMaxX = patternOffset.x - center.x + bounds.maxX;
const patternMinY = patternOffset.y - center.y + bounds.minY;
const patternMaxY = patternOffset.y - center.y + bounds.maxY;
// Hoop bounds (centered at origin)
const hoopMinX = -maxWidth / 2;
const hoopMaxX = maxWidth / 2;
const hoopMinY = -maxHeight / 2;
const hoopMaxY = maxHeight / 2;
// Check if pattern exceeds hoop bounds
const exceedsLeft = patternMinX < hoopMinX;
const exceedsRight = patternMaxX > hoopMaxX;
const exceedsTop = patternMinY < hoopMinY;
const exceedsBottom = patternMaxY > hoopMaxY;
if (exceedsLeft || exceedsRight || exceedsTop || exceedsBottom) {
const directions = [];
if (exceedsLeft)
directions.push(
`left by ${((hoopMinX - patternMinX) / 10).toFixed(1)}mm`,
);
if (exceedsRight)
directions.push(
`right by ${((patternMaxX - hoopMaxX) / 10).toFixed(1)}mm`,
);
if (exceedsTop)
directions.push(
`top by ${((hoopMinY - patternMinY) / 10).toFixed(1)}mm`,
);
if (exceedsBottom)
directions.push(
`bottom by ${((patternMaxY - hoopMaxY) / 10).toFixed(1)}mm`,
);
return {
fits: false,
error: `Pattern exceeds hoop bounds: ${directions.join(", ")}. Adjust pattern position in preview.`,
};
}
return { fits: true, error: null };
}, [pesData, machineInfo, patternOffset, patternRotation]);
}

101
src/stores/usePatternStore.test.ts
View file

@ -5,7 +5,6 @@ import {
selectUploadedPatternCenter,
selectRotatedBounds,
selectRotationCenterShift,
selectPatternValidation,
} from "./usePatternStore";
import type { PesPatternData } from "../formats/import/pesImporter";
@ -198,104 +197,4 @@ describe("usePatternStore selectors", () => {
expect(shift!.y).toBeCloseTo(0, 0);
});
});
describe("selectPatternValidation", () => {
const machineInfo = { maxWidth: 1000, maxHeight: 800 };
it("should return fits=true when no pattern", () => {
usePatternStore.setState({ pesData: null });
const state = usePatternStore.getState();
const result = selectPatternValidation(state, machineInfo);
expect(result.fits).toBe(true);
expect(result.error).toBeNull();
});
it("should return fits=true when no machine info", () => {
const state = usePatternStore.getState();
const result = selectPatternValidation(state, null);
expect(result.fits).toBe(true);
expect(result.error).toBeNull();
});
it("should return fits=true when pattern fits in hoop", () => {
// Pattern bounds: -100 to 100 (200 wide), -50 to 50 (100 high)
// Hoop: 1000 wide, 800 high (centered at origin)
const state = usePatternStore.getState();
const result = selectPatternValidation(state, machineInfo);
expect(result.fits).toBe(true);
expect(result.error).toBeNull();
});
it("should detect when pattern exceeds hoop bounds", () => {
// Create a pattern that's too large
const pesData = createMockPesData({
minX: -600,
maxX: 600,
minY: -500,
maxY: 500,
});
usePatternStore.setState({ pesData });
const state = usePatternStore.getState();
const result = selectPatternValidation(state, machineInfo);
expect(result.fits).toBe(false);
expect(result.error).not.toBeNull();
expect(result.error).toContain("exceeds hoop bounds");
});
it("should account for pattern offset when validating", () => {
// Pattern bounds: -100 to 100 (200 wide), -50 to 50 (100 high)
// Hoop: 1000 wide (-500 to 500), 800 high (-400 to 400)
// Pattern fits, but when offset by 450, max edge is at 550 (exceeds 500)
usePatternStore.getState().setPatternOffset(450, 0);
const state = usePatternStore.getState();
const result = selectPatternValidation(state, machineInfo);
expect(result.fits).toBe(false);
expect(result.error).toContain("right");
});
it("should account for rotation when validating", () => {
// Pattern that fits normally but exceeds when rotated 45°
const pesData = createMockPesData({
minX: -450,
maxX: 450,
minY: -50,
maxY: 50,
});
usePatternStore.setState({ pesData });
usePatternStore.getState().setPatternRotation(45);
const state = usePatternStore.getState();
const result = selectPatternValidation(state, machineInfo);
// After 45° rotation, the bounds expand and may exceed
expect(result).toBeDefined();
});
it("should provide detailed error messages with directions", () => {
// Pattern that definitely exceeds on the left side
// Hoop: -500 to 500 (X), -400 to 400 (Y)
// Pattern with minX at -600 and maxX at 600 will exceed both bounds
const pesData = createMockPesData({
minX: -600,
maxX: 600,
minY: -50,
maxY: 50,
});
usePatternStore.setState({ pesData });
const state = usePatternStore.getState();
const result = selectPatternValidation(state, machineInfo);
expect(result.fits).toBe(false);
expect(result.error).toContain("left");
expect(result.error).toContain("mm");
});
});
});

96
src/stores/usePatternStore.ts
View file

@ -50,17 +50,6 @@ export interface TransformedBounds {
center: PatternCenter;
}
export interface PatternValidationResult {
fits: boolean;
error: string | null;
worldBounds: {
minX: number;
maxX: number;
minY: number;
maxY: number;
} | null;
}
export const usePatternStore = create<PatternState>((set) => ({
// Initial state - original pattern
pesData: null,
@ -219,80 +208,6 @@ export const selectRotationCenterShift = (
};
};
/**
* Select pattern validation against machine hoop bounds
* Returns whether pattern fits and error message if not
*/
export const selectPatternValidation = (
state: PatternState,
machineInfo: { maxWidth: number; maxHeight: number } | null,
): PatternValidationResult => {
if (!state.pesData || !machineInfo) {
return { fits: true, error: null, worldBounds: null };
}
// Get rotated bounds
const transformedBounds = selectRotatedBounds(state);
if (!transformedBounds) {
return { fits: true, error: null, worldBounds: null };
}
const { bounds, center } = transformedBounds;
const { maxWidth, maxHeight } = machineInfo;
// Calculate actual bounds in world coordinates
// The patternOffset represents the pattern's CENTER position (due to offsetX/offsetY in canvas)
const patternMinX = state.patternOffset.x - center.x + bounds.minX;
const patternMaxX = state.patternOffset.x - center.x + bounds.maxX;
const patternMinY = state.patternOffset.y - center.y + bounds.minY;
const patternMaxY = state.patternOffset.y - center.y + bounds.maxY;
const worldBounds = {
minX: patternMinX,
maxX: patternMaxX,
minY: patternMinY,
maxY: patternMaxY,
};
// Hoop bounds (centered at origin)
const hoopMinX = -maxWidth / 2;
const hoopMaxX = maxWidth / 2;
const hoopMinY = -maxHeight / 2;
const hoopMaxY = maxHeight / 2;
// Check if pattern exceeds hoop bounds
const exceedsLeft = patternMinX < hoopMinX;
const exceedsRight = patternMaxX > hoopMaxX;
const exceedsTop = patternMinY < hoopMinY;
const exceedsBottom = patternMaxY > hoopMaxY;
if (exceedsLeft || exceedsRight || exceedsTop || exceedsBottom) {
const directions = [];
if (exceedsLeft)
directions.push(
`left by ${((hoopMinX - patternMinX) / 10).toFixed(1)}mm`,
);
if (exceedsRight)
directions.push(
`right by ${((patternMaxX - hoopMaxX) / 10).toFixed(1)}mm`,
);
if (exceedsTop)
directions.push(`top by ${((hoopMinY - patternMinY) / 10).toFixed(1)}mm`);
if (exceedsBottom)
directions.push(
`bottom by ${((patternMaxY - hoopMaxY) / 10).toFixed(1)}mm`,
);
return {
fits: false,
error: `Pattern exceeds hoop bounds: ${directions.join(", ")}. Adjust pattern position in preview.`,
worldBounds,
};
}
return { fits: true, error: null, worldBounds };
};
/**
* Hook to get pattern center (memoized with shallow comparison)
*/
@ -311,17 +226,6 @@ export const useUploadedPatternCenter = () =>
export const useRotatedBounds = () =>
usePatternStore(useShallow(selectRotatedBounds));
/**
* Hook to get pattern validation result (requires machineInfo)
* Uses shallow comparison to prevent infinite re-renders from new object references
*/
export const usePatternValidationFromStore = (
machineInfo: { maxWidth: number; maxHeight: number } | null,
) =>
usePatternStore(
useShallow((state) => selectPatternValidation(state, machineInfo)),
);
// Subscribe to pattern deleted event.
// This subscription is intended to persist for the lifetime of the application,
// so the unsubscribe function returned by `onPatternDeleted` is intentionally