Fix: Support custom asset loading for GSplat (getter-only props)

[No-LAN-B]

Problem (Addresses Issue - #315 )

Using with a custom-loaded asset (e.g. from a @playcanvas/splat-transform loader) throws:

Cannot set property id of #<GSplatComponent> which has only a getter
  at createComponentDefinition (validation.ts)

The same pattern can affect any component that exposes getter-only props (e.g. id).

Root cause

• 1.) getPseudoPublicProps includes getter-only properties (e.g. GSplatComponent.id) in the props used to build the schema.
• 2.) createComponentDefinition then adds a schema entry for each of those props with a default.
• 3.) validatePropsWithDefaults / validatePropsPartial fill validated props from the schema, so read-only props like id are always present with a default.
• 4.) applyProps assigns every validated prop to the instance. Assigning to a getter-only property throws, so the flow fails even when the user never sets id.

So the failure comes from the generic “apply all schema props” path trying to assign to getter-only props, not from the custom asset loading path itself.

Solution

• Detect getter-only in createComponentDefinition using the property descriptor (get without set) and mark them as read-only.
• Schema entries without apply (number, string, boolean, Mat4, Material): set readOnly: true on the schema entry when the prop is getter-only.
• Schema entries with apply (Color, Vec2, Vec3, Vec4, Quat, Arrays, Null): keep the existing apply callbacks and add an early return when the prop is read-only so they never assign.
• applyProps: skip when propDef is missing or propDef.readOnly is true, so we never call apply or direct-assign for read-only props.

Getter-only props stay in the schema (so the public API stays complete and anything that relies on them being present still works), but they are never applied, so the “Cannot set property id” error is fixed.

Changes

• PropValidator: optional readOnly?: boolean.
• applyProps: guard with if (!propDef || propDef.readOnly) return before applying.
• createComponentDefinition: detect getter-only via descriptor and set propertyInfo.readOnly; add ...(propertyInfo.readOnly && { readOnly: true }) to schema entries that have no apply (Mat4, number, string, boolean, Material); add if (propertyInfo.readOnly) return at the start of every apply callback for types that have apply.

Result

• Custom-loaded assets work with without throwing.
• No change for components that don’t expose getter-only props.
• Getter-only props remain in the schema for docs/API completeness and are simply skipped at apply time.

[changeset-bot]

⚠️ No Changeset found

Latest commit: d80fca7

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[Copilot]

Pull request overview

This PR fixes a runtime error ("Cannot set property id of # which has only a getter") that occurs when using <GSplat asset={customAsset} /> with a custom-loaded asset. The root cause is that getter-only properties (like GSplatComponent.id) were being included in the component schema and subsequently assigned during prop application. The fix introduces a readOnly flag on schema entries for getter-only properties, and guards against assignment in applyProps and inside individual apply callbacks.

Changes:

• Adds readOnly?: boolean to the PropValidator type and PropertyInfo type
• Adds getter-only detection in createComponentDefinition using property descriptors, and marks matching schema entries as readOnly
• Guards applyProps and individual apply callbacks against assigning to read-only props

---

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

[Copilot]

There is a spurious trailing blank line with leading whitespace at line 244, immediately after the closing brace of applyProps. This stray indented blank line is inconsistent with the rest of the file's formatting and should be removed.

Suggested change

[Copilot]

The new readOnly guard in applyProps and the getter-only detection in createComponentDefinition are the core of this fix but have no test coverage. Given that the codebase has component-level tests (e.g., Screen.test.tsx, Script.test.tsx) and the fix is non-trivial, a test should be added to verify that: (1) a component schema entry for a getter-only prop has readOnly: true (for primitive/Mat4/Material types), (2) applyProps does not throw or attempt to assign when a schema entry has readOnly: true, and (3) the original failing scenario (passing a custom asset to <GSplat />) works without throwing.

[Copilot]

The closing brace of the PropertyInfo type is indented with 2 spaces ( };) while the type body uses 4-space indentation. This is inconsistent with the rest of the file which uses 4-space indentation for type declarations. The closing brace should be at column 0, consistent with how PropValidator and Schema types are defined in this file.

Suggested change

	};
	};

[No-LAN-B]

I normally have a lint script run on commit so I must have simply forgot to run the linter explicitly here should be a simple linter run to adapt some of the inline consistency.

[Copilot]

For schema entries that have an apply callback (Color, Vec2, Vec3, Vec4, Quat, Array, Null), the readOnly flag is not set on the schema entry itself — unlike types without an apply callback (Mat4, number, string, boolean, Material) which correctly spread ...(propertyInfo.readOnly && { readOnly: true }).

This inconsistency means:

1. The outer guard in applyProps (if (!propDef || propDef.readOnly) return) will never skip these props at the top level even when the property is getter-only, because propDef.readOnly is undefined for these types.
2. Any external code inspecting propDef.readOnly to determine if a prop is read-only (e.g. for docs/tooling) will get incorrect results for getter-only Color, Vec2, Vec3, Vec4, Quat, Array, or Null props.

The early-return inside the apply callback does prevent the actual write, so no exception is thrown — but the approach is not consistent with the rest of the implementation. The schema entries for these types should also spread ...(propertyInfo.readOnly && { readOnly: true }) so propDef.readOnly is reliable as a public indicator of read-only status.

[No-LAN-B]

The main goal was to implement early returns as I did not want to disrupt any preexisting code that may have rely on the type failure to catch I couldn't find anything that did this but my aim was to make the change with as little potential ripples as possible. The early exit is the simplest and easiest fix besides simply skipping getter only properties but, I wanted to provide a read-only property implementation as a proof of concept for further development. For types without an apply callback implementing the read-only property for consistency made sense by doing a simple append to the schema as the previous logic was based off this universal 'apply all' idea which is flawed.

This boils down to 2 things:

• Firstly the segment of code addressing apply callbacks is the essential bugfix here and I can definitely see fully implementing the read-only property to the schema being beneficial. It likely would become a much bigger change that I am willing to undertake but wanted to wait for approval as insurance.

• Secondly the read-only being a public indicator would allow for both custom debugging / a more informative error message so rather than 'is getter only' pops up something like property is of read only or more informative messages involving class access can be provided through the property as a point of reference.

NOTE: Testing can and should use this read-only property especially for asset creation it is likely required to revisit some older tests and add some insurance guards. I ran the test module and everything passed quite nicely however it passing nicely also likely means that the testing scripts should be more strict with class access properties as support for flexibility of asset expression IMO should be a test in and of itself as the original engine addresses custom assets cleanly and as a wrapper for the engine should ensure that feature carries over.

[Copilot]

The getter-only detection in createComponentDefinition uses Object.getOwnPropertyDescriptor((instance as any).constructor.prototype, key), which only looks at the direct prototype of the constructor. If a getter-only property is inherited from a grandparent class (not defined on the immediate prototype but on a higher ancestor), this check will return undefined and the property will not be marked as readOnly.

Note that getPseudoPublicProps has the same limitation (only traverses one prototype level), so such a property would likely not be included in the schema at all — meaning the current code is consistent. However, this is a latent bug: if getPseudoPublicProps is ever extended to walk the full prototype chain, the getter-only detection here would need to be updated to match. Consider using a prototype-walking loop (similar to how getPseudoPublicProps could be extended) or using Object.getOwnPropertyDescriptor on each level of the prototype chain to be safe.

[No-LAN-B]

This was actually one concern with integration and why I decided to go with early exits at the beginning. I would personally suggest downstream making a test case for grandparent classes ( if its a point of interest ) and simply run it to detect any fault lines in the class structure for the future.

As of writing this there are ZERO explicit grandparent classes in the traditional inheritance sense for the schema generator. Instead, the current system uses a composition-based pattern with type definitions and factory functions. So this is a misguided concern and Copilot didn't really do its due diligence here.

[No-LAN-B]

I addressed copilots review with comments that illustrate some extra rationale. TLDR: All of copilots concerns were more aligned with consistency and I made a few suggestions in that regard as Copilot is a little bit out of scope in reference here.

Looking forward to seeing what becomes of this! I'd love to be able to abstract from the engine a bit more in my current implementation and I believe this should allow for a few cool features and implementations to be viable. :)