deblockt / hal-rest-client

Typescript HAL Rest client
Apache License 2.0
25 stars 11 forks source link

Link override props #26

Closed dimik closed 5 years ago

dimik commented 6 years ago

Hi! We are trying to use your library but step into the issue. If we have prop in json and have link with similar name

{
  "_links": {
    "parent": { "href": "..."  }
  },
  "title": "Test category",
  "parent": "Test parent"
}

When we parse it: resource.props.parent - undefined resource.prop('parent') - returns HalResource, but we're expecting to get "Test parent" here

I check your code and find condition that is responsible for this bug

Also if you look closer to the parser, it's obviously have a racing issue and result depends on the json key order When you iterating over the json the key order is not determinated

for (const key in json) {
  ...
}

So in case your json has "_links" key before other props – you get one result If key order is different you will get another result (Your if-condition will go to the "else", because _links field will be parsed after "parent" :

{
  "title": "Test category",
  "parent": "Test parent",
  "_links": {
    "parent": { "href": "..."  }
  }
}
deblockt commented 6 years ago

the main issue is that prop function look for link or simple prop. If you have a link and a prop with same name wich element to return? I think there are a conception issue.

deblockt commented 6 years ago

is it a good idea to have a link and a prop with same name?

dimik commented 6 years ago

Our Server side Api has certain projections (using templated links) that expands some links into the props with json of the link. I think if you ask for a prop it should return prop, but calling prop to return link instead seems like unexpected behavior

deblockt commented 6 years ago

Yes, I have do that to simplify usage. I have correct the parser but I have an issue with object mapping (curently we can not have two properties with same name).

deblockt commented 5 years ago

Sorry , this is not possible with the current library architecture. maybe this will be possible in a later version