Usability problems

[smnbbrv]

First of all, I like the library. Thank you for you effort!

After some playing I found out some things concerning the usability improvement

1st. I'm used to define the data type inside of resource file to successfully reuse it everywhere.

E.g. wherever I call the resource I use the following:

myResource.read().subscribe(res => res. /* and here it knows what is res */)

What you offer, as I can see, does not really allow that. This is the best I could come to to make it possible

import { Resource, ResourceAction, ResourceParams, ResourceResult } from 'ng2-resource-rest';
import { RequestMethod } from '@angular/http';
import { Observable } from 'rxjs';

@ResourceParams({
  url: '/api/',
  path: 'v1/preview/{id}'
})
export class PreviewResource extends Resource {

  @ResourceAction({ method: RequestMethod.Get })
  private _read(data?: { id: string; }): ResourceResult { return null; }

  public read(id: string): Observable<{ name: string; value: string }> {
    return this._read({ id }).$observable;
  }

}

Would be nice to have something like

import { Resource, ResourceAction, ResourceParams, ResourceResult } from 'ng2-resource-rest';
import { RequestMethod } from '@angular/http';
import { Observable } from 'rxjs';

interface onSuccess {  ... }

interface onError { ... }

@ResourceParams({
  url: '/api/',
  path: 'v1/preview/{id}'
})
export class PreviewResource extends Resource {

  @ResourceAction({ method: RequestMethod.Get })
  public read(data: { id: string }): ResourceResult<OnSuccess, OnError> {
    return this._read({ id }).$observable;
  }

}

This allows to use the resource way smoother, encapsulates the logic and response format in the place where it should be and finally uses the TypeScript with its full power.

Also you would get rid of the array problem inside of the result, because it could be simply the part of generics: <OnSuccess[], OnError>

2nd. Another thing I cannot understand is why instead of returning a raw Observable the object is returned. Observable gives enough flexibility to check whether it is resolved or not. Is there a way to return the observable directly?

3rd. Can we specify the url ResourceParamsBase property in a global way for the whole application (e.g. in a bootstrap file)? Maybe it is not that useful for every project but still would be nice to have

[troyanskiy]

Hello.

1. Data type
   You can try to extend the ResourceResult

export interface IMyData {
  id: string;
  name: string;
}
export interface IMyDateResourceResult extends IMyData, ResourceResult;

@ResourceParams({
  url: '/api/',
  path: 'v1/preview/{id}'
})
export class PreviewResource extends Resource {

  @ResourceAction({ method: RequestMethod.Get })
  read(data?: { id: string; }): IMyDateResourceResult { return null; }
}

1. It's depends what type of response you receive object or array it will return object or array.
   So you can do the following

let receivedData = this.previewResource.read();

The data will be added after receiving the response.
So if you will do console.log(receivedData) you will see the object

{
  $observable - the raw observable
  $resolved: false
}

And then after receiving the data the object will mutate.
After setTimeout(() => console.log(receivedData), 2000) you will see

{
  $observable: object, - the raw observable
  $resolved: false,
  id: 'your id'
  name: 'some name'
}

So you can directly map the variable to the template. Like that

@Component({
  // i will skip other props
  template: '<div (click)="clicked(data)" *ngIf="data?.$resolved">{{data.name}}</div>'
})
export class TestComponent implements OnInit {

  data:IMyDateResourceResult = null;
  constructor(private previewResource: PreviewResource){}

  NgOnInit() {
    this.data = this.previewResource.read();
  }

  clicked(data:IMyDateResourceResult) {
    console.log('Clocked data id:' + data.id);
  }

}

[smnbbrv]

@troyanskiy thank you for so fast response.

This becomes more clear... But if I use a raw Observable, does the TypeScript know that its result is of type IMyDateResourceResult? When I look into index.d.ts I see the following:

export interface ResourceResult {
    $resolved?: boolean;
    $observable?: Observable<any>;
}

That means if I use a raw Observable it won't know of the res type:

$observable.map(res => res.value).subscribe();

this res would be of any type regardless of the interface I create. I would need to explicitly define it

$observable.map((res: IMyDateResourceResult) => res.value).subscribe(...);

or do it inside of the resource:

interface IMyData {
  id: string;
  name: string;
}

interface IMyResourceResult<T> extends ResourceResult {
  $observable?: Observable<T>;
}

export interface IMyDateResourceResult extends IMyResourceResult<IMyData>, IMyData;

What would be way nicer is to use this solution microsoft/TypeScript#2225 to have something similar to the following:

interface Observable<T> {}

interface IMyData {
  id: string;
  name: string;
}

type ResourceResult<D extends {}> = D & {
  $resolved?: boolean;
  $observable?: Observable<D>;  
}


let a: ResourceResult<IMyData> = { id: '1123', name: 'fsfdsdf' };

let b: ResourceResult<IMyData[]> = [{ id: '1123', name: 'fsfdsdf' }];

This works for both: the returned object and the observable result. And again, you don't need to have ArrayResourceResult: this is automatically covered with the solution above (see let b)

[smnbbrv]

I understand, this might break the previous ResourceResult because there is no support for default generic values yet microsoft/TypeScript#2175 but in my opinion this is a better way to go to the future

[troyanskiy]

Thank you.
That's interesting... I will check that a bit later.

[smnbbrv]

created #30 for a not solved thingy...

[smnbbrv]

@troyanskiy still there is one problem... When I want to override e.g. an update property with a new way to enhance it with the type I can't really do that because a member property cannot override a member function (because the Resource class uses a function update). To fix it I think it still requires some breaking changes... What do you think about v1.0.0 with no support for the old way anymore?

[troyanskiy]

I will think about new way of defining members. And the new version will be v1 with removed old way.
I hope that I will have time to do that this week.

[smnbbrv]

@troyanskiy Thank you! I was already thinking of possible problems of a property-based notation... There is an upcoming problem which might look interesting. Assume we have the following and it works:

import { Resource, ResourceAction, ResourceParams, ResourceProcessor } from 'ng2-resource-rest';
import { RequestMethod } from '@angular/http';

interface User {
  id: string;
  fullname: string;
}

@ResourceParams({ path: '/api/v1/preview/{id}' })
export class UserResource extends Resource {

  @ResourceAction({ method: RequestMethod.Get })
  public read: ResourceProcessor<{ id: string }, User>;

}

Then accidentally (as it always happens) the API gets changed. The version is increased to v2 and now instead of fullname it returns the firstname, lastname and phone. We don't want to change our app but we want to use the new fields. That is how it might work:

import { Resource, ResourceAction, ResourceParams, ResourceProcessor } from 'ng2-resource-rest';
import { RequestMethod } from '@angular/http';

interface User {
  id: string;
  fullname: string;
  firstname: string;
  lastname: string;
  phone: string;
}

@ResourceParams({ path: '/api/v2/preview/{id}' })
export class UserResource extends Resource {

  // the same read request
  @ResourceAction({ method: RequestMethod.Get })
  private _read: ResourceProcessor<{ id: string }, User>;

  // legacy method declaration
  public read: ResourceProcessor<{ id: string }, User>;

  constructor () {
    this.read = (input: { id: string }) => {
      let resourceResponse = this._read(input),
          response = {
            $resolved: false,
            $observable: SomeSubject // another observable implementation
          },
          subscription = resourceResponse.$observable.subscribe(res => {
            // actually that is why we need all this logic around
            // generate the not existing anymore property
            res.fullname = res.firstname + ' ' + res.lastname;

            Object.assign(response, res);
            subscription.unsubscribe();
          });

      return response;
    };
  }

}

The problem is solvable even with a property-based resource. But still it makes sense to make kind of a proxy for some sort of these manipulations. Currently it looks awkward... What do you think?

Maybe this is kind of the way to go:

import { Resource, ResourceAction, ResourceParams, ResourceProcessor } from 'ng2-resource-rest';
import { RequestMethod } from '@angular/http';

interface User {
  id: string;
  fullname: string;
  firstname: string;
  lastname: string;
  phone: string;
}

@ResourceParams({ path: '/api/v2/preview/{id}' })
export class UserResource extends Resource {

  // the same read request
  @ResourceAction({ method: RequestMethod.Get })
  private _read: ResourceProcessor<{ id: string }, User>;

  // legacy method declaration
  public read: ResourceProcessor<{ id: string }, User>;

  constructor () {
    this.read = (input: { id: string }) => {
      return this._read(input).$proxy(res => {
        res.fullname = res.firstname + ' ' + res.lastname;
      });
    };
  }

}

[smnbbrv]

ok, I guess this can be achieved with responseInterceptor... Maybe a bit low-level, but still possible. Not an issue then.

[smnbbrv]

Another question: currently the Resource class implements default methods get, post, etc. Does it make sense to have two classes, let's say bare bone Resource (without standard methods) and StandardResource (with them) that it is possible on compiler level to disable the not existing requests? E.g. only get is supported by the backend, so I'd rather use Resource in this case.

[troyanskiy]

Hi @smnbbrv
I have released v1.0.1
Also I have updated the README.MD file.
It work fine for me.

[smnbbrv]

Hi @troyanskiy

This looks awesome 👍 ! I will try it as soon as I can. At first I need to finish the migration to RC6 :)