How should I handle a type for sap/ui/base/Event?

[sz3lbi]

Hello,
I see that between UI5 1.108 and 1.120 a rather big change has been introduced to the type of an Event (sap/ui/base/Event).

1.108:

export default class Event extends BaseObject implements Poolable

1.120:

export default class Event<
  ParamsType extends Record<string, any> = object,
  SourceType extends EventProvider = EventProvider
>
extends BaseObject
implements Poolable

Before, I could easily bind my function to an event and the types would match. Afterwards, in the handler, when I wanted to get a parameter, I would cast it to unknown and check the shape of and object before accessing properties like:

const vArguments = oEvent.getParameter("arguments") as unknown;
if (
  !vArguments ||
  typeof vArguments !== "object" ||
  !("parentPath" in vArguments) ||
  typeof vArguments.parentPath !== "string"
) {
  // Some error handling and logging.
  return;
}

Now, after the change which has been introduced, I am able to define the shape of an Event object in the function definition. However, then I face some problems when trying to attach this handler to an event:

private registerP13n(): void {
  // Some logic.
  // Engine.attachStateChange(fnStateEventHandler: (p1: Event) => void): Engine
  Engine.getInstance().attachStateChange((oEvent) => {
    this.stateChange(oEvent); // Argument of type 'Event<object, EventProvider>' is not assignable to parameter of type 'Event<{ control: Table; state: Record<string, SelectionController>; }, EventProvider>'.
  });
}

private stateChange(
    oEvent: Event<{
      control: Table;
      state: Record<string, SelectionController>;
    }>
  ): void {
    // Whatever.
  }

The attachStateChange method requires attaching a function that takes an Event type which basically means Event<object, EventProvider> and my type is not compatible there.
I could of course use a type like Event<any> in the handler and then do what I did before, but that would defeat the purpose of the change introduced.

How should I handle this? What am I not seeing here?
Thanks.

[nikoletavnv]

Hello @sz3lbi,

Can you give us more context on the things you have tried? Have you tried to cast explicitly oEvent object to the desired type?
What is the structure of oEvent object? Where is the EventProvider part?

Kind regards,
Nikoleta

[sz3lbi]

Okay, let me explain what my concerns and wonders are here.

For other components and their events, such as ActionItem and its press event we have the specific types that can be used such as ActionItem$PressEvent.
The event I have a problem with is the state change event on the Engine instance, which I attach using the attachStateChange method, as explained above. There are no occurrences of Engine$ or $State strings in the sap/m/p13n/Engine module, so I assume there is no specific type for this event.

With the old version of UI5, the Event type was by default of the any type. So, I had no choice but to manually check if the properties exist on this type. Here, in 1.120, considering that the generics were introduced for the Event type, I assumed that there was a better/nicer way to make it work.
What I am trying to understand is why would you introduce this change when even now it requires checking for the properties or casting to a type I would have to create manually? What is the benefit of this change?

What is the structure of oEvent object?

The shape of the oEvent object in the debugger looks like this:

bCancelBubble: false
bPreventDefault: false
mParameters:
  control: c {bAllowTextSelection: true, mEventRegistry: {…}, sId: 'container-com.mycompany.myapp---MyView--myTable', mProperties: t, mAggregations: {…}, …}
  state: 
    Columns: (2) [{…}, {…}]
    Sorter: []
    [[Prototype]]: Object
  [[Prototype]]: Object
oSource: constructor {mEventRegistry: {…}}
sId: "stateChange"

Where is the EventProvider part?

The EventProvider part of a type is not required. I only provided the first part and for the second part the default was used.

I hope I have made my concerns and questions clear enough. Thank you!

[hinzzx]

Hello @sz3lbi ,

Thank you for clarifying ! I've created an internal incident DINC0460651. The status will be updated here in GitHub.

[akudev]

Hi @sz3lbi

I would cast it to unknown and check the shape of and object before accessing properties

Well, that's exactly what we wanted to get rid of with that event generics change. In TypeScript things should be typed and you shouldn't have to do such casts and checks where we can avoid it.

The event I have a problem with is the state change event on the Engine instance

This makes the root cause here clear: most events are declared in the metadata and the attach/detach methods and the generics are generated automatically. But this attach method is coded and documented explicitly: https://github.com/SAP/openui5/blob/master/src/sap.m/src/sap/m/p13n/Engine.js#L246-L249
This is why it lacks the proper typing.
This happens for some classes which do not inherit from ManagedObject, so they cannot declare the event metadata.

why would you introduce this change when even now it requires checking for the properties or casting to a type I would have to create manually?

Basically, this is only required where this change we did does not have any effect. For the vast majority of events, the change is beneficial, as it brings type safety and avoids the checking and casting.

But your point is of course valid that there are events which do not get this improvement. For some of the non-metadata-declared events, the JSDoc has been adapted to also get the event parameters typed, e.g. for Core#themeScopingChanged here. We'll try to get an overview where this is still missing. And I'll trigger the enhancement of your specific Engine event.

Does this clarify things?

[sz3lbi]

Hi @akudev,

Everything is clear now. Thanks for the explanation and willingness to improve the remaining events.

[akudev]

Small correction: sap/base/i18n/Localization shows how it actually should be done:

openui5/src/sap.ui.core/src/sap/base/i18n/Localization.js

Lines 217 to 260 in d73b87b

	/**
	* The localization change event. Contains only the parameters which were changed.
	*
	* The list below shows the possible combinations of parameters available as part of the change event.
	*
	* <ul>
	* <li>{@link module:sap/base/i18n/Localization.setLanguage Localization.setLanguage}:
	* <ul>
	* <li><code>language</code></li>
	* <li><code>rtl?</code> (only if language change also changed RTL)</li>
	* </ul>
	* </li>
	* <li>{@link module:sap/base/i18n/Localization.setRTL Localization.setRTL}:
	* <ul>
	* <li><code>rtl</code></li>
	* </ul>
	* </li>
	* <li>{@link module:sap/base/i18n/Localization.setTimezone Localization.setTimezone}:
	* <ul>
	* <li><code>timezone</code></li>
	* </ul>
	* </li>
	* </ul>
	*
	* @typedef {object} module:sap/base/i18n/Localization$ChangeEvent
	* @property {string} [language] The newly set language.
	* @property {boolean} [rtl] Whether the page uses the RTL text direction.
	* @property {string} [timezone] The newly set timezone.
	* @public
	* @since 1.120.0
	*/
	/**
	* Attaches the <code>fnFunction</code> event handler to the {@link #event:change change} event
	* of <code>module:sap/base/i18n/Localization</code>.
	*
	* @param {function(module:sap/base/i18n/Localization$ChangeEvent)} fnFunction
	* The function to be called when the event occurs
	* @public
	* @since 1.120.0
	* @static
	*/
	attachChange: function(fnFunction) {
	oEventing.attachEvent("change", fnFunction);
	},

The one linked above is different and not that good, and not public, and also other ones in the code don't define an event type and use it for typing the function.