Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

findById and find method in custom API - return type problem #63

Closed
sprypradeep opened this issue Jul 29, 2016 · 6 comments
Closed

findById and find method in custom API - return type problem #63

sprypradeep opened this issue Jul 29, 2016 · 6 comments

Comments

@sprypradeep
Copy link

In the findById and find methods, the return type is being modified before returning:
findById

  public findById(id: any, filter: LoopBackFilter = undefined) {
...
    let result = this.request(method, url, routeParams, urlParams, postBody)
    return result.map((result: School | Array<School>) => Array.isArray(result)
                             ? result.map((instance: School)=> new School(instance))
                             : new School(result));
  }

find

  public find(filter: LoopBackFilter = undefined) {
...
    let result = this.request(method, url, routeParams, urlParams, postBody)
    return result.map((result: School | Array<School>) => Array.isArray(result)
                             ? result.map((instance: School)=> new School(instance))
                             : new School(result));
  }

Shouldnt they be returned as-is ? the find should result in an array always and never as a single Object, which creates problems with the caller, and also results in error on compilation. In case of findById, it can not be an array, so why are we introducing the check.

@jonathan-casarrubias
Copy link
Collaborator

jonathan-casarrubias commented Jul 29, 2016

Mapping the type of a result while fetching data is an expected behaviour that allows you to extend the SDK Models and add your own logic.

Requested in issue #11 it is also documented in here https://github.com/jonathan-casarrubias/loopback-sdk-builder/blob/master/ANGULAR2-DOCS.md#extending-models

Since all the methods are created in a loop of methods, fetch methods are added with this functionality and works standarazied -the same- in each of these methods, I may add logic while generating the sdk to avoid doing the check during API calls, but the functionality is expected and will remain.

I just don't understand the errors you comment, the functionality has been there since beta 18 and it has been tested in Angular CLI and Web Pack, never had a compilation issue because of this, for instance, it does not really matter if the result is an object or an array, thanks to (result: School | Array<School>) you can receive any of these, in the case you receive a single object then the service return an actual instance of the School model and not just a json, in the case it is an array there is a validation for that and will result in an array of Schools.

I don't see how receiving a single object or an array will cause a compilation issue since it is handled in there, both in typings and in javascript logic.

But if you got a use case that actually generates compilation issue, could you elaborate and show the compilation error stack?

Cheers
Jon

@sprypradeep
Copy link
Author

sprypradeep commented Jul 29, 2016

Okay, so you are saying that the code for find() and findById() are coming from same template. The problem I face is the following compilation error (with ng serve / angular-cli)

Error: Typescript found the following errors:
  /home/XXX/tmp/broccoli_type_script_compiler-input_base_path-E2gLxjkB.tmp/0/src/app/XXX/school-list.component.ts (24, 18): Type 'School[] | School' is not assignable to type 'School[]'.
  Type 'School' is not assignable to type 'School[]'.
  /home/XXX/tmp/broccoli_type_script_compiler-input_base_path-E2gLxjkB.tmp/0/src/app/XXX/school-list.component.ts (24, 18): Type 'School[] | School' is not assignable to type 'School[]'.
  Type 'School' is not assignable to type 'School[]'.
    Property 'length' is missing in type 'School'.

with following component code

  schools: School[];

  constructor(
    private service: SchoolApi) {}

  ngOnInit() {
    this.service.find()
    .subscribe(
      schools => this.schools = schools,
      error =>  this.errorMessage = <any>error);
  }

Another error occurs on findById usage which includes message:

Type 'School[] | School' is not assignable to type 'School'.
  Type 'School[]' is not assignable to type 'School'.
    Property 'name' is missing in type 'School[]'.

To avoid the errors I have to declare my variables as union type

school: School[] | School;
schools: School[] | School;

@jonathan-casarrubias
Copy link
Collaborator

jonathan-casarrubias commented Jul 29, 2016

So, correct me if I'm wrong, but then you are receiving only 1 school in the call result? its strange because find should always return an array, not sure why would not pass the Array.isArray validation

  ngOnInit() {
    this.service.find()
    .subscribe(
      schools => this.schools = schools,  // <----- HERE?
      error =>  this.errorMessage = <any>error);
  }

If you are actually receiving an array, then have you tried this?

schools: School[];

  ngOnInit() {
    this.service.find()
    .subscribe(
      (schools: School[]) => this.schools = schools,
      error =>  this.errorMessage = <any>error);
  }

@sprypradeep
Copy link
Author

Of course setting the type in response callback fixes the error. And Yes earlier also the result was in array for find() (thanks to Array.isArray), but wouldnt it be right if the two methods had different result dispensation.

@jonathan-casarrubias
Copy link
Collaborator

yes I just wanted to make sure that fixes your problem, but im keeping te issue open to handle find, findOne and findById while generating and not while excecuting

@jonathan-casarrubias
Copy link
Collaborator

Fixed, will be launched in RC6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants