wopian / kitsu

🦊 A simple, lightweight & framework agnostic JSON:API client
https://yarn.fyi/kitsu
MIT License
273 stars 41 forks source link

Document links pagination to retrieve the next/previous/first/last page #451

Open bagley2014 opened 4 years ago

bagley2014 commented 4 years ago

The resourceCase option defaults to 'kebab' even though that's invalid for Kitsu. For example, when trying to include "animeStaff" when fetching anime, the error "anime-staff is not a valid relationship of anime" is produced when using the default resource option.

wopian commented 4 years ago

Kitsu.io uses kebab-case for their API https://kitsu.io/api/edge/anime-staff.

Parameters (e.g. { include: 'animeStaff' }) do not have their case converted in either packages. The example earlier would produce https://kitsu.io/api/edge/anime?include=animeStaff which is valid for Kitsu.io.

What is the:

wopian commented 4 years ago

@bagley2014 what is the request you're making with the package?

bagley2014 commented 4 years ago

Sorry, I should have taken the effort to provide a minimal reproducible example instead of just assuming it would be easy to reproduce. I went to work on one, and the issue might be that my include is in the query string, not a parameter to the function. (I'm doing it like that because I'm pulling the url from the relationship section of a response.) The code below should reproduce the error.

const Kitsu = require('kitsu');
const api = new Kitsu();

address = `users/103224/library-entries?include=anime.animeStaff`

api.get(address).catch((reason) =>{
    console.log(
        `My address: ${address}` +
        `\n\nErrored address: ${reason.config.url}` +
        `\n\nErrors: ${JSON.stringify(reason.errors)}`
    );
});

My output from this code is:

My address: users/103224/library-entries?include=anime.animeStaff

Errored address: users/103224/library-entries?include=anime.anime-staff

Errors: [{"title":"Invalid field","detail":"anime-staff is not a valid relationship of anime","code":"112","status":"400"}]

I'm using kitsu version 9.1.13, for the record.

wopian commented 4 years ago

Thank you and no worries 👍

const address = users/103224/library-entries?include=anime.animeStaff

api.get(address)

JSON:API request queries are not valid in the model parameter (first parameter of get, post, patch, delete). https://github.com/wopian/kitsu/tree/v9.1.13/packages/kitsu#examples-4

What you're after is the following:

const address = 'users/103224/library-entries'
const params = { include: 'anime.animeStaff' }

api.get(address, params) // users/103224/library-entries?include=anime.animeStaff

I'll update the documentation for the model parameters to make the expected values to be more clear:

bagley2014 commented 4 years ago

Though not supported, so far I've been able to get the functionality I need by initializing the api with resourceCase: 'none'

The reason I ended up with query parameters is that I thought the best solution for handling pagination would just be to make another api call using the "next" link that was returned. I think an example of getting all the pages of a library or something would be a valuable addition to the documentation as well.

wopian commented 4 years ago

The reason I ended up with query parameters is that I thought the best solution for handling pagination would just be to make another api call using the "next" link that was returned.

Parsing a JSON:API query is a little more complicated than the reverse (turning an object into a JSON:API query string) due to JSON:API having nested query parameters (i.e fields[users]=slug) which no built-in API on Node or Browser engines support.

I'm reluctant to adding it to kitsu-core as a built-in as it will likely double or triple the library size if I imported or re-implemented qs's nested query parameters implementation.

Which is something I'm trying to undo and avoid in the future with the existing pluralize dependency (20% of kitsu's library size) with the 10.x releases currently in an alpha state. Where kitsu will behave like kitsu-core by providing the bare minimum (while defaults still work with Kitsu.io out of the box) and allow users to build on top of it - i.e import a pluralisation dependency if you want automatic pluralisation of type instead of how it is in current kitsu releases where you can disable it , but still have the dependency sitting in your production code.

I think an example of getting all the pages of a library or something would be a valuable addition to the documentation as well.

You can achieve this with qs. This code snippet will be added to the examples in https://github.com/wopian/kitsu/tree/master/packages/kitsu/example

Live demo: https://codesandbox.io/s/polished-sun-vsn7s?file=/src/index.js

import { parse } from 'qs'
import Kitsu from 'kitsu'

const baseURL = 'https://kitsu.io/api/edge/'
const api = new Kitsu({ baseURL })

const next = 'https://kitsu.io/api/edge/users/103224/library-entries?include=anime.animeStaff&fields[anime]=slug&fields[library-entries]=progress,anime'

// [ 'users/library-entries', 'include=anime.animeStaff&fields[anime]=slug...' ]
const [address, params] = next.split(baseURL)[1].split('?')

// 'users/library-entries', { include: 'anime.animeStaff', fields: { anime: 'slug', ... } }
console.log(address, parse(params))

api.get(address, parse(params))

I'll add baseURL as a get/setter of the Kitsu class in the next 9.x minor release. Apparently I never added it as a get/setter unlike the other Kitsu class options. This would avoid having to pass the baseURL around as a separate variable:

import { parse } from 'qs'
import Kitsu from 'kitsu'

const api = new Kitsu()

const next = 'https://kitsu.io/api/edge/users/103224/library-entries?include=anime.animeStaff&fields[anime]=slug&fields[library-entries]=progress,anime';
const [address, params] = next.split(api.baseURL)[1].split('?')

api.get(address, parse(params))