search

orval-labs / orval

orval is able to generate client with appropriate type-signatures (TypeScript) from any valid OpenAPI v3 or Swagger v2 specification, either in yaml or json formats. 🍺
https://orval.dev
MIT License
3.18k stars 336 forks source link

feat: improve type-safety (like the queryOptions type-helper) #1672

Closed AmitBar2001 closed 3 weeks ago

AmitBar2001 commented 4 weeks ago

Status

READY

Description

Fix #1671

following the implementation described here: https://tkdodo.eu/blog/the-query-options-api#datatag

note: this is the first PR I've opened, I would very much appreciate your feedback.

Related PRs

List related PRs against other branches:

branch PR
other_pr_production [link]()
other_pr_master [link]()

Todos

Steps to Test or Reproduce

Outline the steps to test or reproduce the PR here.

> git pull --prune
> git checkout <branch>
> grunt jasmine

1.

melloware commented 4 weeks ago

looks like something is messed up with the build?

AmitBar2001 commented 4 weeks ago

looks like something is messed up with the build?

Yes, it fails in the install stage. It seems to me unrelated to this PR's changes as package.json files were not modified. image

I think I may have found the cause: https://github.com/orval-labs/orval/commit/87e11889758610ab36d7bca6e0c513c03055d0a8#diff-af75921c98100c36b0ba10db16be2bf8abd330024b5e63a9c219ffe4d9be5cceR17

The same dependencies that fail the install were bumped in this commit from 7.1.1 to 7.2.0, but it seems the yarn.lock file was not updated accordingly. I noticed this section in the ci log output: image

Maybe this is the reason this surfaced only now and not before, since this PR is from the fork I made.

Would you like me to add the update to the lockfile? (generated from my local yarn install)

-- Of course i did not pay attention and clicked sync changes on my fork repo, without installing locally and committing locally, which would probably have caught this change earlier

melloware commented 4 weeks ago

Yes please it was probably from last release on Monday.

melloware commented 4 weeks ago

ok now run npm run format to format the files. Probably also not your fault

melloware commented 4 weeks ago

All good now! Will just let @soartec-lab or @anymaniax review

soartec-lab commented 4 weeks ago

Thank you! I would like to see this PR, but I need a few days.