iTwin / appui

Monorepo for iTwin.js AppUi
MIT License
8 stars 2 forks source link

Deprecate UIEvents subclasses in AppUi-React #678

Closed raplemie closed 4 months ago

raplemie commented 7 months ago

Is your feature request related to a problem? Please describe. Many event used in our system (IE. FronstageActivatedEvent in FrameworkFrontstages, ContentLayoutActivatedEvent in ContentLayout) are subclasses of UiEvent defined as publicly available classes, and used.

These are not "body" of events but the actual event emitters to which application register and the event emitted is an interface, so there is no reason to make these public (an application shouldnt have a reason of creating a new FronstageActivatedEvent() emiter in their application, as the only thing this will give them is a UiEvent (emitter) with the FrontstageActivatedEvent type event.

Describe the solution you'd like Deprecated these emmiter classes. (we should be able to public static readonly onFrontstageActivatedEvent = new BeUiEvent<FrontstageActivatedEventArgs>();

raplemie commented 7 months ago

As the FrameworkFronstage interface is exported as public, it would be a violation of support policy to change the interface from a class to a different one, however, these were made public simply because of the system misunderstanding as there is no reason for anyone to recreate an object following this interface (or it to be exported, really), it is simply the type of UiFramework.fronstage, which wouldnt be broken by changing if all the event handler were defined as interfaces themselves as the behavior would remain the same. So when deprecating these classes, the "FrameworkXXX" classes should also be deprecated as they are not publicly meaningful. These interfaces themselves should not be exported but simply used internally as the type of the members of UiFramework and remove all class that forces an implementation method.