Skip to content

Commit 91fac3a

Browse files
authored
Enhance property handling in validation utility (#228)
* Enhance property handling in validation utility - Updated `getPseudoPublicProps` to return property information including whether properties are defined with setters. - Introduced `isDefinedWithSetter` function to check if a property is defined using a setter. - Modified `createComponentDefinition` to utilize the new property information for applying values based on setter presence. * Fix prop assignment for components with setters in @playcanvas/react * fix(validation): update prop assignment for Vec2, Vec3, Vec4, and Quat to use fromArray method - Modified the apply function in createComponentDefinition to utilize the fromArray method for Vec2, Vec3, Vec4, and Quat, ensuring proper assignment of properties when defined with setters.
1 parent 8138b6a commit 91fac3a

File tree

2 files changed

+92
-38
lines changed

2 files changed

+92
-38
lines changed

.changeset/happy-groups-burn.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@playcanvas/react": patch
3+
---
4+
5+
Fixes prop assignment for components with setters

packages/lib/src/utils/validation.ts

Lines changed: 87 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -235,51 +235,91 @@ export function applyProps<T extends Record<string, unknown>, InstanceType>(inst
235235

236236

237237
/**
238-
* Get the pseudo public props of an instance. This is useful for creating a component definition from an instance.
238+
* Property information including whether it's defined with a setter.
239+
*/
240+
export type PropertyInfo = {
241+
value: unknown;
242+
isDefinedWithSetter: boolean;
243+
};
244+
245+
/**
246+
* Get the pseudo public props of an instance with setter information. This is useful for creating a component definition from an instance.
239247
* @param container The container to get the pseudo public props from.
240-
* @returns The pseudo public props of the container.
248+
* @returns The pseudo public props of the container with setter flags.
241249
*/
242-
export function getPseudoPublicProps(container: Record<string, unknown>) {
250+
export function getPseudoPublicProps(container: Record<string, unknown>): Record<string, PropertyInfo> {
251+
const result: Record<string, PropertyInfo> = {};
252+
243253
// Get regular enumerable properties
244254
const entries = Object.entries(container)
245255
.filter(([key]) => !key.startsWith('_') && typeof container[key] !== 'function');
246256

257+
// Add regular properties (not defined with setters)
258+
entries.forEach(([key, value]) => {
259+
result[key] = {
260+
value,
261+
isDefinedWithSetter: false
262+
};
263+
});
264+
247265
// Get getters and setters from the prototype
248266
const prototype = Object.getPrototypeOf(container);
249267
if (prototype && prototype !== Object.prototype) {
250268
const descriptors = Object.getOwnPropertyDescriptors(prototype);
251-
const getterSetterEntries = Object.entries(descriptors)
252-
.filter(([key, descriptor]) => {
253-
// Include properties that have setters (and optionally getters) and don't start with _
254-
return (descriptor.get && descriptor.set) &&
255-
!key.startsWith('_') &&
256-
key !== 'constructor';
257-
})
258-
.map(([key, descriptor]) => {
259-
// For properties with both getter and setter, try to get the value if possible
260-
if (descriptor.get) {
261-
try {
262-
const value = descriptor.get.call(container);
263-
// Create a shallow copy of the value to avoid reference issues
264-
const safeValue = value !== null && typeof value === 'object'
265-
? value.clone ? value.clone() : { ...value }
266-
: value;
267-
return [key, safeValue];
268-
} catch {
269-
// If we can't get the value, just use the key
270-
return [key, undefined];
271-
}
272-
}
273-
// For setters only, we can't get the value
274-
return [key, undefined];
275-
});
276269

277-
// Combine regular properties with getter/setter properties
278-
return Object.fromEntries([...entries, ...getterSetterEntries]);
270+
Object.entries(descriptors).forEach(([key, descriptor]) => {
271+
// Skip private properties and constructor
272+
if (key.startsWith('_') || key === 'constructor') return;
273+
274+
// Check if this property has a setter
275+
const hasSetter = descriptor.set !== undefined;
276+
277+
// If it's a getter/setter property, try to get the value
278+
if (descriptor.get) {
279+
try {
280+
const value = descriptor.get.call(container);
281+
// Create a shallow copy of the value to avoid reference issues
282+
const safeValue = value !== null && typeof value === 'object'
283+
? value.clone ? value.clone() : { ...value }
284+
: value;
285+
286+
result[key] = {
287+
value: safeValue,
288+
isDefinedWithSetter: hasSetter
289+
};
290+
} catch {
291+
// If we can't get the value, just use undefined
292+
result[key] = {
293+
value: undefined,
294+
isDefinedWithSetter: hasSetter
295+
};
296+
}
297+
} else if (hasSetter) {
298+
// Setter-only property
299+
result[key] = {
300+
value: undefined,
301+
isDefinedWithSetter: true
302+
};
303+
}
304+
});
279305
}
280306

281-
// If no prototype or it's just Object.prototype, just return the regular properties
282-
return Object.fromEntries(entries);
307+
return result;
308+
}
309+
310+
/**
311+
* Check if a specific property is defined using a setter (like `set prop(){}`).
312+
* @param container The container to check.
313+
* @param propName The property name to check.
314+
* @returns True if the property is defined using a setter, false otherwise.
315+
*/
316+
export function isDefinedWithSetter(container: Record<string, unknown>, propName: string): boolean {
317+
const prototype = Object.getPrototypeOf(container);
318+
if (prototype && prototype !== Object.prototype) {
319+
const descriptor = Object.getOwnPropertyDescriptor(prototype, propName);
320+
return descriptor?.set !== undefined;
321+
}
322+
return false;
283323
}
284324

285325
/**
@@ -300,10 +340,11 @@ export function createComponentDefinition<T, InstanceType>(
300340
const instance: InstanceType = createInstance();
301341
const schema: Schema<T, InstanceType> = {};
302342
const props = getPseudoPublicProps(instance as Record<string, unknown>);
303-
const entries = Object.entries(props) as [keyof T, unknown][];
343+
const entries = Object.entries(props) as [keyof T, PropertyInfo][];
304344

305345
// Basic type detection
306-
entries.forEach(([key, value]) => {
346+
entries.forEach(([key, propertyInfo]) => {
347+
const { value, isDefinedWithSetter } = propertyInfo;
307348

308349
// Colors
309350
if (value instanceof Color) {
@@ -329,7 +370,9 @@ export function createComponentDefinition<T, InstanceType>(
329370
default: [value.x, value.y],
330371
errorMsg: (val) => `Invalid value for prop "${String(key)}": "${JSON.stringify(val)}". ` +
331372
`Expected an array of 2 numbers.`,
332-
apply: (instance, props, key) => {
373+
apply: isDefinedWithSetter ? (instance, props, key) => {
374+
(instance[key as keyof InstanceType] as Vec2).fromArray(props[key] as number[]);
375+
} : (instance, props, key) => {
333376
(instance[key as keyof InstanceType] as Vec2).set(...props[key] as [number, number]);
334377
}
335378
};
@@ -341,7 +384,9 @@ export function createComponentDefinition<T, InstanceType>(
341384
default: [value.x, value.y, value.z],
342385
errorMsg: (val) => `Invalid value for prop "${String(key)}": "${JSON.stringify(val)}". ` +
343386
`Expected an array of 3 numbers.`,
344-
apply: (instance, props, key) => {
387+
apply: isDefinedWithSetter ? (instance, props, key) => {
388+
(instance[key as keyof InstanceType] as Vec3).fromArray(props[key] as number[]);
389+
} : (instance, props, key) => {
345390
(instance[key as keyof InstanceType] as Vec3).set(...props[key] as [number, number, number]);
346391
}
347392
};
@@ -352,7 +397,9 @@ export function createComponentDefinition<T, InstanceType>(
352397
validate: (val) => Array.isArray(val) && val.length === 4,
353398
default: [value.x, value.y, value.z, value.w],
354399
errorMsg: (val) => `Invalid value for prop "${String(key)}": "${JSON.stringify(val)}". Expected an array of 4 numbers.`,
355-
apply: (instance, props, key) => {
400+
apply: isDefinedWithSetter ? (instance, props, key) => {
401+
(instance[key as keyof InstanceType] as Vec4).fromArray(props[key] as number[]);
402+
} : (instance, props, key) => {
356403
(instance[key as keyof InstanceType] as Vec4).set(...props[key] as [number, number, number, number]);
357404
}
358405
};
@@ -365,7 +412,9 @@ export function createComponentDefinition<T, InstanceType>(
365412
default: [value.x, value.y, value.z, value.w],
366413
errorMsg: (val) => `Invalid value for prop "${String(key)}": "${JSON.stringify(val)}". ` +
367414
`Expected an array of 4 numbers.`,
368-
apply: (instance, props, key) => {
415+
apply: isDefinedWithSetter ? (instance, props, key) => {
416+
(instance[key as keyof InstanceType] as Quat).fromArray(props[key] as number[]);
417+
} : (instance, props, key) => {
369418
(instance[key as keyof InstanceType] as Quat).set(...props[key] as [number, number, number, number]);
370419
}
371420
};

0 commit comments

Comments
 (0)