devour-js / devour-client

Don't just consume your JSON API, Devour it...
https://www.npmjs.com/package/devour-client
ISC License
429 stars 90 forks source link

Nested resource URLs #11

Closed Emerson closed 8 years ago

Emerson commented 8 years ago

@themarkappleby pointed out an issue that we need to figure out – especially since it's a valid JSON-API convention.

When fetching a resource, it's perfectly valid to do so through a nested URL, you can see a clear example of this articulated in the specification here.

And the following request fetches an article’s author: GET /articles/1/author HTTP/1.1

Right now, our find calls just accept the name of the model jsonApi.find('author', 1)... so what's the best way to support this feature?

@themarkappleby was thinking we could parse a raw string jsonApi.find('articles/1/author'), but I'd be a little worried about model names vs urls and figuring out how to map all that back.

I was thinking we could maybe pass an object in some instances:

jsonApi.find({model: 'author', url: 'articles/1/author'}, 1)

Or maybe an options object?

jsonApi.find('author', 1, {nested: 'articles/1'})

Happy to hear any thoughts about this – @derek-watson I know you are questioning the requirements of having defined models all together (I still need to respond to that)

derek-watson commented 8 years ago

Restangular does a nice job of expressing nested resource relationships, but they're able to do it because they separate the resource definition from the action in separate methods:

https://github.com/mgonto/restangular#url-building

Devour's solution to the example above might be

jsonApi.find('articles', 1).find('author').get()

Methods like find and findAll would return a reference to a resource, and you'd have to choose what action you wanted to take with it by adding a call to get, post, patch or destroy.

Emerson commented 8 years ago

Yea, that's what I was saying to Mark... would love to have that pattern, would be a fair amount of work to implement I think

derek-watson commented 8 years ago

I updated my comment after you replied with more detail :)

Emerson commented 8 years ago

Feeling like we should take things in that direction, so so nice.

derek-watson commented 8 years ago

If we do, it might make sense to rename some of the methods that describe resource locations, as they'll be used for multiple actions. For example,

Before:

// To find many...
jsonApi.findAll('post')

// To find many with filters...
jsonApi.findAll('post', {page: {number: 2}})

// To find one...
jsonApi.find('post', 5)

// To create...
jsonApi.create('post', {
  title: 'hello',
  content: 'some content',
  tags: ['one', 'two']
})

// To update...
jsonApi.update('post', {
  id: 5,
  title: 'new title',
  content: 'new content',
  tags: ['new tag']
})

// To destroy...
jsonApi.destroy('post', 5)

After:

// To find many...
jsonApi.all('post').get()

// To find many with filters...
jsonApi.all('post').get({page: {number: 2}})

// To find one...
jsonApi.one('post', 5).get()

// To create...
jsonApi.all('post').post({
  title: 'hello',
  content: 'some content',
  tags: ['one', 'two']
})

// To update...
jsonApi.one('post', 5).patch({
  id: 5,
  title: 'new title',
  content: 'new content',
  tags: ['new tag']
})

// To destroy...
jsonApi.one('post', 5).destroy()
Emerson commented 8 years ago

If anyone feels the will to work on this, I dropped a branch we can hack on:

https://github.com/twg/devour-jsonapi-client/tree/feature/resource-locations-refactor

billxinli commented 8 years ago

So we are basically copying Restangular's style?

Emerson commented 8 years ago

@billxinli I think that's the idea – but open to hear any thoughts

billxinli commented 8 years ago

I used Restangular before and quite liked it. I might take this and give ES6 a spin. Curious on the backend/browser support this library will have?

Emerson commented 8 years ago

Would love for this to work in both node and in browser, although I'd assume that its primary use will be in browser. I don't have any strong opinions about supported node versions, so ¯\\_(ツ)_/¯

billxinli commented 8 years ago

Should the builder method allow the following?

var jsonApi = new JsonApi({apiURl: 'http://example.net'})
jsonApi.get() // GET http://example.net/ 

Here we are assuming / is a resource of some sort.

Emerson commented 8 years ago

This has been resolved 🎉