feat: add strict infinite hook generation#3106
Draft
the-inconvenience-store wants to merge 16 commits intoorval-labs:masterfrom
Draft
feat: add strict infinite hook generation#3106the-inconvenience-store wants to merge 16 commits intoorval-labs:masterfrom
the-inconvenience-store wants to merge 16 commits intoorval-labs:masterfrom
Conversation
Introduce useInfiniteRequireQueryParam to gate infinite and suspense-infinite - useInfiniteRequireQueryParam to query option types and normalisation - gate infinite/suspense-infite generation in query generator when strict mode is enabled - update framework adapaters (shared, angular and vue) to support strict pageParam injection - add unit tests for query option nroamlisation - add regression spec and config scenarios for react,vue, and angular query - include angular query in api snapshot harness closes orval-labs#3101
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEnhanced infinite-query generation to validate the configured query parameter exists in an endpoint's schema before emitting infinite-query hooks. Added schema-parsing utilities and conditional gating logic that inspects operation parameters, logs warnings when mismatches occur, and propagates a Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a strict mode option to Orval Query generation so infinite (and suspense-infinite) hooks are only generated for endpoints that actually declare the configured useInfiniteQueryParam in their query params, with adapter updates and new regression fixtures/snapshots across React/Vue/Angular Query.
Changes:
- Introduce
useInfiniteRequireQueryParamin query option types + normalization (@orval/core,@orval/orval,@orval/query). - Gate infinite/suspense-infinite hook generation when strict mode is enabled and the configured query param isn’t present on the endpoint.
- Add new test specification/config scenarios and extend snapshot harness to include Angular Query output.
Reviewed changes
Copilot reviewed 99 out of 99 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/specifications/infinite-require-query-param.yaml | New OpenAPI fixture to validate strict infinite param gating. |
| tests/configs/vue-query.config.ts | Adds a Vue Query config scenario enabling strict infinite behavior. |
| tests/configs/react-query.config.ts | Adds a React Query config scenario enabling strict infinite behavior. |
| tests/configs/angular-query.config.ts | Adds an Angular Query config scenario enabling strict infinite behavior. |
| tests/api-generation.spec.ts | Includes angular-query in snapshot generation harness. |
| packages/query/src/utils.ts | Adds normalization support for useInfiniteRequireQueryParam. |
| packages/query/src/utils.test.ts | Unit tests for query option normalization of the new flag. |
| packages/query/src/query-generator.ts | Implements strict gating + passes strictness through to adapters. |
| packages/query/src/frameworks/vue.ts | Updates Vue adapter to support strict pageParam injection behavior. |
| packages/query/src/frameworks/index.ts | Updates shared default adapter infinite param injection to support strict mode. |
| packages/query/src/frameworks/angular.ts | Updates Angular adapter infinite param injection to support strict mode. |
| packages/query/src/framework-adapter.ts | Extends adapter interface to accept requireQueryParam. |
| packages/orval/src/utils/options.ts | Normalizes useInfiniteRequireQueryParam at CLI/config layer. |
| packages/core/src/types.ts | Adds useInfiniteRequireQueryParam to QueryOptions/NormalizedQueryOptions. |
| tests/snapshots/vue-query/use-infinite-require-query-param/endpoints.ts | Snapshot for Vue Query strict infinite scenario. |
| tests/snapshots/vue-query/use-infinite-require-query-param/model/dashboardPageDto.ts | Snapshot model output for Vue Query scenario. |
| tests/snapshots/vue-query/use-infinite-require-query-param/model/dashboardPageDtoItemsItem.ts | Snapshot model output for Vue Query scenario. |
| tests/snapshots/vue-query/use-infinite-require-query-param/model/getFloorplanImageParams.ts | Snapshot model output for Vue Query scenario. |
| tests/snapshots/vue-query/use-infinite-require-query-param/model/getFloorplanImageVariant.ts | Snapshot model output for Vue Query scenario. |
| tests/snapshots/vue-query/use-infinite-require-query-param/model/index.ts | Snapshot model barrel for Vue Query scenario. |
| tests/snapshots/vue-query/use-infinite-require-query-param/model/listDashboardsParams.ts | Snapshot model output for Vue Query scenario. |
| tests/snapshots/react-query/use-infinite-require-query-param/endpoints.ts | Snapshot for React Query strict infinite scenario. |
| tests/snapshots/react-query/use-infinite-require-query-param/model/dashboardPageDto.ts | Snapshot model output for React Query scenario. |
| tests/snapshots/react-query/use-infinite-require-query-param/model/dashboardPageDtoItemsItem.ts | Snapshot model output for React Query scenario. |
| tests/snapshots/react-query/use-infinite-require-query-param/model/getFloorplanImageParams.ts | Snapshot model output for React Query scenario. |
| tests/snapshots/react-query/use-infinite-require-query-param/model/getFloorplanImageVariant.ts | Snapshot model output for React Query scenario. |
| tests/snapshots/react-query/use-infinite-require-query-param/model/index.ts | Snapshot model barrel for React Query scenario. |
| tests/snapshots/react-query/use-infinite-require-query-param/model/listDashboardsParams.ts | Snapshot model output for React Query scenario. |
| tests/snapshots/angular-query/use-infinite-require-query-param/endpoints.ts | Snapshot for Angular Query strict infinite scenario. |
| tests/snapshots/angular-query/use-infinite-require-query-param/model/dashboardPageDto.ts | Snapshot model output for Angular Query scenario. |
| tests/snapshots/angular-query/use-infinite-require-query-param/model/dashboardPageDtoItemsItem.ts | Snapshot model output for Angular Query scenario. |
| tests/snapshots/angular-query/use-infinite-require-query-param/model/getFloorplanImageParams.ts | Snapshot model output for Angular Query scenario. |
| tests/snapshots/angular-query/use-infinite-require-query-param/model/getFloorplanImageVariant.ts | Snapshot model output for Angular Query scenario. |
| tests/snapshots/angular-query/use-infinite-require-query-param/model/index.ts | Snapshot model barrel for Angular Query scenario. |
| tests/snapshots/angular-query/use-infinite-require-query-param/model/listDashboardsParams.ts | Snapshot model output for Angular Query scenario. |
| tests/snapshots/angular-query/tags-split/pets/pets.ts | Adds Angular Query snapshot coverage for tags-split mode. |
| tests/snapshots/angular-query/tags-split/health/health.ts | Adds Angular Query snapshot coverage for tags-split mode. |
| tests/snapshots/angular-query/tags-split/model/index.ts | Adds Angular Query model snapshots for tags-split mode. |
| tests/snapshots/angular-query/tags-split/model/* | Adds Angular Query model snapshot set for tags-split mode (pets + related types). |
| tests/snapshots/angular-query/split/endpoints.ts | Adds Angular Query snapshot coverage for split mode. |
| tests/snapshots/angular-query/split/model/index.ts | Adds Angular Query model snapshots for split mode. |
| tests/snapshots/angular-query/split/model/* | Adds Angular Query model snapshot set for split mode (pets + related types). |
| tests/snapshots/angular-query/basic/endpoints.ts | Adds Angular Query snapshot coverage for basic mode. |
| tests/snapshots/angular-query/basic/model/index.ts | Adds Angular Query model snapshots for basic mode. |
| tests/snapshots/angular-query/basic/model/* | Adds Angular Query model snapshot set for basic mode (pets + related types). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/__snapshots__/vue-query/use-infinite-require-query-param/endpoints.ts
Show resolved
Hide resolved
tests/__snapshots__/angular-query/use-infinite-require-query-param/endpoints.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
packages/query/src/utils.test.ts (1)
71-90: Consider adding a merge-precedence regression test.A case where global and per-query options set different
useInfiniteRequireQueryParamvalues would protect future refactors in option merge logic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/query/src/utils.test.ts` around lines 71 - 90, Add a regression test for merge precedence of useInfiniteRequireQueryParam: call normalizeQueryOptions with a globalOptions object (second arg or merged shape used by normalizeQueryOptions) setting useInfiniteRequireQueryParam to one value and pass a per-query options object to normalizeQueryOptions with the opposite value, then assert that the returned result follows the intended precedence (per-query option should override global). Reference normalizeQueryOptions and the useInfiniteRequireQueryParam option in the test to ensure future refactors preserve this override behavior.packages/query/src/utils.ts (1)
30-32: Validate strict-mode config dependencies early.At Line 30,
useInfiniteRequireQueryParamis normalized independently. Consider failing fast whenuseInfiniteRequireQueryParamistruebutuseInfiniteQueryParamis missing, to avoid silent misconfiguration during hook generation.Proposed guard in normalization
export const normalizeQueryOptions = ( queryOptions: QueryOptions = {}, outputWorkspace: string, ): NormalizedQueryOptions => { + if ( + queryOptions.useInfiniteRequireQueryParam && + !queryOptions.useInfiniteQueryParam + ) { + throw new Error( + 'useInfiniteRequireQueryParam requires useInfiniteQueryParam to be configured.', + ); + } + return {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/query/src/utils.ts` around lines 30 - 32, The normalization currently adds useInfiniteRequireQueryParam independently which can silently misconfigure hooks; update the normalization in utils.ts (the function that processes queryOptions) to validate and fail fast: if queryOptions.useInfiniteRequireQueryParam === true and the corresponding useInfiniteQueryParam (or normalizedUseInfiniteQueryParam) is not present/true, throw a clear Error (or invariant) naming both useInfiniteRequireQueryParam and useInfiniteQueryParam so callers see the missing dependency during generation instead of silently proceeding.packages/query/src/query-generator.ts (1)
63-78: Prefer structural/top-level key detection over full-model regex matching.Current detection relies on scanning
queryParams.schema.modeltext and can be brittle for complex/nested model shapes. A top-level structural check will make strict gating more reliable and avoid accidental matches.♻️ Suggested direction
-const hasQueryParamInSchema = ( - queryParams: GetterQueryParam | undefined, - queryParam: string, -): boolean => { - if (!queryParams?.schema.model) { - return false; - } - - const escapedParam = escapeRegExp(queryParam); - const paramKeyRegex = new RegExp( - `(?:^|\\n)\\s*(?:'${escapedParam}'|\"${escapedParam}\"|${escapedParam})\\??:\\s`, - 'm', - ); - - return paramKeyRegex.test(queryParams.schema.model); -}; +const hasQueryParamInSchema = ( + queryParams: GetterQueryParam | undefined, + queryParam: string, +): boolean => { + const model = queryParams?.schema.model; + if (!model) return false; + + // Restrict matching to top-level properties only + let depth = 0; + for (const rawLine of model.split('\n')) { + const line = rawLine.trim(); + depth += (line.match(/{/g)?.length ?? 0); + + if (depth === 1) { + const escapedParam = escapeRegExp(queryParam); + const topLevelKey = new RegExp( + `^(?:'${escapedParam}'|"${escapedParam}"|${escapedParam})\\??:\\s`, + ); + if (topLevelKey.test(line)) return true; + } + + depth -= (line.match(/}/g)?.length ?? 0); + } + + return false; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/query/src/query-generator.ts` around lines 63 - 78, Replace brittle regex scanning in hasQueryParamInSchema with a structural top-level key check: if queryParams?.schema.model is an object, test for the param by checking top-level keys (e.g., Object.prototype.hasOwnProperty.call(queryParams.schema.model, queryParam) or presence in schema.model.properties/top-level fields) instead of regex; only fall back to any string-based heuristic when schema.model is a string (but prefer parsing it into an object first or using a schema parser) so hasQueryParamInSchema reliably detects top-level queryParam presence in queryParams.schema.model.tests/__snapshots__/angular-query/basic/endpoints.ts (1)
28-36: ImportingErrorshadows the globalErrorconstructor.The imported
Errortype from the model shadows JavaScript's built-inErrorclass. If any code in this file needs to throw or catch standard errors, it would inadvertently reference the imported type instead.Since this is auto-generated code, the fix should be applied in the Orval generator—consider renaming the imported type (e.g.,
ApiError,PetStoreError) to avoid shadowing the global.🔧 Suggested approach (in generator)
import type { CreatePetsBody, CreatePetsParams, - Error, + Error as ApiError, ListPetsParams, Pet, PetWithTag, Pets, } from './model';Then update all usages of
ErrortoApiErrorthroughout the generated file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/__snapshots__/angular-query/basic/endpoints.ts` around lines 28 - 36, The imported type named Error from the model shadows the global JavaScript Error constructor; update the generator to rename this type (e.g., to ApiError or PetStoreError) and change the import in the generated file so it reads the new name instead of Error, then update all references in the generated code that used Error (e.g., type annotations, return types, and interfaces in endpoints.ts such as any usage alongside CreatePetsBody, CreatePetsParams, ListPetsParams, Pet, PetWithTag, Pets) to the new ApiError name so the global Error remains unshadowed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/query/src/frameworks/angular.ts`:
- Around line 97-100: The current construction overwrites caller-provided seed
values because when requireQueryParam is true it sets params[queryParam] to
pageParam even if pageParam is undefined; update the expression so that when
requireQueryParam is true you use the nullish-coalescing form (pageParam ??
params?.['${queryParam}']) instead of just pageParam to preserve an explicit
seed while keeping strict semantics; apply this change to the same expression in
the adapters that use the variables queryParam, requireQueryParam, pageParam and
params (present in angular.ts, vue.ts and index.ts) so all infinite-query
branches behave the same.
In `@tests/__snapshots__/angular-query/tags-split/health/health.ts`:
- Line 22: The imported schema name Error shadows the global Error; change the
import "import type { Error }" to a non-conflicting alias such as "import type {
Error as ErrorModel }" and update all local references from Error to ErrorModel
(e.g., the usages currently named Error in this file). Instead of editing
snapshots directly, update the Angular generator/template that emits this import
so generated files use the aliased name (ErrorModel) consistently across all
generated outputs (including the other snapshot file mentioned).
In
`@tests/__snapshots__/angular-query/use-infinite-require-query-param/endpoints.ts`:
- Around line 15-23: The import block is missing
CreateSuspenseInfiniteQueryOptions from `@tanstack/angular-query-experimental`
which causes compile failures where CreateSuspenseInfiniteQueryOptions is
referenced; update the type import list (the block that currently imports
CreateInfiniteQueryOptions, CreateInfiniteQueryResult, CreateQueryOptions,
CreateQueryResult, InfiniteData, QueryFunction, QueryKey) to also include
CreateSuspenseInfiniteQueryOptions so the referenced types resolve for the
functions/classes using CreateSuspenseInfiniteQueryOptions.
In
`@tests/__snapshots__/react-query/use-infinite-require-query-param/endpoints.ts`:
- Around line 39-47: The test stub currently declares the image response types
(getFloorplanImageResponse200 / getFloorplanImageResponseSuccess /
getFloorplanImageResponse) as Blob but the implementation is parsing the body as
text and running JSON.parse (which fails for binary image payloads); update the
response handling where the endpoint reads the body to call res.blob() instead
of res.text()/JSON.parse, return that Blob on success, and ensure any caller
expecting getFloorplanImageResponse uses the Blob value; apply the same change
to the other affected endpoint handling around the 68-85 region.
In `@tests/__snapshots__/vue-query/use-infinite-require-query-param/endpoints.ts`:
- Around line 7-18: The file is missing imports for the suspense-infinite API
used later; add the missing symbols from '@tanstack/vue-query'—specifically
import UseSuspenseInfiniteQueryOptions and useSuspenseInfiniteQuery (and any
related return types like UseSuspenseInfiniteQueryReturnType if used) into the
existing import list so references to UseSuspenseInfiniteQueryOptions and
useSuspenseInfiniteQuery compile.
---
Nitpick comments:
In `@packages/query/src/query-generator.ts`:
- Around line 63-78: Replace brittle regex scanning in hasQueryParamInSchema
with a structural top-level key check: if queryParams?.schema.model is an
object, test for the param by checking top-level keys (e.g.,
Object.prototype.hasOwnProperty.call(queryParams.schema.model, queryParam) or
presence in schema.model.properties/top-level fields) instead of regex; only
fall back to any string-based heuristic when schema.model is a string (but
prefer parsing it into an object first or using a schema parser) so
hasQueryParamInSchema reliably detects top-level queryParam presence in
queryParams.schema.model.
In `@packages/query/src/utils.test.ts`:
- Around line 71-90: Add a regression test for merge precedence of
useInfiniteRequireQueryParam: call normalizeQueryOptions with a globalOptions
object (second arg or merged shape used by normalizeQueryOptions) setting
useInfiniteRequireQueryParam to one value and pass a per-query options object to
normalizeQueryOptions with the opposite value, then assert that the returned
result follows the intended precedence (per-query option should override
global). Reference normalizeQueryOptions and the useInfiniteRequireQueryParam
option in the test to ensure future refactors preserve this override behavior.
In `@packages/query/src/utils.ts`:
- Around line 30-32: The normalization currently adds
useInfiniteRequireQueryParam independently which can silently misconfigure
hooks; update the normalization in utils.ts (the function that processes
queryOptions) to validate and fail fast: if
queryOptions.useInfiniteRequireQueryParam === true and the corresponding
useInfiniteQueryParam (or normalizedUseInfiniteQueryParam) is not present/true,
throw a clear Error (or invariant) naming both useInfiniteRequireQueryParam and
useInfiniteQueryParam so callers see the missing dependency during generation
instead of silently proceeding.
In `@tests/__snapshots__/angular-query/basic/endpoints.ts`:
- Around line 28-36: The imported type named Error from the model shadows the
global JavaScript Error constructor; update the generator to rename this type
(e.g., to ApiError or PetStoreError) and change the import in the generated file
so it reads the new name instead of Error, then update all references in the
generated code that used Error (e.g., type annotations, return types, and
interfaces in endpoints.ts such as any usage alongside CreatePetsBody,
CreatePetsParams, ListPetsParams, Pet, PetWithTag, Pets) to the new ApiError
name so the global Error remains unshadowed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 16d58673-f16c-4e9d-a8f9-17e374fb4e6a
📒 Files selected for processing (99)
packages/core/src/types.tspackages/orval/src/utils/options.tspackages/query/src/framework-adapter.tspackages/query/src/frameworks/angular.tspackages/query/src/frameworks/index.tspackages/query/src/frameworks/vue.tspackages/query/src/query-generator.tspackages/query/src/utils.test.tspackages/query/src/utils.tstests/__snapshots__/angular-query/basic/endpoints.tstests/__snapshots__/angular-query/basic/model/cat.tstests/__snapshots__/angular-query/basic/model/catType.tstests/__snapshots__/angular-query/basic/model/createPetsBody.tstests/__snapshots__/angular-query/basic/model/createPetsParams.tstests/__snapshots__/angular-query/basic/model/createPetsSort.tstests/__snapshots__/angular-query/basic/model/dachshund.tstests/__snapshots__/angular-query/basic/model/dachshundBreed.tstests/__snapshots__/angular-query/basic/model/dog.tstests/__snapshots__/angular-query/basic/model/dogType.tstests/__snapshots__/angular-query/basic/model/error.tstests/__snapshots__/angular-query/basic/model/index.tstests/__snapshots__/angular-query/basic/model/labradoodle.tstests/__snapshots__/angular-query/basic/model/labradoodleBreed.tstests/__snapshots__/angular-query/basic/model/listPetsParams.tstests/__snapshots__/angular-query/basic/model/listPetsSort.tstests/__snapshots__/angular-query/basic/model/pet.tstests/__snapshots__/angular-query/basic/model/petCallingCode.tstests/__snapshots__/angular-query/basic/model/petCountry.tstests/__snapshots__/angular-query/basic/model/petWithTag.tstests/__snapshots__/angular-query/basic/model/pets.tstests/__snapshots__/angular-query/split/endpoints.tstests/__snapshots__/angular-query/split/model/cat.tstests/__snapshots__/angular-query/split/model/catType.tstests/__snapshots__/angular-query/split/model/createPetsBody.tstests/__snapshots__/angular-query/split/model/createPetsParams.tstests/__snapshots__/angular-query/split/model/createPetsSort.tstests/__snapshots__/angular-query/split/model/dachshund.tstests/__snapshots__/angular-query/split/model/dachshundBreed.tstests/__snapshots__/angular-query/split/model/dog.tstests/__snapshots__/angular-query/split/model/dogType.tstests/__snapshots__/angular-query/split/model/error.tstests/__snapshots__/angular-query/split/model/index.tstests/__snapshots__/angular-query/split/model/labradoodle.tstests/__snapshots__/angular-query/split/model/labradoodleBreed.tstests/__snapshots__/angular-query/split/model/listPetsParams.tstests/__snapshots__/angular-query/split/model/listPetsSort.tstests/__snapshots__/angular-query/split/model/pet.tstests/__snapshots__/angular-query/split/model/petCallingCode.tstests/__snapshots__/angular-query/split/model/petCountry.tstests/__snapshots__/angular-query/split/model/petWithTag.tstests/__snapshots__/angular-query/split/model/pets.tstests/__snapshots__/angular-query/tags-split/health/health.tstests/__snapshots__/angular-query/tags-split/model/cat.tstests/__snapshots__/angular-query/tags-split/model/catType.tstests/__snapshots__/angular-query/tags-split/model/createPetsBody.tstests/__snapshots__/angular-query/tags-split/model/createPetsParams.tstests/__snapshots__/angular-query/tags-split/model/createPetsSort.tstests/__snapshots__/angular-query/tags-split/model/dachshund.tstests/__snapshots__/angular-query/tags-split/model/dachshundBreed.tstests/__snapshots__/angular-query/tags-split/model/dog.tstests/__snapshots__/angular-query/tags-split/model/dogType.tstests/__snapshots__/angular-query/tags-split/model/error.tstests/__snapshots__/angular-query/tags-split/model/index.tstests/__snapshots__/angular-query/tags-split/model/labradoodle.tstests/__snapshots__/angular-query/tags-split/model/labradoodleBreed.tstests/__snapshots__/angular-query/tags-split/model/listPetsParams.tstests/__snapshots__/angular-query/tags-split/model/listPetsSort.tstests/__snapshots__/angular-query/tags-split/model/pet.tstests/__snapshots__/angular-query/tags-split/model/petCallingCode.tstests/__snapshots__/angular-query/tags-split/model/petCountry.tstests/__snapshots__/angular-query/tags-split/model/petWithTag.tstests/__snapshots__/angular-query/tags-split/model/pets.tstests/__snapshots__/angular-query/tags-split/pets/pets.tstests/__snapshots__/angular-query/use-infinite-require-query-param/endpoints.tstests/__snapshots__/angular-query/use-infinite-require-query-param/model/dashboardPageDto.tstests/__snapshots__/angular-query/use-infinite-require-query-param/model/dashboardPageDtoItemsItem.tstests/__snapshots__/angular-query/use-infinite-require-query-param/model/getFloorplanImageParams.tstests/__snapshots__/angular-query/use-infinite-require-query-param/model/getFloorplanImageVariant.tstests/__snapshots__/angular-query/use-infinite-require-query-param/model/index.tstests/__snapshots__/angular-query/use-infinite-require-query-param/model/listDashboardsParams.tstests/__snapshots__/react-query/use-infinite-require-query-param/endpoints.tstests/__snapshots__/react-query/use-infinite-require-query-param/model/dashboardPageDto.tstests/__snapshots__/react-query/use-infinite-require-query-param/model/dashboardPageDtoItemsItem.tstests/__snapshots__/react-query/use-infinite-require-query-param/model/getFloorplanImageParams.tstests/__snapshots__/react-query/use-infinite-require-query-param/model/getFloorplanImageVariant.tstests/__snapshots__/react-query/use-infinite-require-query-param/model/index.tstests/__snapshots__/react-query/use-infinite-require-query-param/model/listDashboardsParams.tstests/__snapshots__/vue-query/use-infinite-require-query-param/endpoints.tstests/__snapshots__/vue-query/use-infinite-require-query-param/model/dashboardPageDto.tstests/__snapshots__/vue-query/use-infinite-require-query-param/model/dashboardPageDtoItemsItem.tstests/__snapshots__/vue-query/use-infinite-require-query-param/model/getFloorplanImageParams.tstests/__snapshots__/vue-query/use-infinite-require-query-param/model/getFloorplanImageVariant.tstests/__snapshots__/vue-query/use-infinite-require-query-param/model/index.tstests/__snapshots__/vue-query/use-infinite-require-query-param/model/listDashboardsParams.tstests/api-generation.spec.tstests/configs/angular-query.config.tstests/configs/react-query.config.tstests/configs/vue-query.config.tstests/specifications/infinite-require-query-param.yaml
tests/__snapshots__/angular-query/use-infinite-require-query-param/endpoints.ts
Outdated
Show resolved
Hide resolved
tests/__snapshots__/react-query/use-infinite-query-param-implicit/endpoints.ts
Outdated
Show resolved
Hide resolved
tests/__snapshots__/vue-query/use-infinite-require-query-param/endpoints.ts
Show resolved
Hide resolved
Collaborator
|
@the-inconvenience-store looks like build is failing and CodeRabbit suggestions? |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/query/src/query-generator.ts`:
- Around line 63-78: The current hasQueryParamInSchema uses a regex
(paramKeyRegex) against queryParams.schema.model which over-matches nested keys
(e.g., properties with the same name deeper in the type), causing false
positives; update the check to reliably detect only top-level properties by
parsing the TypeScript/JSON model text instead of relying solely on regex, e.g.,
use the TypeScript compiler API / ts-morph to parse queryParams.schema.model and
inspect the top-level type or object literal properties for queryParam, or if
you must keep a text-based approach tighten the match in hasQueryParamInSchema
so the pattern disallows matches preceded by a dot/identifier or nested context
(ensure no '.' or identifier char immediately before the match and require it to
be a direct property of the top-level object); change function
hasQueryParamInSchema to use the parser-based top-level-key check (or stricter
regex) on queryParams.schema.model and remove the existing paramKeyRegex usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e371193-4f98-49e8-a884-b6577403bc42
📒 Files selected for processing (5)
packages/orval/src/utils/options.tspackages/query/src/frameworks/angular.tspackages/query/src/frameworks/index.tspackages/query/src/frameworks/vue.tspackages/query/src/query-generator.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/query/src/frameworks/angular.ts
This reverts commit 098504a.
Signed-off-by: Sam Stevens <6634981+the-inconvenience-store@users.noreply.github.com>
Collaborator
|
@the-inconvenience-store is there any way to implement this without another new param? Also if there is a new parameter it means you need to update the |
Author
yeah i just didn't want to make any changes that might confuse/break people's current setups. of course it could be automatic, so i suppose it's up to you if you want to release it as breaking change for v9 instead of a minor feat. let me know your preference and i'll make edits accordingly cheers |
Collaborator
|
so my thoguht is if this is a string |
Author
would you rather: ...
useInfiniteQueryParam : {
queryParam: string // <-- current query param behaviour
strictMode: bool // <-- toggle new behaviour
} | string // <-- backwards compat with current query param behaviour |
Collaborator
|
I think i like your implicitly apply this behavior option. |
Author
🫡 will push my changes soon, cheers! |
…param is present BREAKING CHANGE
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/query/src/query-generator.ts`:
- Around line 898-905: The warning is emitted whenever useInfiniteQueryParam is
configured but missing, even for operations that never request an infinite hook;
update the guard so createLogger().warn(...) only runs when this specific
operation actually requests an infinite query (introduce or reuse a
per-operation boolean like operationRequestsInfinite) — e.g. only call
createLogger().warn if operationRequestsInfinite && effectiveInfiniteQueryParam
&& !hasConfiguredInfiniteQueryParam, and compute canGenerateInfiniteQuery
accordingly (e.g. const canGenerateInfiniteQuery = !operationRequestsInfinite ?
true : (!effectiveInfiniteQueryParam || hasConfiguredInfiniteQueryParam)); apply
the same change to the other similar block(s) around the
canGenerateInfiniteQuery logic (references: effectiveInfiniteQueryParam,
hasConfiguredInfiniteQueryParam, createLogger().warn, canGenerateInfiniteQuery,
operationName).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ffc2f7b7-8da6-4983-8e4e-6fa7f7b9286d
⛔ Files ignored due to path filters (14)
samples/svelte-query/custom-fetch/src/gen/models/cat.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/catType.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/createPetsBodyItem.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/dachshund.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/dachshundBreed.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/dog.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/dogType.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/error.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/index.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/listPetsParams.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/pet.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/models/pets.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/pets/pets.msw.tsis excluded by!**/gen/**samples/svelte-query/custom-fetch/src/gen/pets/pets.tsis excluded by!**/gen/**
📒 Files selected for processing (45)
docs/content/docs/reference/configuration/output.mdxpackages/query/src/framework-adapter.tspackages/query/src/query-generator.tssamples/angular-app/src/api/endpoints/index.msw.tssamples/angular-app/src/api/endpoints/pets/pets.msw.tssamples/angular-app/src/api/endpoints/pets/pets.service.tssamples/angular-app/src/api/model/createPetsBody.tssamples/angular-app/src/api/model/createPetsBodyStatus.tssamples/angular-app/src/api/model/error.tssamples/angular-app/src/api/model/index.tssamples/angular-app/src/api/model/listPetsParams.tssamples/angular-app/src/api/model/pet.tssamples/angular-app/src/api/model/petStatus.tssamples/angular-app/src/api/model/pets.tssamples/angular-app/src/api/model/searchPetsParams.tssamples/angular-app/src/api/model/searchPetsStatus.tssamples/react-query/basic/src/api/endpoints/petstoreFromFileSpecWithTransformer.tssamples/vue-query/vue-query-basic/src/api/endpoints/petstoreFromFileSpecWithTransformer.tstests/__snapshots__/react-query/error-type/endpoints.tstests/__snapshots__/react-query/mutator-client/endpoints.tstests/__snapshots__/react-query/mutator-multi-arguments/endpoints.tstests/__snapshots__/react-query/mutator/endpoints.tstests/__snapshots__/react-query/use-infinite-query-param-implicit/endpoints.tstests/__snapshots__/react-query/use-infinite-query-param-implicit/model/dashboardPageDto.tstests/__snapshots__/react-query/use-infinite-query-param-implicit/model/dashboardPageDtoItemsItem.tstests/__snapshots__/react-query/use-infinite-query-param-implicit/model/getFloorplanImageParams.tstests/__snapshots__/react-query/use-infinite-query-param-implicit/model/getFloorplanImageVariant.tstests/__snapshots__/react-query/use-infinite-query-param-implicit/model/index.tstests/__snapshots__/react-query/use-infinite-query-param-implicit/model/listDashboardsParams.tstests/__snapshots__/svelte-query/mutator/endpoints.tstests/__snapshots__/svelte-query/named-parameters/endpoints.tstests/__snapshots__/svelte-query/petstore/endpoints.tstests/__snapshots__/vue-query/mutator/endpoints.tstests/__snapshots__/vue-query/petstore/endpoints.tstests/__snapshots__/vue-query/use-infinite-query-param-implicit/endpoints.tstests/__snapshots__/vue-query/use-infinite-query-param-implicit/model/dashboardPageDto.tstests/__snapshots__/vue-query/use-infinite-query-param-implicit/model/dashboardPageDtoItemsItem.tstests/__snapshots__/vue-query/use-infinite-query-param-implicit/model/getFloorplanImageParams.tstests/__snapshots__/vue-query/use-infinite-query-param-implicit/model/getFloorplanImageVariant.tstests/__snapshots__/vue-query/use-infinite-query-param-implicit/model/index.tstests/__snapshots__/vue-query/use-infinite-query-param-implicit/model/listDashboardsParams.tstests/configs/angular-query.config.tstests/configs/react-query.config.tstests/configs/vue-query.config.tstests/specifications/infinite-query-param-implicit.yaml
💤 Files with no reviewable changes (13)
- samples/angular-app/src/api/model/createPetsBody.ts
- samples/angular-app/src/api/endpoints/index.msw.ts
- samples/angular-app/src/api/model/listPetsParams.ts
- samples/angular-app/src/api/model/pet.ts
- samples/angular-app/src/api/model/pets.ts
- samples/angular-app/src/api/model/searchPetsParams.ts
- samples/angular-app/src/api/model/createPetsBodyStatus.ts
- samples/angular-app/src/api/model/petStatus.ts
- samples/angular-app/src/api/model/index.ts
- samples/angular-app/src/api/model/searchPetsStatus.ts
- samples/angular-app/src/api/model/error.ts
- samples/angular-app/src/api/endpoints/pets/pets.msw.ts
- samples/angular-app/src/api/endpoints/pets/pets.service.ts
✅ Files skipped from review due to trivial changes (13)
- tests/snapshots/react-query/use-infinite-query-param-implicit/model/dashboardPageDtoItemsItem.ts
- docs/content/docs/reference/configuration/output.mdx
- tests/snapshots/vue-query/use-infinite-query-param-implicit/model/getFloorplanImageParams.ts
- tests/snapshots/react-query/use-infinite-query-param-implicit/model/listDashboardsParams.ts
- tests/snapshots/react-query/use-infinite-query-param-implicit/model/dashboardPageDto.ts
- tests/snapshots/react-query/use-infinite-query-param-implicit/model/getFloorplanImageParams.ts
- tests/snapshots/vue-query/use-infinite-query-param-implicit/model/listDashboardsParams.ts
- tests/snapshots/react-query/use-infinite-query-param-implicit/model/index.ts
- tests/snapshots/vue-query/use-infinite-query-param-implicit/model/dashboardPageDtoItemsItem.ts
- tests/snapshots/vue-query/use-infinite-query-param-implicit/model/dashboardPageDto.ts
- tests/snapshots/vue-query/use-infinite-query-param-implicit/model/getFloorplanImageVariant.ts
- tests/snapshots/react-query/use-infinite-query-param-implicit/model/getFloorplanImageVariant.ts
- tests/snapshots/vue-query/use-infinite-query-param-implicit/model/index.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/query/src/framework-adapter.ts
- tests/configs/vue-query.config.ts
- tests/configs/angular-query.config.ts
Collaborator
|
looks like Snapshots are failing now |
Collaborator
|
looks like conflicts need to be resolved |
Author
|
@melloware resolved, cheers :) |
Collaborator
|
Looks like SNAPSHOTs are failing now |
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 free
to 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.
Introduce useInfiniteRequireQueryParam to gate infinite and suspense-infinite hook generation to endpoints that actually include the configured useInfiniteQueryParam
closes #3101
Not sure if there was a reason why
angular-querywas excluded for the snapshot testing harness, but this PR does re-introduce it as I was testing the changes in Angular.If desired I can push a commit with angular-query snapshots disabled and removed again.
Thanks
Summary by CodeRabbit
New Features
Documentation