acacode / acacode/swagger-typescript-api

--extract-request-params behavior depends on presence of query params

Open
#322 4 comments 3 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
4.1k
Forks
436
PR merge metrics
No merged PRs in 30d

Description

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;
```

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.