Closed kadamwhite closed 10 years ago
Tagging @carldanley @tkellen @bmac @tbranyen @rmccue @iros for your thoughts, since I've either talked to you about this project, or heard you mentioned by people I've talked to about it
Also @tlovett1 @tollmanz
@kadamwhite Excellent writeup. All of your points are great. While I was reading through it, I really felt like the following would be a decent solution:
.head()
method that allows you to retrieve the header.raw()
method that returns the raw request object, if needed (includes option 1)This would solve most of what you need and allow the developer to make decisions about pagination as needed.
I need more time to digest this idea but these are my initial thoughts, at the very least. If you expose an augmentation layer, you can introduce new stuff to enhance the library as needed. Meanwhile, you're allowing the developer to write their own methods; which I think results in a very powerful API.
Thanks for the quick response, @carldanley. Some thoughts:
.head()
exists, as it is one of the request methods available on WPRequest
. As such, getting the headers independently is still possible..raw()
is interesting, and worth considering regardless of the decision here.link
header is a raw string: You have to use a link parser library like parse-link-header or parse-links to deconstruct that into something useful; I think the value that's returned should be exposed in some way to client consumers, because being able to work intuitively with pagination is going to be critical because requests will be limited to max 100 items at once.Playing devil's advocate here:
Couldn't you provide the augmentation layer and a built-in transformation for handling pagination? Then have links automatically consume this transformation to deliver the results you're looking for?
I'm a fan of option 2. It seems the most intuitive to me rather than having to deal with checking headers. Pagination needs to be added to collections in the WP API backbone client so I'll definitely be following this closely.
This is a great writeup @kadamwhite.
here's my 2 cents:
Considering you're making a network request, using then
is not that unreasonable. If anything, it should be clear to the user that this is an async operation and might take time. As such, this approach feels the most natural to me.
wp.posts().then(function(resp) {
// collection:
console.log( resp.data );
// pagination URL:
console.log( resp.links.next );
});
I do like the properties you established in Option 1, example 2 in terms of expressiveness. Not sure this helps much, but I think you're on the right track.
@iros, there weren't any examples under Option 1: did you mean Option 2, example 2?
The library uses then
, or a node-style callback syntax, to handle the asynchronicity no matter what; these are equivalent in the current build:
// Do a GET
wp.posts().get(function(err, data) {});
wp.posts().then(function(data) {}, function(err) {});
// Do a POST
wp.posts().data({}).post(function(err, data) {});
wp.posts().data({}).post().then(function(err, data) {});
As such, it's not the presence of then
that bothers me: it's that in the above, the value with which the promise is resolved is the request payload, and if we converted that into the full request (option 1) then every handler would have to explicitly pluck the body off. Not the end of the world, but that's what I was trying to articulate that I was hoping to avoid with Option 2.
@kadamwhite I think augmenting the collection makes the most sense. Simply exposing headers or transforms is useful, but not particularly great for pagination discovery. Also introducing a new method next()
is rather confusing since you already have a way to fetch posts.
I think that Option 2 offers all the flexibility that's necessary assuming you also change how wp.posts()
works so that passing in a page offset will fetch it:
// Make the first request for the first page with the default limit.
wp.posts().then(function(page1) {
// Resolve with both the first and second page.
return Promise.all([
page1,
// Also fetch the second page for example purposes.
wp.posts({ page: page1.paging.next })
]);
}).then(pages) {});
This would make it very clear on how you could fetch all pages. Building up that Promise.all
array and then chaining with a .then
.
/2c
Alright; this is the proposed structure for collection responses:
paging
(object)total
(int; total # of records)totalPages
(int; total # of pages)links
(object)
next
(string; URL to next collection)prev
(string; URL to previous collection)I'm also planning to pre-bind paging.next
and paging.prev
properties, which would be WPRequest
objects bound directly to the URLs exposed through paging.links.(prev|next)
. The reasoning for this is to make it easy to immediately trigger a request for the next or previous collection without changing the behavior of the base wp.*
methods (i.e., the parameter suggestion @tbranyen brought up), and to make a clear distinction between exposing the raw URL in paging.links.(next|prev)
(which is all we have for links right now) vs a user-facing object in paging.(next|prev)
.
Initial implementation largely handled in #77, #79 and #80.
Closing as this is officially released in #82 (0.3.0); we may come in and refactor this to make pagination work more as a chained output transformation, rather than part of the core WPRequest functionality, but the public API for pagination info (at least for what the WP API currently provides) is implemented as documented above.
Many responses from the API will be paginated, either explicitly or by default. For example, if post collections return 10 items by default, and you have 100 items in your DB, both of these will come back with a header link with
rel="next"
:The general approach I have taken with this library is to let the user specify a request, and return what they asked for. At the moment the way this manifests is that if you request
wp.posts()
..., you get the posts collection (that is, the response body) back in the request callback: the headers are not included. I favored this over returning the full object, because this felt like a very clunky thing to have to do every time:when in many cases it's only the response body that you care about. The one exception is paginated collections, as it is the response header that includes the pagination information (via the
link
header and two custom header properties).I want to provide an intuitive way to expose collection paging to users, preferably without making it substantially more verbose to request a specific resource. I am not sure how to proceed.
Option 1: Return the raw request object
This is what is outlined above, where the request object is passed back in full and you can manually grab the body or check the
link
header as you need to. This feels like a bad solution because it makes the default usage (get a specific resource) more difficult by requiring a pipe through athen
, and it doesn't give you any additional functionality (you need to manually parse headers to handle paging).Option 2: Augment the response data with paging properties
The idea is to augment the response object with properties or methods relating to pagination in cases when the resulting data is determined to be paginated. There were various ways it could be implemented:
Example 1, just expose the links and pagination headers:
Example 2, give the returned collection methods that would function as fully-fledged WPRequest objects
Things I like about this approach:
wp.posts.head()
), which keeps the data returned from the service directly accessible to clients without transformationThat said, when I pitched this to a collaborator he described it as "yucky" and I couldn't think of any other examples of systems that work this way. I want to present it for review anyway, though, because it was my first thought.
Option 2A: Let users define custom data augmentation methods
This is an extension of one way the above could be implemented. If you imagine a prototype method on
CollectionRequest
calledtransform
, which could be used to register a methods that would be run on the request response before being returned (which would replace the existingreturnBody
andreturnHeaders
methods), you could let users opt-in to things like lodash collection methods, pagination handling, or their own custom transformations:Upshots:
Downsides:
wp.transforms
orwp.utils
namespace to hold the default transformation methodsOption 3: Return a custom object
This is in some ways an extension of 1, just with some of the things I noted addressed. Basically, all requests come back as an object with four properties:
The advantage of this over the first option is that this, to me, feels more intuitive than returning
resp.body
:The disadvantage is that I'm right back where I started in terms of having to pipe responses through yet another
then
in order to boil them down to the actual response body, if that's what I wanted originally.I am probably overlooking additional options, here. These are the actual pagination-related header properties exposed by the API endpoint, for an example collection of two posts:
It's possible the WordPress API itself could be extended to provide more headers or more links such as a first or last page, as it's a work-in-progress, but for the time being I'm trying to figure out if there's any other approach than the ones outlined above to make answering these questions (and others like them) easy:
Questions a solution should answer
Any recommendations for good API clients that handle this sort of behavior are welcome, as is commentary or dissension around the above options I have outlined.