feat(javascript): expose requestOptions and cache options
#283
Conversation
✅ Deploy Preview for api-clients-automation canceled.
|
|
✗ The generated branch has been deleted. If the PR has been merged, you can check the generated code on the |
|
Can you make it ready for review please ? 😄 |
Yep I'm resolving conflicts first, sorry for the ping D: |
| const mergedData: Request['data'] = Array.isArray(baseRequest.data) | ||
| ? baseRequest.data | ||
| : { | ||
| ...baseRequest.data, | ||
| ...baseRequestOptions?.data, | ||
| }; | ||
| const request: Request = { | ||
| ...baseRequest, | ||
| data: mergedData, | ||
| }; | ||
| const requestOptions: RequestOptions = { | ||
| cacheable: baseRequestOptions?.cacheable, | ||
| timeout: baseRequestOptions?.timeout, | ||
| queryParameters: { | ||
| ...baseRequestOptions?.queryParameters, | ||
| ...methodOptions.queryParameters, | ||
| }, | ||
| headers: { | ||
| Accept: 'application/json', | ||
| ...baseRequestOptions?.headers, | ||
| ...methodOptions.headers, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Not a fan of this part, but those changes at the method level added like 1kb to the bundle size of each clients
There was a problem hiding this comment.
(Logic could definitely be extracted in a method)
| cacheable: baseRequestOptions?.cacheable, | ||
| timeout: baseRequestOptions?.timeout, |
There was a problem hiding this comment.
| cacheable: baseRequestOptions?.cacheable, | |
| timeout: baseRequestOptions?.timeout, | |
| ...baseRequestOptions, |
There was a problem hiding this comment.
It's indeed better but small size impact, so I've went with that instead
| ): Promise<TResponse> { | ||
| const mergedData: Request['data'] = Array.isArray(baseRequest.data) | ||
| ? baseRequest.data |
There was a problem hiding this comment.
Is it really useful to have optional data ? Do you have a use case in mind ?
There was a problem hiding this comment.
The only thing I could see is when an option is not available in the client but exists on tHe REST api.
e.g. if they try dev things on the engine, they can test it without changing clients.
It was already there in v4 so I've kept it
There was a problem hiding this comment.
Okay this is already handled by the customRequest endpoint but why not
🧭 What and Why
🎟 JIRA Ticket: https://algolia.atlassian.net/browse/APIC-176
Changes included:
Follow up of #274
Follow up of #281
requestsOptionson the method, like we currently have on the clients.cacheable, and more.optionscoming from 3 sources, doing it in thetransporterwas the less impactful solution (both for size and redudancy), but I'm still not a fan of it. Don't hesitate to suggest alternatives.Next steps:
There is a looot of duplicate types, we could have a small cleaning PR to try to re-use them.
🧪 Test
CI :D