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 event type as generic in EventDispatcher #468

Closed
wants to merge 3 commits into from

Conversation

repalash
Copy link

Add event type as generic in EventDispatcher

Add Support for event types in Object3D, Material, MeshBasicMaterial, MeshPhysicalMaterial, MeshStandardMaterial, ShaderMaterial, BufferGeometry, Camera.

Why

Makes it easier to specify types of events like: OrbitControls extends EventDispatcher<Event, 'change'|'end'|'start'>.

Also adds generics for Core classes so that event types can be specified when inheriting the three.js classes.

Checklist

  • Checked the target branch (current goes master, next goes dev)
  • Added myself to contributors table
  • Ready to be merged

One of my first contributions here. Let me know if anything is wrong. Thanks.

…bject3D, Material, MeshBasicMaterial, MeshPhysicalMaterial, MeshStandardMaterial, ShaderMaterial, BufferGeometry, Camera.
Copy link
Contributor

@Methuselah96 Methuselah96 left a comment

Choose a reason for hiding this comment

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

I think we want there to be a correlation between the event name and the type of the event, which is something @rafaelsc has started working on as part of #369. I'm not sure if @rafaelsc is still working on that PR, but there are still some unresolved comments on that PR as well.

@repalash
Copy link
Author

repalash commented Jun 6, 2023

Hello @Methuselah96, Ah, I didn't see that PR, will go through it.
In this way it's possible to infer the type from the event object type, but I added this as a shorthand for specifying the possible event types without the need for defining a separate typed event interface for each class. (Basically to get autocomplete in addEventListener and dispatchEvent functions for the types like this OrbitControls extends EventDispatcher<Event, 'change'|'end'|'start'>

We can hold on this till #369 is completed and see if it's useful then.

@rafaelsc
Copy link
Contributor

Sorry, @Methuselah96 I finally solved the conflict in my PR. I'm not using GitHub like I used before.

@repalash yes, I started working exactly your current PR, but in the end, I prefer the way that TS does for HTML events. Have autocomplete for the event name and the event properties.

@Methuselah96
Copy link
Contributor

Should be covered by #369, can be reopened if not for some reason.

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