chore: hey-api bump fixes generated code (#2049) #2066
Conversation
WalkthroughThe changes in this pull request involve modifications to several TypeScript files within the frontend structure. Key alterations include the removal of export statements from Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
deps-report 🔍Commit scanned: ee9055d Vulnerable dependencies4 dependencies have vulnerabilities 😱
Outdated dependencies38 outdated dependencies found (including 17 outdated major versions)😢
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
frontend/ext/varfish-api/src/lib/types.gen.ts (2)
3257-3257: Consider adding JSDoc documentationWhile the type is straightforward, adding JSDoc documentation would improve clarity about its intended usage.
+/** + * Type alias for an array of strings, used for string list fields in the API. + */ export type strList = Array<(string)>;
1833-1836: Consider adding JSDoc documentationThe type would benefit from JSDoc documentation explaining the purpose of GC content statistics and field meanings.
+/** + * Type representing GC content statistics from samtools stats output. + * Each record contains: + * - gc_content: The GC content percentage + * - count: Number of reads/bases with this GC content + */ export type SamtoolsStatsGcRecordList = Array<{ gc_content: number; count: number; }>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
- frontend/ext/varfish-api/src/lib/index.ts (0 hunks)
- frontend/ext/varfish-api/src/lib/types.gen.ts (14 hunks)
- frontend/ext/viguno-api/src/lib/@tanstack/vue-query.gen.ts (2 hunks)
- frontend/ext/viguno-api/src/lib/index.ts (0 hunks)
- frontend/ext/viguno-api/src/lib/schemas.gen.ts (0 hunks)
- frontend/ext/viguno-api/src/lib/services.gen.ts (2 hunks)
- frontend/ext/viguno-api/src/lib/types.gen.ts (1 hunks)
- frontend/package.json (2 hunks)
💤 Files with no reviewable changes (3)
- frontend/ext/varfish-api/src/lib/index.ts
- frontend/ext/viguno-api/src/lib/index.ts
- frontend/ext/viguno-api/src/lib/schemas.gen.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/ext/viguno-api/src/lib/services.gen.ts
🔇 Additional comments (8)
frontend/ext/viguno-api/src/lib/@tanstack/vue-query.gen.ts (3)
16-16: LGTM: Improved client configuration flexibility.The change to use
options?.client ?? clientis a good improvement that allows for client override while maintaining backward compatibility.
39-52: LGTM: Well-structured query options with proper signal handling.The implementation is consistent across all HPO-related functions with proper request cancellation support via the signal parameter and consistent error handling.
Also applies to: 58-71, 77-90, 96-109, 115-128
41-47: Verify signal handling in consuming components.The signal parameter is now properly passed to the service calls. Ensure that components using these queries properly handle cleanup by aborting queries when components are unmounted.
Also applies to: 60-66, 79-85, 98-104, 117-123
✅ Verification successful
The previous search might have failed because we were looking for option types rather than actual query usage. Let's try to find where these queries are actually being used in components to verify the cleanup handling.
Let's try a different approach by finding components that import from this generated query file, as the previous patterns might have been too specific.
Let's examine the actual usage patterns in the files that import from vue-query.gen, focusing on both query usage and lifecycle hooks.
Signal handling is properly implemented through Vue Query's built-in cleanup
The codebase uses TanStack Vue Query's
useQueryhooks which automatically handle cleanup and request cancellation when components are unmounted. The implementation shows:
- Proper usage of Vue Query's built-in signal handling through the generated options
- Correct implementation of
enabledconditions to prevent unnecessary requests- Appropriate usage of stale times and query invalidation
No manual cleanup is required as Vue Query internally manages the AbortController signals and cleanup on component unmount.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components using these HPO queries to verify cleanup rg -l "hpo(Genes|Omims|SimTermGene|SimTermTerm|Terms)Options" | xargs rg -A 10 "onUnmounted|watchEffect|watch"Length of output: 112
Script:
#!/bin/bash # Let's first find files that use these HPO queries rg -l "useQuery.*hpo(Genes|Omims|SimTermGene|SimTermTerm|Terms)" # Then let's look for actual query usage patterns rg "useQuery.*hpo(Genes|Omims|SimTermGene|SimTermTerm|Terms)" -A 5Length of output: 135
Script:
#!/bin/bash # Let's search for Vue components that import from the vue-query.gen.ts file rg -l "from.*vue-query\.gen" # And then look for their cleanup patterns rg "from.*vue-query\.gen" --type vue -A 20Length of output: 425
Script:
#!/bin/bash # Let's examine the files that import from vue-query.gen to see their usage for file in $(rg -l 'from.*vue-query\.gen'); do echo "=== $file ===" rg -A 10 "useQuery|onMounted|onUnmounted|watchEffect|watch" "$file" done # Also let's check the direct imports from this file rg "import.*from.*vue-query\.gen" -A 3Length of output: 10753
frontend/package.json (2)
67-67: Verify the impact of @hey-api/openapi-ts upgrade.The upgrade from ^0.52.11 to ^0.54.0 involves multiple minor version increments in a 0.x package. Since this affects code generation:
- Review the generated TypeScript files for any breaking changes
- Ensure all removed exports (mentioned in the AI summary) were intentional
✅ Verification successful
Let me analyze the results from the shell scripts. The output shows:
The generated TypeScript files contain
anyandunknowntypes, but these are part of the API type definitions and error handling patterns, not type errors. They appear in:
- Documentation comments (e.g., "Base serializer for any SODAR model...")
- Error type definitions (e.g.,
export type CasesAnalysisApiCaseanalysisListError = unknown)- Valid enum choices (e.g.,
SeqvarsGenotypeChoice = 'any' | 'ref' | 'het'...)The imports from generated files are consistent across the codebase, with proper imports from:
@varfish-org/varfish-api/lib/@tanstack/vue-query.gen@varfish-org/varfish-api/lib/services.gen@varfish-org/varfish-api/lib/types.genThere are no signs of broken imports or type errors that would indicate breaking changes from the upgrade.
The upgrade from ^0.52.11 to ^0.54.0 appears safe to proceed with.
The generated types maintain proper type safety with intentional use of
unknownfor error states, and all imports remain valid across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Look for any type errors in the generated files rg -g 'frontend/ext/varfish-api/src/lib/*.gen.*' 'any|unknown' # Check for potentially broken imports of removed exports rg -g '!frontend/ext/varfish-api/src/lib/*.gen.*' "from ['\"].*\.gen['\"]"Length of output: 48462
19-19: Verify the impact of @hey-api/client-fetch upgrade.The upgrade from ^0.2.4 to ^0.4.2 is a significant minor version bump for a 0.x package, which may introduce breaking changes. Please ensure that:
- All generated code changes have been properly reviewed
- The application's integration tests pass with the new version
✅ Verification successful
Let me generate new shell scripts to better verify the impact of the package upgrade:
Let me fix the shell script syntax and try again:
Package upgrade appears safe to proceed
The
@hey-api/client-fetchpackage is used in generated code files and one store file. The generated files will be automatically updated during the build process, and the store file only uses a type import which is unlikely to have breaking changes. The minor version bump (0.2 -> 0.4) in a pre-1.0 package suggests possible breaking changes, but the actual usage in the codebase is minimal and well-contained.
- Generated files that will be updated:
- frontend/ext/varfish-api/src/lib/services.gen.ts
- frontend/ext/viguno-api/src/lib/services.gen.ts
- Single direct usage in frontend/src/seqvars/stores/presets/store.ts (type import only)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining references to old API patterns rg -g '!package*.json' '@hey-api/client-fetch.*0\.2' # Look for any TODO or FIXME comments in generated files that might indicate issues rg -g 'frontend/ext/varfish-api/src/lib/*.gen.*' '(TODO|FIXME)'Length of output: 48462
Script:
#!/bin/bash # Check for any direct imports of @hey-api/client-fetch rg -g '!package*.json' 'from ["\']@hey-api/client-fetch["\']' # Check for any direct usage of client-fetch APIs rg -g '!package*.json' '@hey-api/client-fetch\.' -A 2 # Look for any generated code files that might be affected fd -g '*.gen.*' -x ls -l {}Length of output: 93
Script:
#!/bin/bash # Check for any direct imports of @hey-api/client-fetch rg -g "!package*.json" "from ['\"]@hey-api/client-fetch['\"]" # Check for any direct usage of client-fetch APIs rg -g "!package*.json" "@hey-api/client-fetch\." -A 2 # Look for any generated code files that might be affected fd -e gen.ts -e gen.js -x ls -l {}Length of output: 1368
frontend/ext/viguno-api/src/lib/types.gen.ts (1)
87-97: Verify the distinction betweenResultandResult_types.The codebase now has two similar result types with subtle differences:
ResultusesResponseQueryfor the query fieldResult_usesQueryfor the query fieldThis distinction appears intentional to separate request and response structures, but let's verify this against the API specification.
✅ Verification successful
Based on the OpenAPI specification and code analysis, I can now provide a definitive response about the distinction between
ResultandResult_types:The distinction between
ResultandResult_types is intentional and correct.The OpenAPI specification shows that:
Result(title: HpoSimTermTermResult) is used for term-term similarity results withResponseQueryResult_(title: HpoTermsResult) is used for HPO terms results withQueryThis is a valid API design pattern where similar but distinct response types are used for different endpoints. The underscore suffix is part of the API specification and the generated types correctly reflect this design.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the OpenAPI specification to verify the type definitions # Look for Result type definitions in OpenAPI spec files fd -e yaml -e json . | xargs rg -A 10 "Result_?\:" # Check for usage patterns of both types rg "Result_?" --type=typescriptLength of output: 206
Script:
#!/bin/bash # Search for Result and Result_ type definitions rg "type Result" -A 10 --type ts # Search for usages of Result and Result_ rg "Result_" --type ts rg "Result[^_]" --type ts # Look for OpenAPI specs fd "openapi|swagger" -e yaml -e json -e ymlLength of output: 65037
Script:
#!/bin/bash # Check the OpenAPI spec for Result and Result_ type definitions cat frontend/ext/viguno-api/openapi.yaml | grep -A 10 "Result_\|Result:" # Search for usages of Result_ type in TypeScript files rg "Result_" --type ts -A 2 -B 2 # Search for imports of Result type rg "import.*Result" --type tsLength of output: 42961
frontend/ext/varfish-api/src/lib/types.gen.ts (2)
3224-3239: LGTM: Well-structured SODAR user type definitionThe SODARUser type is well-defined with proper field types, optionality, and comprehensive JSDoc documentation.
3241-3255: LGTM: Well-structured SODAR user request typeThe SODARUserRequest type appropriately omits the server-generated sodar_uuid field while maintaining the rest of the structure from SODARUser.
holtgrewe
deleted the
2049-bump-hey-api-dependency-to-fix-generated-code
branch
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2066 +/- ##
=====================================
Coverage 91% 91%
=====================================
Files 677 677
Lines 38435 38435
=====================================
Hits 35093 35093
Misses 3342 3342 |
Summary by CodeRabbit
Release Notes
New Features
SODARUserandSODARUserRequest.strListfor arrays of strings.Improvements
signalparameter.createQueryKeyfunction for client configuration.Bug Fixes
Dependencies
@hey-api/client-fetchto version^0.4.2.@hey-api/openapi-tsto version^0.54.0.