Add cursor docs #309
Add cursor docs #309
Conversation
README-source.md
Outdated
| @@ -1339,8 +1348,8 @@ Paging is **only used when a trigger is part of a dynamic dropdown**. Depending | |||
|
|
|||
| Paging is a lot like a regular trigger except the range of items returned is dynamic. The most common example of this is when you can pass a `start` parameter: | |||
There was a problem hiding this comment.
"when you can pass a start parameter:"
->
"when you can pass an offset parameter:"
?
I know this isn't something you changed in this PR, but it's odd we say start and then use offset just below in the example
snippets/paging-cursor.js
Outdated
| return Promise.all([response.items, z.cursor.set(response.nextPage)]); | ||
| }) | ||
| .then(promises => { | ||
| // [items[], null] |
There was a problem hiding this comment.
I'd go ahead and use destructuring here unless we're intentionally not doing that to try to keep the docs really simple
.then(([items, /* null */]) => {
return items;
});
snippets/paging-cursor.js
Outdated
| // --------------------------------------------------- | ||
|
|
||
| const performWithAsync = async (z, bundle) => { | ||
| const cursor = await z.cursor.get(); // string | null |
There was a problem hiding this comment.
we probably want to do something similar to performWithoutAsync:
let cursor;
if (bundle.meta.page) {
cursor = await z.cursor.get();
}
snippets/paging-cursor.js
Outdated
| // the perform method of our trigger | ||
| // ensure operation.canPaginate is true! | ||
|
|
||
| const performWihoutAsync = (z, bundle) => { |
README-source.md
Outdated
| [insert-file:./snippets/paging-cursor.js] | ||
| ``` | ||
|
|
||
| Cursors are stored per-zap and last about an hour. Per the above, make sure to only include the cursor if `bundle.meta.page === 0`, so you don't accidentally reuse a cursor from a previous poll. |
There was a problem hiding this comment.
Don't we only want to use the cursor if bundle.meta.page !== 0?
snippets/async-polling.js
Outdated
| let results = response.json; | ||
|
|
||
| // keep paging until the last item was created over two hours ago | ||
| // then we know we almost certainly havne't missed anything and can let |
snippets/async-polling.js
Outdated
| // then we know we almost certainly havne't missed anything and can let | ||
| // deduper handle the rest | ||
|
|
||
| while (new Date(results[results.length - 1].createdAt) < hoursAgo) { |
There was a problem hiding this comment.
isn't this backwards? posts newer than 2 hours ago will have createdAt > hoursAgo, yeah (assuming descending date sort, so we're counting down)?
also, I'd suggest having a "bail out" counter var in case something goes wrong, like
let cycles = 0;
while (
new Date(results[results.length - 1].createdAt) > hoursAgo &&
cycles++ < 10
) {
// ...
}There was a problem hiding this comment.
good call on the <! I'm going to leave off the bailout because it might lead to missed triggers and overcomplicates the docs
| if (bundle.meta.page === 0) { | ||
| // first page, no need to fetch a cursor | ||
| return Promise.resolve(); | ||
| } else { |
There was a problem hiding this comment.
unnecessary else — since the if has a return, you can just return z.cursor.get() directly at the top of this .then() callback's scope
There was a problem hiding this comment.
fair point. I know that's what we usually do, but in the interest of simplicity, i'm going to leave as-is unless you care a lot
There was a problem hiding this comment.
fine by me to leave as-is 👍
snippets/paging-cursor.js
Outdated
| const response = await z.request( | ||
| 'https://5ae7ad3547436a00143e104d.mockapi.io/api/recipes', | ||
| { | ||
| params: { cursor: cursor } // if cursor is null, it's ignored here |
There was a problem hiding this comment.
if cursor is null
Same when it's undefined? Since that's the more likely scenario (I think?), I'd lean towards stating that directly.
> null == undefined
true
> null === undefined
false
There was a problem hiding this comment.
I dug in here - we use node's querystring module. non-zero falsy values get passed in without values:
_limit=3&_start=3&null=&undef=&zero=0&emptyStr=
That said, the cursor response explicitly returns null when there's no cursor value. I can update that doc to be more clear about what happens to null values in a querystring though, since it could (though i don't think it's supposed to) matter on the external server
|
Woof, that was a weird one. travis was only failing on node 8 because of a missing dep. Not sure what the bug exactly is, but there's an npm bug where some required dependencies aren't in the package-lock.json. The fix: run Maybe we should switch these to yarn. |
This will go out with zapier/zapier-platform-core#76.
As with all docs, the PR is overly messy. The only important pieces are the snippet files and the two blurbs in
README-sourceJacob, requested you since you were in a ticket where someone was complaining about how confusing the original paging snippet was. Also with docs, it'a always nice to get fresh eyes to see if you think you could come in and use this feature with only these to go on.
Thanks a bunch!