steelsojka / aurelia-redux-plugin

A Redux plugin for the Aurelia framework
MIT License
17 stars 4 forks source link

select decorator context callback issue #5

Open dpinart opened 8 years ago

dpinart commented 8 years ago

Hi

I'm trying to use the select decorator in my viewModel and want to fetch some data whenever there's a change in the store. the viewModel looks as follows:

import {autoinject} from "aurelia-dependency-injection";
import {HttpClient} from "aurelia-fetch-client";
import { select} from "aurelia-redux-plugin";
@autoinject
export class MyViewModel{

  constructor(private httpClient: HttpClient){

  }
  @select('myState', {subscribe: true})
  public myState: any;

  myStateChanged(newValue: any, oldValue: any): void{
    this.httpClient.fetch('myuri')
      .then(r => r.json())
      .then(r => alert(r.message));
  }
}

The issue is that when the select decorator invokes myStateChanged method this.httpClient is undefined. I'm not sure how decorators are called but it seems the context sent is not the final instance of MyViewModel but some kind of metada or prototype...

I can bypass this issue by just calling the select method in the constructor, this way;

import {autoinject} from "aurelia-dependency-injection";
import {HttpClient} from "aurelia-fetch-client";
import {select} from "aurelia-redux-plugin";
@autoinject
export class MyViewModel{

  constructor(private httpClient: HttpClient){
    select('myState', {subscribe: true})(this, 'myState');
  }
  public myState: any;

  myStateChanged(newValue: any, oldValue: any): void{
    this.httpClient.fetch('myuri')
      .then(r => r.json())
      .then(r => alert(r.message));
  }
}

This works like a charm, is less declarative but it's enough at this time.

The issue, I think, appears that using the select feature not as a decorator can lead to memory leaks. If subscribe option is supplied, select feature observes the store for changes in the selected property in order to notify the target that a change ocurred. If every instance invokes the select feature a hook to allow observation disposal should be provided.

An easy way to offer the hook would be "select" returns a Disposable instance so the target can dispose. For example:

import {autoinject} from "aurelia-dependency-injection";
import {HttpClient} from "aurelia-fetch-client";
import {select} from "aurelia-redux-plugin";
import {Disposable} from "aurelia-binding";
@autoinject
export class MyViewModel{

  private _subscription: Disposable;
  constructor(private httpClient: HttpClient){
    this._subscription = <any> select('myState', {subscribe: true})(this, 'myState');
  }
  public myState: any;

  myStateChanged(newValue: any, oldValue: any): void{
    this.httpClient.fetch('myuri')
      .then(r => r.json())
      .then(r => alert(r.message));
  }

  unbind(){
    this._subscription.dispose();
  }
}

Another alternative, complementary, would be to provide some kind of mechanism to automatically dispose the subscription. Maybe, as most of select consumer will be some sort of aurelia component that follows the Aurelia component lifecycle, select feature might supply an unbind method (or wrap the existing one) and automotically dispose the observation

steelsojka commented 7 years ago

It looks like it's calling it with the constructor instead of the instance. I'll fix this soon.