Skip to content

fix: type _entities as EntityObserver[] in component inspectors#1886

Merged
willeastcott merged 9 commits intomainfrom
fix/entity-observer-type
Mar 5, 2026
Merged

fix: type _entities as EntityObserver[] in component inspectors#1886
willeastcott merged 9 commits intomainfrom
fix/entity-observer-type

Conversation

@willeastcott
Copy link
Contributor

@willeastcott willeastcott commented Mar 5, 2026

Summary

Types _entities as EntityObserver[] instead of Observer[] across all component inspectors and their parent EntityInspector. EntityObserver (defined in editor-api/entity.ts) extends Observer with entity-specific properties like .entity, .apiEntity, .latestFn, and .history that the inspectors already use at runtime.

  • Changes _entities and link() in EntityInspector from Observer[] to EntityObserver[], fixing the type mismatch at the call sites that pass into ComponentInspector.link()
  • Changes _entities declaration in base ComponentInspector from Observer[] | null to EntityObserver[] | null
  • Updates link(entities) parameter type from Observer[] to EntityObserver[] in all 24 inspector subclasses
  • Updates _entities field in SoundSlotInspector and SpriteClipInspector to EntityObserver[] | null for consistency
  • Updates remaining Observer[] parameter types to EntityObserver[] in helper functions and methods (e.g. animation.ts, sprite.ts)
  • Each subclass imports EntityObserver directly from @/editor-api rather than re-exporting through component.ts
  • Removes unused Observer imports from @playcanvas/observer in files where it was only used for the link() parameter type

@vercel
Copy link

vercel bot commented Mar 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
editor Ready Ready Preview, Comment Mar 5, 2026 10:03am

Request Review

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves TypeScript type safety in the editor's component inspector layer by replacing the loosely-typed Observer[] with the more specific EntityObserver[] for the _entities field and link() method signatures across all component inspectors. EntityObserver (defined in src/editor-api/entity.ts) extends Observer with entity-specific properties (.entity, .apiEntity, .latestFn, .history) that the inspectors already use at runtime, so this change aligns the type definitions with actual usage.

Changes:

  • Updates _entities declaration and link() parameter in the base ComponentInspector from Observer[] to EntityObserver[]
  • Updates link() parameter types in all 24 component/panel inspector subclasses to use EntityObserver[]
  • Removes now-unused Observer imports from 12 files where it was only used for the link() parameter

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
component.ts Updates base ComponentInspector._entities and link() to use EntityObserver[]; imports EntityObserver from @/editor-api
anim.ts Updates link() to use EntityObserver[]; removes unused Observer import
animation.ts Updates link() and 4 helper methods to use EntityObserver[]; removes Observer from @playcanvas/observer import
audiolistener.ts Replaces Observer import with EntityObserver; updates link()
audiosource.ts Replaces Observer import with EntityObserver; updates link()
button.ts Removes Observer from imports; adds EntityObserver; updates link()
camera.ts Replaces Observer import with EntityObserver; updates link()
collision.ts Adds EntityObserver; retains Observer (still needed for _handleTypeChange); updates link()
element.ts Adds EntityObserver to import; retains Observer (still used by other helpers); updates link()
gsplat.ts Replaces Observer import with EntityObserver; updates link()
layoutchild.ts Replaces Observer import with EntityObserver; updates link()
layoutgroup.ts Replaces Observer import with EntityObserver; updates link()
light.ts Removes Observer from imports; adds EntityObserver; updates link() and _updateShadows()
model.ts Removes Observer from imports; adds EntityObserver; updates link(), _getMeshInstanceName(), and _createMappingInspector()
particlesystem.ts Replaces Observer import with EntityObserver; updates link()
render.ts Removes Observer from imports; adds EntityObserver; updates link()
rigidbody.ts Replaces Observer import with EntityObserver; updates link()
screen.ts Replaces Observer import with EntityObserver; updates link()
script.ts Adds EntityObserver to import; updates ScriptInspector._entities, link(), and ScriptComponentInspector.link()
scrollbar.ts Replaces Observer import with EntityObserver; updates link()
scrollview.ts Replaces Observer import with EntityObserver; updates link()
sound.ts Adds EntityObserver to import; updates SoundSlotInspector.link() and SoundComponentInspector.link() — but misses updating SoundSlotInspector._entities field type
sprite.ts Adds EntityObserver to import; updates SpriteClipInspector.link() and SpriteComponentInspector.link() — but misses updating SpriteClipInspector._entities field type
zone.ts Replaces Observer import with EntityObserver; updates link()
Comments suppressed due to low confidence (1)

src/editor/inspector/components/component.ts:281

  • The PR description states that EntityObserver is re-exported from component.ts for convenient access by subclasses, but this re-export is not present in the code. The export statement in component.ts only exports ComponentInspector and ComponentInspectorArgs. All subclasses instead import EntityObserver directly from @/editor-api.

If re-exporting from component.ts was intended, the export should be added as: export { ComponentInspector, type ComponentInspectorArgs, type EntityObserver }. Otherwise, the PR description should be corrected.

    link(entities: EntityObserver[]) {
        this.unlink();
        this._entities = entities;

        const path = `components.${this._component}.enabled`;
        this._fieldEnable.link(entities, path);
    }

    unlink() {
        this._fieldEnable.unlink();

        this._entityEvents.forEach(e => e.unbind());
        this._entityEvents.length = 0;
        this._entities = null;
    }

    destroy() {
        if (this._destroyed) {
            return;
        }

        if (this._templateOverridesInspector) {
            this._templateOverridesInspector.unregisterElementForPath(`components.${this._component}`);
            this._templateOverridesInspector.unregisterElementForPath(`components.${this._component}.enabled`);
        }

        this._contextMenu.destroy();

        super.destroy();
    }
}

export { ComponentInspector, type ComponentInspectorArgs };

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

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.


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

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.


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

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 25 out of 25 changed files in this pull request and generated no new comments.


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

You can also share your feedback on Copilot code review. Take the survey.

@willeastcott willeastcott merged commit f27f814 into main Mar 5, 2026
11 checks passed
@willeastcott willeastcott deleted the fix/entity-observer-type branch March 5, 2026 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants