Conversation
use pipe
lukechesser
left a comment
There was a problem hiding this comment.
Outside of the methods we don't implement, are there any breaking changes?
There's more! It's pretty much all changed. I'll need to write up a new readme of course. Biggest things:
A very quick overview of how to consume response from the new API: const api = Unsplash({ accessKey: 'abc123' });
api.photos.get({ photoId: 'foo' }).then(result => {
if (result.type === 'error') {
console.log('error occurred: ', result.errors[0]);
} else if (result.type === 'aborted') {
console.log('fetch request aborted');
} else {
console.log('received photo: ', result.response);
}
});One other thing I am also adding is for feed methods: the lib will automatically handle extracting the total from |
…into feature/typescript
…ne helpers" This reverts commit bc34062.
| type Query = { | ||
| [index: string]: string | number | boolean; | ||
| }; |
There was a problem hiding this comment.
Could we replace all instances of this with URLSearchParams?
There was a problem hiding this comment.
Not quite, because URLSearchParams is not a plain object but a class instance. So we'd have to upgrade our query arguments created in our methods to actually be new URLSearchParams(query). It's a bit too involved of a change so I'd rather leave that out for later.
There was a problem hiding this comment.
Yeah, I was suggesting using URLSearchParams everywhere, rather than try to maintain two representations of the same thing.
There was a problem hiding this comment.
I see. I've added it to #145 to tackle post-adding the response types.
| ...paginationParams | ||
| }: CollectionId & PaginationParams & OrientationParam) => ({ | ||
| pathname: `${COLLECTIONS_PATH_PREFIX}/${collectionId}/photos`, | ||
| query: compactDefined({ ...Query.getFeedParams(paginationParams), orientation }), |
There was a problem hiding this comment.
Do we need to use compactDefined here? I thought you decided to do that inside createRequestHandler?
Also applies to other places in this PR.
There was a problem hiding this comment.
Oh sorry. I ended up tightening the Query type to not allow any non-values after all, which required me to add compactDefined wherever it was needed in methods. We no longer need to compact in createRequestHandler, so I removed it from there just now.
Co-authored-by: Oliver Joseph Ash <oliverjash@gmail.com>
Co-authored-by: Oliver Joseph Ash <oliverjash@gmail.com>
Overview
TO-DO
Languageand string literal unions for args)urlmodule