kuzzleio
feat: support trigger events option
This PR is intended to support triggerEvents request parameter for embeded sdk.
It is unit tested and will be functionnal tested when released on kuzzle side.
A lot of doc as been realigned with code
JIRA: KZLPRD-579
Why didn't you put this (duplicated) piece of code in the query method directly?
if (options.triggerEvents !== undefined) {
request.triggerEvents = options.triggerEvents;
}
I see that stuff like this is already done in this method: https://github.com/kuzzleio/sdk-javascript/blob/83e434b679f884604e567976967f31562ec5ba32/src/Kuzzle.ts#L901-L906
Why didn't you put this (duplicated) piece of code in the
querymethod directly?if (options.triggerEvents !== undefined) { request.triggerEvents = options.triggerEvents; }I see that stuff like this is already done in this method:
https://github.com/kuzzleio/sdk-javascript/blob/83e434b679f884604e567976967f31562ec5ba32/src/Kuzzle.ts#L901-L906
I've thought about that, it was an option but not all methods got options and it could lead to useless code for those. I thought it was less readable to apply it were needed but more optimized. If we decide to apply it directly on query it is doable for sure
Everything look good and updated.
There is just something odd about the :any you added to the request. Since you have the type for the KuzzleRequest isn't that possible to put the type instead of any ?
Or like Alex said, it could be possible to avoid duplication at the Kuzzle.ts file level
There's no KuzzleRequest type in js SDK
I could use Omit<RequestPayload, "controller"> wiches weird too but maybe more acceptable. Controller is abstracted and taken from controller name and it leads to ts errors without omitting
For maintainability purpose and after doing pro and cons, I'll stick to maintainability purpose where the query solution is better than little optim nit picking.
I will refactor it
