More Type-Safety (React MVVM Version 3 Proposal)

[Andrei15193]

It has not been that long since I've published version 2.2.0 and it will take a while until I publish version 3. In the mean time I want to write a few of the changes I will be making to further improve the library.

Before going into the details of one of the changes, I want to publish a new major version to clean the API a little bit. There are a few deprecations. Besides this, some of the changes I want to make may introduce some breaking changes, none of them major, just some tidying up along the way.

Without further ado, I will describe one of the changes I will be making to the library.

More Type-Safety

As the title suggests, I want to further enhance the type checking features of the library, especially around notifying about changed properties.

What has been somewhat bothering me is that whenever I want to notify about which properties may have changed, I do not get a list from where I can pick correct values. If I rename a property, I do not get compilation errors about it or have the IDE perform the rename in this case as well.

To illustrate the issue, consider the following code snippet.

class MyViewModel extends ViewModel {
  public doSomething(): void {
    // perform action

    this.notifyPropertiesChanged("I do not exist");
  }
}

There is no compilation error, I can notify about anything.

Version 3 Proposed Change

This may introduce a breaking change, I want to change the method declaration for notifyPropertiesChanged to use keyof. Initially I tried to use it in combination with Extract<,> to only select keys that are of type string, however this does not work with the compiler and I do not get the auto-complete or compilation checks that I wanted and thus decided to postpone this.

After some thinking, using keyof directly is not such a bad idea. You get all the options and will need to pick the ones that make sense. Not only this will offer a simple interface, but can be extended to event handlers as well.

protected notifyPropertiesChanged(changedProperty: keyof this, ...otherChangedProperties: readonly (keyof this)[]): void {
  this._propertiesChangedEvent.dispatch(this, [changedProperty, ...otherChangedProperties]);
}

This will have ripple effects on the propertiesChanged event as well, but since it goes from a list of string values to a list of keyof values everything should still work without any breaking changes.

The only issues that can happen are compilation errors about inexistent properties.

class BaseViewModel extends ViewModel {
  public myProp: string;

  public doSomething(): void {
    this.notifyPropertiesChanged("myProp"); // works
    this.notifyPropertiesChanged("myOtherProp"); // compilation error
  }
}

class MyViewModel extends BaseViewModel {
  public myOtherProp: string;

  public doSomething(): void {
    this.notifyPropertiesChanged("myProp"); // works
    this.notifyPropertiesChanged("myOtherProp"); // works
  }
}

Extending to Event Handlers

This will change event handlers as with this change we would need to use an IEventHandler<readonly (keyof this)[]> to get the benefits.

I want to introduce a few interfaces that simplify this.

export interface IPropertiesChangedEventHandler<T> extends IEventHandler<readonly (keyof T)[]> {
  handle(viewModel: object, changedProperties: readonly (keyof T)[]): void;
}

export interface IPropertiesChangedEvent<T> extends IEvent<readonly (keyof T)[]> {
  subscribe(eventHandler: IPropertiesChangedEventHandler<T>): void;

  unsubscribe(eventHandler: IPropertiesChangedEventHandler<T>): void;
}

This should help with readability and library exploration as well as getting the benefits of type checking into event handlers.

const baseViewModel = new BaseViewModel();
baseViewModel.propertiesChanged.subscribe({
  handle(_, changedProperties) {
    changedProperties.includes("myProp"); // works
    changedProperties.includes("myOtherProp"); // compilation error
  }
});

const myViewModel = new MyViewModel();
myViewModel.propertiesChanged.subscribe({
  handle(_, changedProperties) {
    changedProperties.includes("myProp"); // works
    changedProperties.includes("myOtherProp"); // works
  }
});

Inline Event Handlers

When using composition and event subscriptions, check ViewModel Design Considerations for more details, is providing an event handler that has more members than what the IEventHandler specifies.

Unfortunately, with what we currently have with React MVVM, the following code snippet will generate compilation errors.

const viewModel = new MyViewModel();
viewModel.propertiesChanged.subscribe({
  handlingEvent: false, // compilation error, handlingEvent is not declared by IEventHandler interface

  handle(_, changedProperties) {
    try {
      this.handlingEvent = true;

      // ...
    }
    finally {
      this.handlingEvent = false;
    }
  }
});

This is not a huge issue, we can work around this using closures or defining classes that implement the interface, however it does take away from the flexibility as well as having to deal with little details when it comes to edge cases such as this.

To solve this, I will be making the subscribe and unsubscribe methods generic.

For some reason we get a compilation error although our object implements the interface and even exposes a few extra properties. This is not a problem if we would define a class implementing the interface and create an instance of it, but when we use object literals it acts differently.

export interface IEvent<TEventArgs = void> {
  subscribe<TEventHandler extends IEventHandler<TEventArgs>>(eventHandler: TEventHandler): void;

  unsubscribe<TEventHandler extends IEventHandler<TEventArgs>>(eventHandler: TEventHandler): void;
}

With this change, the initial snippet that was giving compilation errors will now work and we will be able to use object literals for inline event handlers giving full flexibility on this is used.

Closing Thoughts

Now this can be a breaking change if I go with it, might change the IEvent and IEventHandler interfaces even more.

The issue is around the sender parameter when handing an event. Currently I've set it as an object which more or less translates to any. While it is not bad, it is not great either.

Generally, when dealing with "general" objects or objects whose value we do not know, we should use unknown instead of any because the former will generate compilation errors when trying to access any members while with the former it will just work.

This will clearly indicate that the source of the event is unknown and we need to perform an explicit cast to handle it.

I have not yet decided if I will have a typed sender object or just switch it to unknown. I haven't really used this parameter much in event handlers (removing it is another option) making the unknown approach more tempting.