acacode / acacode/swagger-typescript-api
--extract-request-params behavior depends on presence of query params
- Linguagem predominante
- TypeScript
- Estrelas
- 4.1k
- Forks
- 436
- Métricas de merge de PRs
- Nenhum PR com merge em 30d
Descrição
The procedure call template uses the presence of `requestParams` to decide whether to actually do the `--extract-request-params` behavior, i.e., put path _and_ query params in a single object that is the first argument to the request function.
If `requestParams` is falsy, we essentially fall back to the default behavior.
https://github.com/acacode/swagger-typescript-api/blob/7df956c236d1e7fc051658cd7012c8ea22286b44/templates/default/procedure-call.eta#L24-L34
The problem (for me, anyway) is that `requestParams` is always null for a given route if the route takes no query params:
https://github.com/acacode/swagger-typescript-api/blob/7df956c236d1e7fc051658cd7012c8ea22286b44/src/routes.js#L295-L302
So, for requests that only have path params but no query params, we will never generate a params object representing the path params, instead inlining them as individual arguments. I was able to confirm that commenting out `if (!queryParams || !queryParams.length) return null; ` produces the desired behavior.
This behavior is counterintuitive given the documentation for the flag. It seems clear that both path params and query params are meant to be considered "request params", which means `requestParams` should not be null if there are path params.
```
--extract-request-params extract request params to data contract (default: false)
Also combine path params and query params into one object
```
If getting rid of that check would break the default `extractRequestParams = false` case, then maybe something like this would be appropriate:
```diff
- if (!queryParams || !queryParams.length) return null;
+ if (!extractRequestParams && (!queryParams || !queryParams.length)) return null;
```
Guia de contribuição
Nenhum guia de contribuição indexado para este repositório
Avaliação
Esta issue ainda não foi avaliada.