Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add isLight: true class property to Light.d.ts #16313

Merged
merged 1 commit into from
Apr 23, 2019

Conversation

kohlmannj
Copy link
Contributor

Description

This PR adds the instance property isLight: true to the TypeScript type definitions for the THREE.Light class.

Use Case

I have some code in a project which uses THREE.Scene.traverse() to determine if a given scene (loaded from glTF, in this case) contains light objects. See this Gist for the code: https://gist.github.com/kohlmannj/d64df72eb8d7309ff7120bc9743e95dd

Since the type definition for THREE.Light doesn’t define the isLight property, this TS code has an error:

scene.traverse((object: THREE.Object3D | THREE.Light) => {
  if ('isLight' in object) {
    console.log('Found light in scene', object);
    // This line has a TypeScript error in Three.js r103:
    object.isLight; // ERROR: Property 'isLight' does not exist on type 'never’. ts(2339)
  }
});

That is, I’m trying to use type narrowing to write a type-safe check that a given object is indeed a Light subclass.

The implementation of the Light class in Three.js r103 does indeed contain an isLight property, and I have previously used a non-type-safe runtime check like (object as any).isLight to successfully identify objects that are lights.

Therefore, based on the actual implementation, I believe it’s appropriate to add the property isLight: true; to the THREE.Light TypeScript definitions.

Thanks and let me know if you have any questions.

@mrdoob mrdoob added this to the r104 milestone Apr 23, 2019
@mrdoob mrdoob merged commit f4c30aa into mrdoob:dev Apr 23, 2019
@mrdoob
Copy link
Owner

mrdoob commented Apr 23, 2019

Thanks!

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.

3 participants