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

Open Beginner friendly
#322 4 comments 3 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
65/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Stale
Tech stack
javascript, typescript
Domain
api, cli, tooling

Research direction

The issue is in src/routes.js lines 295-302 and templates/default/procedure-call.eta lines 24-34. Start by understanding how requestParams is generated and used. The fix likely involves modifying the condition in routes.js to consider path params when extractRequestParams is true. Test by running the generator on a spec with only path params to verify the params object is created.

Written by the indexing model from the issue text.

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:

- if (!queryParams || !queryParams.length) return null;
+ if (!extractRequestParams && (!queryParams || !queryParams.length)) return null;
Dominant language
TypeScript
Stars
4.1k
Forks
436
Avg merge
9d 7h
Merged PRs (30d)
3

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from acacode/swagger-typescript-api

All issues in acacode/swagger-typescript-api

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.