refactor(components): drive cloneComponent from shared _properties lists#8879
Merged
Conversation
Button, scroll-view, scrollbar and collision systems already define a module-level _properties array used by initializeComponentData, but each duplicated the same list as a hand-written object literal in cloneComponent. Build the clone data from the shared list instead (the camera pattern from #8878) so the two paths can no longer drift. Collision keeps its two intentional differences explicit: type is initialized outside the property list, and shape is excluded because the type implementation creates it during initialization. No behavior changes - all four clone literals matched their lists 1:1. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors several component systems so cloneComponent derives its clone data from the same module-level _properties list used by initializeComponentData, reducing drift risk and keeping clone/init behavior consistent across UI + collision components.
Changes:
- Updated
button,scroll-view, andscrollbarsystems to build clone data by iterating_properties(plusenabled). - Updated
collisionsystem clone to iterate_propertieswhile explicitly handlingtypeand intentionally skippingshape.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/framework/components/button/system.js |
Builds clone payload from _properties to match initializer behavior and avoid list drift. |
src/framework/components/scroll-view/system.js |
Builds clone payload from _properties while preserving the initializer’s property ordering assumptions. |
src/framework/components/scrollbar/system.js |
Builds clone payload from _properties for consistency with initialization. |
src/framework/components/collision/system.js |
Builds clone payload from _properties, explicitly includes type, and intentionally skips cloning shape. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
willeastcott
added a commit
that referenced
this pull request
Jun 11, 2026
…body (#8880) Continue the pattern from #8878/#8879: hoist the inline property list in initializeComponentData to a module-level _properties array and drive cloneComponent from the same list, so the two paths cannot drift. Special-case handling is preserved: - model: material/materialAsset are still resolved by the dedicated clone logic, mapping is still shallow-copied via extend - sound: slots are still converted back to plain option objects - rigid-body: linearFactor/angularFactor now pass the component Vec3s directly instead of converting to arrays; the component setters copy values, so the end state is identical with no aliasing Also drops the unused legacy properties parameter from the initializeComponentData overrides. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jun 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second PR in the component-consistency series (follows #8878).
Button, scroll-view, scrollbar and collision systems already define a module-level
_propertiesarray used byinitializeComponentData, but each duplicated the same list as a hand-written object literal incloneComponent— the exact drift hazard that bit the camera system in #8878. This PR builds the clone data from the shared list instead.Behavior-preserving — before converting, each clone literal was verified to match its
_propertieslist 1:1:enabled, exact match.enabled, exact match (init order, which is what matters, is unchanged).enabled, exact match.typeis initialized outside the property list (written to the backing field before the list is applied), andshapeis skipped because the type implementation creates it during initialization and must not be shared with the clone.Clone value semantics are unchanged: the loop passes the same references the literals passed (component setters handle copying, covered by the existing Color/Vec input-cloning tests).
Testing
#cloneComponenttests (clone round-trips, entity-ref remapping) — 95 tests pass unchanged, no test edits needednpm testsuite passes (1817 passing)Remaining PRs in the series
beforeremovehandler naming across all systems🤖 Generated with Claude Code