rintoj / statex

StateX is a state management library for modern web applications with unidirectional data flow and immutable uni-state (just like redux)
MIT License
68 stars 18 forks source link

Can't seem to use classes for items in the state #10

Closed Nxt3 closed 6 years ago

Nxt3 commented 6 years ago

@rintoj

I have the following state:

export interface AppState {
  batchId?: string;
  events?: EventLogModel[];
  isLoading?: boolean;
  hasError?: boolean;
}

events is an array of EventLogModel objects:

export class EventLogModel {
  private id: string;
  private event_type: string;
  private occurred_on: string;
  private producedDomain: string;

  constructor(values: Object = {}) {
    Object.assign(this, values);
  }

  public getId(): string {
    return this.id;
  }

  public getEventType(): string {
    return this.event_type;
  }

  public getOccurredOn(): string {
    return this.occurred_on;
  }

  public getProducedDomain(): string {
    return this.producedDomain;
  }

  /**
   * If the event_type contains the word "reject", then mark it as an error.
   * This is used for the ListView status directive
   * @return {string} "error" or "success"
   */
  public determineStatus(): string {
    return this.event_type.toLowerCase().includes('reject') ? 'error' : 'success';
  }

  /**
   * If the eventConsumers array is greater than 0, then allow the ListView
   * to be expanded.
   * @return {boolean} true or false depending on the length of eventConsumers
   */
  public determineExpandable(): boolean {
    return this.eventConsumers.length > 0;
  }
}

After going through the action:

  @action()
  public searchBatches(state: AppState, payload: SearchBatchesAction): Observable<AppState> {
    return Observable.create((observer: Observer<AppState>) => {
      observer.next({ isLoading: true, hasError: false });

      this.eventListService.fetchEvents(payload.bId).subscribe(
        (e: Array<EventLogModel>) => {
          observer.next({
            batchId: payload.bId,
            events: e,
            isLoading: false
          });
          observer.complete();
        },
        error => {
          observer.next({
            events: [],
            isLoading: false,
            hasError: true
          });
          observer.complete();
        }
      );
    });
  }

and after being in the component:

export const eventList = (state: AppState) => state.events;

@Component({
  selector: 'pi-event-timeline',
  templateUrl: './event-timeline.component.html',
  styleUrls: ['./event-timeline.component.scss']
})
export class EventTimelineComponent extends DataObserver implements OnInit, OnDestroy {
  @data(eventList) public events: Array<EventLogModel>;

   public testEventsFunc(): void {
        console.log(`${this.events[0] instanceof EventLogModel}`); //this will print "false"
        for (let event of events) {
            console.log(`${event.getId()}`); //Will fail
        }
   }
}

I'm certain it's something with statex or something I'm doing wrong with statex because I'm using the same code for the service in another application; I'm in the process of using two apps that are identical in functionality: one with statex and the other with no real design pattern. This issue only occurs in the former example.

All of this works if I rewrite the EventLogModel to be an interface instead--but I shouldn't need to.

Am I doing something wrong?

rintoj commented 6 years ago

@Nxt3, The value returned by searchBatches through observable.next(<value>) is merged with the current app state. To keep the app state immutable (https://github.com/rintoj/statex#immutable-application-state), StateX will convert any object to an immutable object (uses seamless-immutable to do this)

In your case instance of EventLogModel is converted to a plain javascript object and merged with the current app state. That is the reason you get false for console.log(``${this.events[0] instanceof EventLogModel}``);

In my experience, converting objects to instances of class (as in other languages like Java) can considerably slow down your application. Therefore it is advisable to make determineStatus and determineExpandable as utility functions and pass on the plain javascript object to derive a value.

  function determineStatus(log: EventLogModel): string {
    return log.event_type.toLowerCase().includes('reject') ? 'error' : 'success';
  }

  function determineExpandable(log: EventLogModel): boolean {
    return log.eventConsumers.length > 0;
  }

Once you do that you can convert class EventLogModel to an interface.

export interface EventLogModel {
  private id: string;
  private event_type: string;
  private occurred_on: string;
  private producedDomain: string;
}

By doing this you will get all the benefits of type checking when do the development (just like the original class), while the production build will work on plain object. This setup will give you the best performance because the app need not create an instance of EventLogModel at run time.

Nxt3 commented 6 years ago

But what if you have an object that should be written as a subclass of another object? (i.e. class EventLogModel extends Event)

My point is that if you use statex, you're forced to use interfaces for any objects you want to work with.

rintoj commented 6 years ago

@Nxt3, there is no way to achieve that right now. That's a trade off for using immutable state.

You are right. StateX will force you to use interfaces for app state. You are free to use classes outside of app state though.