Uh oh!
There was an error while loading. Please reload this page.
fix(api): disable path-param queries explicitly, then bump orval to 8.26.0 - #685
Merged
Conversation
….26.0 orval changed the guard it generates for a required path param: - enabled: !!(id) + enabled: id !== null && id !== undefined That was a deliberate upstream fix, not a regression -- orval-labs/orval#3241 ("0 as param causes query to be disabled") reported that a legitimate `0` disabled the query, and #3325 replaced the truthiness test with a null check. It shipped in 8.10.0, so every version worth upgrading to has it. The consequence for us is that an empty string now passes the guard. Left alone, `usePackage('')` would issue `GET /api/packages/`. Two existing tests in hooks.test.tsx caught this, which is the only reason it was noticed. So the hooks now say what they mean instead of leaning on generated truthiness. This matches what the file already did elsewhere -- useGithubSearch has `enabled: query.length >= 2` and the watchlist hook has `enabled: !!user`: usePackage(id) enabled: !!id useRelease(id) enabled: !!id usePackageReleases(packageId) enabled: !!packageId usePackagesByOwner(owner) enabled: !!owner usePackageByOwnerRepo(owner, repo) enabled: !!(owner && repo) The generated code spreads `...queryOptions` after `enabled`, so the caller's value wins. Three of those five had no empty-param test, and the guards are now the only thing standing between an empty value and a malformed request, so they are covered directly -- each new test was confirmed to fail with its guard removed. On the version choice: 8.26.0 rather than 8.27.0 or 8.28.0. The current `latest`, 8.28.0, is uninstallable -- it depends on @orval/angular@8.28.0, which was never published (8.28.1 is queued upstream but not out). Between 8.26.0 and 8.27.0 the generated output differs only by extracting mutation keys into exported `get*MutationKey()` helpers, which nothing here uses, so 8.26.0 costs nothing and has eleven days of soak rather than five. Pinned exact, no caret, given they just shipped a broken `latest`. This does not unblock TypeScript 7. That is a separate problem -- orval rejects the aliased `typescript` spec as invalid semver -- and it is removed by the oxlint migration, not by any orval version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
orval changed the guard it generates for a required path param:
That was a deliberate upstream fix, not a regression -- orval-labs/orval#3241
("0 as param causes query to be disabled") reported that a legitimate
0disabled the query, and #3325 replaced the truthiness test with a null check.
It shipped in 8.10.0, so every version worth upgrading to has it.
The consequence for us is that an empty string now passes the guard. Left
alone,
usePackage('')would issueGET /api/packages/. Two existing testsin hooks.test.tsx caught this, which is the only reason it was noticed.
So the hooks now say what they mean instead of leaning on generated
truthiness. This matches what the file already did elsewhere -- useGithubSearch
has
enabled: query.length >= 2and the watchlist hook hasenabled: !!user:The generated code spreads
...queryOptionsafterenabled, so the caller'svalue wins. Three of those five had no empty-param test, and the guards are now
the only thing standing between an empty value and a malformed request, so they
are covered directly -- each new test was confirmed to fail with its guard
removed.
On the version choice: 8.26.0 rather than 8.27.0 or 8.28.0. The current
latest, 8.28.0, is uninstallable -- it depends on @orval/angular@8.28.0,which was never published (8.28.1 is queued upstream but not out). Between
8.26.0 and 8.27.0 the generated output differs only by extracting mutation
keys into exported
get*MutationKey()helpers, which nothing here uses, so8.26.0 costs nothing and has eleven days of soak rather than five. Pinned
exact, no caret, given they just shipped a broken
latest.This does not unblock TypeScript 7. That is a separate problem -- orval
rejects the aliased
typescriptspec as invalid semver -- and it is removedby the oxlint migration, not by any orval version.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Stack created with GitHub Stacks CLI • Give Feedback 💬