fix: mark live API getters readonly#8959
Conversation
Public API reportThis PR changes the public API surface (+89 / −89), per the docs' rules (@ignore / @Private / undocumented are excluded). Show API diff-AnimComponent.get layers(): AnimComponentLayer[]
+AnimComponent.get layers(): readonly AnimComponentLayer[]
-CameraComponent.get layers(): number[]
+CameraComponent.get layers(): readonly number[]
-CameraComponent.get rect(): Vec4
+CameraComponent.get rect(): Readonly<Vec4>
-CameraComponent.set layers(newValue: number[])
+CameraComponent.set layers(newValue: readonly number[])
-CameraComponent.set rect(value: Vec4)
+CameraComponent.set rect(value: Readonly<Vec4>)
-CollisionComponent.get angularOffset(): Quat
+CollisionComponent.get angularOffset(): Readonly<Quat>
-CollisionComponent.get halfExtents(): Vec3
+CollisionComponent.get halfExtents(): Readonly<Vec3>
-CollisionComponent.get linearOffset(): Vec3
+CollisionComponent.get linearOffset(): Readonly<Vec3>
-CollisionComponent.set angularOffset(arg: Quat)
+CollisionComponent.set angularOffset(arg: Readonly<Quat>)
-CollisionComponent.set halfExtents(arg: Vec3)
+CollisionComponent.set halfExtents(arg: Readonly<Vec3>)
-CollisionComponent.set linearOffset(arg: Vec3)
+CollisionComponent.set linearOffset(arg: Readonly<Vec3>)
-ElementComponent.get anchor(): number[] | Vec4
+ElementComponent.get anchor(): Readonly<Vec4>
-ElementComponent.get color(): Color
+ElementComponent.get color(): Readonly<Color>
-ElementComponent.get layers(): number[]
+ElementComponent.get layers(): readonly number[]
-ElementComponent.get margin(): Vec4
+ElementComponent.get margin(): Readonly<Vec4>
-ElementComponent.get pivot(): number[] | Vec2
+ElementComponent.get pivot(): Readonly<Vec2>
-ElementComponent.get rect(): Vec4
+ElementComponent.get rect(): Readonly<Vec4> | null
-ElementComponent.set anchor(value: number[] | Vec4)
+ElementComponent.set anchor(value: Readonly<Vec4>)
-ElementComponent.set color(arg: Color)
+ElementComponent.set color(arg: Readonly<Color>)
-ElementComponent.set layers(value: number[])
+ElementComponent.set layers(value: readonly number[])
-ElementComponent.set margin(value: Vec4)
+ElementComponent.set margin(value: Readonly<Vec4>)
-ElementComponent.set pivot(value: number[] | Vec2)
+ElementComponent.set pivot(value: Readonly<Vec2>)
-ElementComponent.set rect(arg: Vec4)
+ElementComponent.set rect(arg: Readonly<Vec4> | null)
-Entity.get children(): GraphNode[]
+Entity.get children(): readonly GraphNode[]
-Entity.get forward(): Vec3
+Entity.get forward(): Readonly<Vec3>
-Entity.get right(): Vec3
+Entity.get right(): Readonly<Vec3>
-Entity.get up(): Vec3
-Entity.getEulerAngles(): Vec3
-Entity.getLocalEulerAngles(): Vec3
-Entity.getLocalPosition(): Vec3
-Entity.getLocalRotation(): Quat
-Entity.getLocalScale(): Vec3
-Entity.getLocalTransform(): Mat4
-Entity.getPosition(): Vec3
-Entity.getRotation(): Quat
-Entity.getWorldTransform(): Mat4
+Entity.get up(): Readonly<Vec3>
+Entity.getEulerAngles(): Readonly<Vec3>
+Entity.getLocalEulerAngles(): Readonly<Vec3>
+Entity.getLocalPosition(): Readonly<Vec3>
+Entity.getLocalRotation(): Readonly<Quat>
+Entity.getLocalScale(): Readonly<Vec3>
+Entity.getLocalTransform(): Readonly<Mat4>
+Entity.getPosition(): Readonly<Vec3>
+Entity.getRotation(): Readonly<Quat>
+Entity.getWorldTransform(): Readonly<Mat4>
-GraphNode.get children(): GraphNode[]
+GraphNode.get children(): readonly GraphNode[]
-GraphNode.get forward(): Vec3
+GraphNode.get forward(): Readonly<Vec3>
-GraphNode.get right(): Vec3
+GraphNode.get right(): Readonly<Vec3>
-GraphNode.get up(): Vec3
-GraphNode.getEulerAngles(): Vec3
-GraphNode.getLocalEulerAngles(): Vec3
-GraphNode.getLocalPosition(): Vec3
-GraphNode.getLocalRotation(): Quat
-GraphNode.getLocalScale(): Vec3
-GraphNode.getLocalTransform(): Mat4
-GraphNode.getPosition(): Vec3
-GraphNode.getRotation(): Quat
-GraphNode.getWorldTransform(): Mat4
+GraphNode.get up(): Readonly<Vec3>
+GraphNode.getEulerAngles(): Readonly<Vec3>
+GraphNode.getLocalEulerAngles(): Readonly<Vec3>
+GraphNode.getLocalPosition(): Readonly<Vec3>
+GraphNode.getLocalRotation(): Readonly<Quat>
+GraphNode.getLocalScale(): Readonly<Vec3>
+GraphNode.getLocalTransform(): Readonly<Mat4>
+GraphNode.getPosition(): Readonly<Vec3>
+GraphNode.getRotation(): Readonly<Quat>
+GraphNode.getWorldTransform(): Readonly<Mat4>
-LightComponent.get color(): Color
+LightComponent.get color(): Readonly<Color>
-LightComponent.get layers(): number[]
+LightComponent.get layers(): readonly number[]
-LightComponent.set color(value: Color)
+LightComponent.set color(value: Readonly<Color>)
-LightComponent.set layers(value: number[])
+LightComponent.set layers(value: readonly number[])
-Material.get blendState(): BlendState
+Material.get blendState(): Readonly<BlendState>
-Material.set blendState(value: BlendState)
+Material.set blendState(value: Readonly<BlendState>)
-ModelComponent.get layers(): number[]
+ModelComponent.get layers(): readonly number[]
-ModelComponent.get mapping(): object
+ModelComponent.get mapping(): Readonly<Record<string, number>>
-ModelComponent.get meshInstances(): MeshInstance[] | null
+ModelComponent.get meshInstances(): readonly MeshInstance[] | null
-ModelComponent.set layers(value: number[])
+ModelComponent.set layers(value: readonly number[])
-ModelComponent.set mapping(value: object)
+ModelComponent.set mapping(value: Readonly<Record<string, number>>)
-ModelComponent.set meshInstances(value: MeshInstance[] | null)
+ModelComponent.set meshInstances(value: readonly MeshInstance[] | null)
-ParticleSystemComponent.get layers(): number[]
+ParticleSystemComponent.get layers(): readonly number[]
-ParticleSystemComponent.set layers(arg: number[])
+ParticleSystemComponent.set layers(arg: readonly number[])
-RenderComponent.get layers(): number[]
+RenderComponent.get layers(): readonly number[]
-RenderComponent.get meshInstances(): MeshInstance[]
+RenderComponent.get meshInstances(): readonly MeshInstance[]
-RenderComponent.set layers(value: number[])
+RenderComponent.set layers(value: readonly number[])
-RenderComponent.set meshInstances(value: MeshInstance[])
+RenderComponent.set meshInstances(value: readonly MeshInstance[])
-RigidBodyComponent.get angularFactor(): Vec3
-RigidBodyComponent.get angularVelocity(): Vec3
+RigidBodyComponent.get angularFactor(): Readonly<Vec3>
+RigidBodyComponent.get angularVelocity(): Readonly<Vec3>
-RigidBodyComponent.get linearFactor(): Vec3
-RigidBodyComponent.get linearVelocity(): Vec3
+RigidBodyComponent.get linearFactor(): Readonly<Vec3>
+RigidBodyComponent.get linearVelocity(): Readonly<Vec3>
-RigidBodyComponent.set angularFactor(factor: Vec3)
-RigidBodyComponent.set angularVelocity(velocity: Vec3)
+RigidBodyComponent.set angularFactor(factor: Readonly<Vec3>)
+RigidBodyComponent.set angularVelocity(velocity: Readonly<Vec3>)
-RigidBodyComponent.set linearFactor(factor: Vec3)
-RigidBodyComponent.set linearVelocity(velocity: Vec3)
+RigidBodyComponent.set linearFactor(factor: Readonly<Vec3>)
+RigidBodyComponent.set linearVelocity(velocity: Readonly<Vec3>)
-Scene.get skyboxRotation(): Quat
+Scene.get skyboxRotation(): Readonly<Quat>
-Scene.set skyboxRotation(value: Quat)
+Scene.set skyboxRotation(value: Readonly<Quat>)
-ScriptComponent.get scripts(): ScriptType[]
+ScriptComponent.get scripts(): readonly ScriptType[]
-ScriptComponent.set scripts(value: ScriptType[])
+ScriptComponent.set scripts(value: readonly ScriptType[])
-ShaderMaterial.get blendState(): BlendState
+ShaderMaterial.get blendState(): Readonly<BlendState>
-ShaderMaterial.set blendState(value: BlendState)
+ShaderMaterial.set blendState(value: Readonly<BlendState>)
-SoundComponent.get slots(): object
+SoundComponent.get slots(): Readonly<Record<string, SoundSlot>>
-SoundComponent.set slots(newValue: object)
+SoundComponent.set slots(newValue: Readonly<Record<string, SoundSlot>>)
-SpriteComponent.get color(): Color
+SpriteComponent.get color(): Readonly<Color>
-SpriteComponent.get layers(): number[]
+SpriteComponent.get layers(): readonly number[]
-SpriteComponent.set color(value: Color)
+SpriteComponent.set color(value: Readonly<Color>)
-SpriteComponent.set layers(value: number[])
+SpriteComponent.set layers(value: readonly number[])
-StandardMaterial.get blendState(): BlendState
+StandardMaterial.get blendState(): Readonly<BlendState>
-StandardMaterial.set blendState(value: BlendState)
+StandardMaterial.set blendState(value: Readonly<BlendState>)Informational only — this never fails the build. |
Build size reportThis PR does not change the size of the minified bundles.
|
4807f22 to
c427b00
Compare
mvaligursky
left a comment
There was a problem hiding this comment.
🤖 Automated PR review — posted on my behalf by Claude Code (Opus 4.8). Not a human review. The points below are suggestions to weigh as possible improvements, not changes that necessarily need to be addressed. (Reviewed retrospectively — this PR is already merged, so these are follow-up considerations.)
Doc/typings-only, no runtime change, and a genuine improvement. I confirmed the anchor/pivot getters narrowing to Readonly<Vec4>/Readonly<Vec2> is safe — their setters carry their own @type {... | number[]} JSDoc, so passing number[] to the setter still type-checks. Two things worth being aware of:
-
Readonly<T>only blocks property assignment, not mutating method calls. It stopsentity.getPosition().x = 1(the documented footgun), butentity.getPosition().add(v)/.set(...)/.copy(...)/.mulScalar(...)still type-check and will mutate the internal vector with no error. Same forReadonly<Quat>/Readonly<Mat4>/Readonly<Color>/Readonly<BlendState>. So the protection is partial — valuable for the most common mistake, but not a full guard. Worth a note in the issue if method-based mutation is a concern (a branded/opaque type would be needed to fully prevent it). -
This is a breaking change for TypeScript consumers.
Readonly<T>/ReadonlyArray<T>are not assignable back toT/T[], so existing downstream code that stores or forwards these return values into mutable-typed variables or parameters (e.g.const p: Vec3 = entity.getPosition()orfn(entity.render.meshInstances)wherefn(x: MeshInstance[])) will newly fail to compile — even when it only reads. That's the intended behavior for catching mutation, but it's worth calling out explicitly in the release notes so users aren't surprised by new type errors on upgrade.
| * Gets the array of layer IDs ({@link Layer#id}) to which this camera belongs. | ||
| * | ||
| * @type {number[]} | ||
| * @type {ReadonlyArray<number>} |
There was a problem hiding this comment.
What's the difference between using that and @readonly? We already use @readonly across the codebase.
There was a problem hiding this comment.
Different levels — one is about the reference, the other about the value.
@readonly marks the property/binding as non-reassignable. It's the JSDoc equivalent of TS's readonly modifier on a member:
/** @readonly */
this.position = new Vec3();
// entity.position = otherVec; ❌ can't rebind the slot
// entity.position.x = 1; ✅ still allowed — the Vec3's contents are mutable
It says nothing about whether the object it points at can be mutated.
Readonly (in @type) changes the value's type so all of T's properties become readonly:
/** @type {Readonly} */
get position() { ... }
// pos.x = 1; ❌ field is readonly
// (but pos.add(v), pos.set(...) ✅ still allowed — see below)
It says nothing about whether you can reassign the slot.
For this PR the goal was to stop entity.getPosition().x = 1 — i.e. mutating the contents of the returned object — so Readonly is the correct tool. @readonly wouldn't help here: these are getters with no setter, so the slot is already non-reassignable, and @readonly wouldn't touch .x.
Two extra nuances worth keeping in mind:
They're orthogonal, not alternatives — you could have both (a readonly field whose value is also Readonly). They guard different things.
ReadonlyArray is stronger than Readonly. TS defines ReadonlyArray specially, so it actually removes the mutating methods (push/pop/splice) from the type. But Readonly is just the generic mapped type — it makes .x/.y/.z readonly but leaves Vec3's mutating methods (.set, .add, .copy) fully callable. That's why the array surfaces in this PR are well-protected, but the Vec/Quat/Mat/Color ones are only partially protected (the point I raised in the review). Fully locking those down would need a branded/opaque type, not Readonly<> or @readonly.
* fix: mark live API getters readonly * test: remove type ergonomics check
Refs #8958
Description
Manual smoke test
entity.getPosition().x = 1orentity.render.meshInstances.push(meshInstance).Expected: the editor reports a readonly/type error for the direct mutation.
const position = entity.getPosition().clone(); position.x = 1; entity.setPosition(position);.Expected: the editor accepts the setter-based update.
Checklist