Conversation
Releases with empty asset arrays passed the filter because `[]` is truthy in JS. This caused incomplete records (missing content_type, download_url, etc.) to be written to the cache. - Check `assets?.length > 0` instead of just truthiness - Fix `assert_error` typo to `asset_error` - Filter out releases with asset errors after formatting - Remove debug `console.log(response.body)` - Add vitest and unit tests for filterReleases and formatRelease
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request integrates Vitest for unit testing, adding a new test suite for release utility functions and a test script to the package configuration. The release handling logic is refined to ensure assets are present, a typo in the asset error property is corrected, and releases with errors are filtered out. Feedback focuses on improving TypeScript type annotations to facilitate safer property access.
| ]; | ||
|
|
||
| function filterReleases(releases: Array<object>): Array<object> { | ||
| export function filterReleases(releases: Array<object>): Array<object> { |
There was a problem hiding this comment.
The type Array is overly restrictive in TypeScript and prevents accessing properties like assets or draft without type casting. Using any[] or Record<string, any>[] would be more appropriate here to allow for safe property access, or ideally, a dedicated interface for the GitHub release object.
| export function filterReleases(releases: Array<object>): Array<object> { | |
| export function filterReleases(releases: any[]): any[] { |
| } | ||
|
|
||
| function formatRelease(release) { | ||
| export function formatRelease(release) { |
There was a problem hiding this comment.
The release parameter is missing a type annotation. Adding a type annotation (e.g., any or a specific interface) improves type safety and maintainability in TypeScript files.
| export function formatRelease(release) { | |
| export function formatRelease(release: any) { |
1 participant
Summary
filterReleasesallowed through releases with empty or nullassetsarrays, leading to corrupt data being cachedassert_error→asset_errorinformatRelease, and added a final filter to strip any releases where asset extraction failed before cachingvitestas a dev dependency and a test suite forfilterReleasesandformatReleaseto prevent regressionsTest plan
npm testand verify all new tests insrc/utils/releases.test.tspass