Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 53 additions & 21 deletions packages/lib/src/utils/compare.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

/**
* Simple deep equality check for two objects
* @param {Record<string, unknown>} a - The first object
Expand All @@ -16,23 +15,61 @@ export function deepEqual(a: Record<string, unknown>, b: Record<string, unknown>
return aKeys.every(key => deepEqual(a[key] as Record<string, unknown>, b[key] as Record<string, unknown>));
}

type Equalable = {
equals: (other: unknown) => boolean;
};

function hasEqualsMethod(obj: unknown): obj is Equalable {
return typeof obj === 'object' &&
obj !== null &&
'equals' in obj &&
typeof (obj as Equalable).equals === 'function';
}

export const shallowEquals = (objA: Record<string, unknown>, objB: Record<string, unknown>): boolean => {
interface Approximate {
equalsApprox(other: unknown): boolean;
}

interface Equatable {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think Equatable is quite right... Exact maybe?

equals(other: unknown): boolean;
}

/**
* Check if two values are equal. Handles primitives, null/undefined,
* and objects with `equalsApprox` or `equals` methods (Vec3, Color, etc.)
*
* Priority order:
* 1. Strict equality (===)
* 2. Floating point approximation (equalsApprox) - handles precision drift
* 3. Structural equality (equals)
*
* @param a - First value to compare
* @param b - Second value to compare
* @returns True if values are equal, false otherwise
*/
export function valuesEqual(a: unknown, b: unknown): boolean {
if (a === b) return true;

// Early exit if either is null/undefined (using type coercion)
if (a == null || b == null) return false;

if (typeof a === 'object') {
// Priority 1: Floating point approximation (handles precision drift)
if ('equalsApprox' in a && typeof (a as Approximate).equalsApprox === 'function') {
return (a as Approximate).equalsApprox(b);
}

// Priority 2: Strict structural equality
if ('equals' in a && typeof (a as Equatable).equals === 'function') {
return (a as Equatable).equals(b);
}
}

// For other objects, return false to trigger re-apply (conservative)
return false;
}
Comment on lines +39 to +59
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new valuesEqual function lacks test coverage. Given the critical nature of this function (it's used to prevent unnecessary prop applications and avoid side effects), it should have comprehensive tests covering:

  • Primitive comparisons (numbers, strings, booleans, null, undefined)
  • Object comparisons using equalsApprox (Vec2, Vec3, Vec4, Color, Quat)
  • Object comparisons using equals
  • Edge cases like comparing objects with arrays

Consider adding a test file similar to picker.test.tsx to ensure this function behaves correctly in all scenarios.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion


/**
* Shallow equality check for two objects. Compares each property using valuesEqual.
*
* @param objA - First object to compare
* @param objB - Second object to compare
* @returns True if objects are shallowly equal, false otherwise
*/
export const shallowEquals = (objA: Record<string, unknown>, objB: Record<string, unknown>): boolean => {
// If the two objects are the same object, return true
if (objA === objB) {
return true;
}


// If either is not an object (null or primitives), return false
if (typeof objA !== 'object' || objA === null || typeof objB !== 'object' || objB === null) {
Expand All @@ -48,15 +85,10 @@ type Equalable = {
return false;
}

// Check if all keys and their values are equal
// Check if all keys and their values are equal using valuesEqual
for (let i = 0; i < keysA.length; i++) {
const key = keysA[i];
const propA = objA[key];
const propB = objB[key];
// If the object has an equality operator, use this
if(hasEqualsMethod(propA)) {
return propA.equals(propB);
} else if (propA !== propB) {
if (!valuesEqual(objA[key], objB[key])) {
return false;
}
}
Expand Down
9 changes: 9 additions & 0 deletions packages/lib/src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Color, Quat, Vec2, Vec3, Vec4, Mat4, Application, NullGraphicsDevice, M
import { getColorFromName } from "./color.ts";
import { Serializable } from "./types-utils.ts";
import { env } from "./env.ts";
import { valuesEqual } from "./compare.tsx";

// Limit the size of the warned set to prevent memory leaks
const MAX_WARNED_SIZE = 1000;
Expand Down Expand Up @@ -218,6 +219,7 @@ export function validatePropsWithDefaults<T extends object, InstanceType>(
* @param schema The schema of the container
* @param props The props to apply
*/

export function applyProps<T extends Record<string, unknown>, InstanceType>(
instance: InstanceType,
schema: Schema<T, InstanceType>,
Expand All @@ -227,6 +229,13 @@ export function applyProps<T extends Record<string, unknown>, InstanceType>(
if (key in schema) {
const propDef = schema[key as keyof T] as PropValidator<T[keyof T], InstanceType>;
if (propDef) {
const currentValue = (instance as Record<string, unknown>)[key];

// Skip if value hasn't changed (avoids side effects from setters)
if (valuesEqual(currentValue, value)) {
return;
}
Comment on lines +232 to +237
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The equality check compares the current instance property value with the new prop value, but this comparison may be incorrect when a custom apply function exists. The apply function often transforms the input (e.g., an array [1, 2, 3] gets transformed into a Vec3 object). This means you're comparing a Vec3 object with an array, which will always return false and the optimization won't work.

Consider checking equality after the transformation, or store the last applied raw value to compare against the new raw value before transformation.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is a good suggestion, will try to rework this


if (propDef.apply) {
// Use type assertion to satisfy the type checker
propDef.apply(instance, props, key as string);
Expand Down
Loading