npm run build fix - #2
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideFixes TypeScript build errors by tightening type assertions around workflow instance responses, removing unused helper functions, and extending metadata typing for advisor absences. Class diagram for updated workflow instance typing and advisor metadataclassDiagram
class ApiData_T_ {
<<interface>>
+Record_string_unknown__index
}
class VnextInstance {
<<interface>>
+string _source
+string key
+string id
+Record_string_unknown_ attributes
+VnextMetadata metadata
}
class VnextMetadata {
<<interface>>
+string currentState
+string createdAt
+string updatedAt
+string status
}
class AdvisorInst {
<<type>>
+string key
+Record_string_unknown_ attributes
}
class Dashboard {
<<component>>
+extractReservations_T_(res)
}
class ChatManagement {
<<component>>
+loadAdvisorInstances()
}
class Absence {
<<component>>
+uses_VnextInstance
}
ApiData_T_ --> VnextInstance : data
VnextInstance *-- VnextMetadata : metadata
ChatManagement --> AdvisorInst : uses
Absence --> VnextInstance : uses
Dashboard --> ApiData_T_ : consumes
Dashboard --> VnextInstance : reads_metadata
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The double cast in
Dashboard(a as unknown as Record<string, unknown>) suggests the type forreservationitems is too loose; consider refining the underlying type or adding a type guard so you don't need to bypass the type system this way. - In
ChatManagement, the nearly identical blocks buildingpmItemsandiaItemscould be DRYed up via a small helper that takes a response and returnsAdvisorInst[], reducing duplication and the chance of inconsistent behavior between the two.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The double cast in `Dashboard` (`a as unknown as Record<string, unknown>`) suggests the type for `reservation` items is too loose; consider refining the underlying type or adding a type guard so you don't need to bypass the type system this way.
- In `ChatManagement`, the nearly identical blocks building `pmItems` and `iaItems` could be DRYed up via a small helper that takes a response and returns `AdvisorInst[]`, reducing duplication and the chance of inconsistent behavior between the two.
## Individual Comments
### Comment 1
<location path="src/pages/admin/Dashboard.tsx" line_range="238-239" />
<code_context>
if (isNaN(endMs)) return false;
if (startMs > now || endMs < now) return false;
- const st = (a.metadata?.currentState ?? (a as Record<string, unknown>).currentState) as string | undefined;
+ const st = (a.metadata?.currentState ??
+ (a as unknown as Record<string, unknown>).currentState) as string | undefined;
const validStates = ['active', 'approved', 'complete', 'complete-with-transfer'];
return !st || validStates.includes(st);
</code_context>
<issue_to_address>
**suggestion:** The double cast through `unknown` could hide real type issues and might be better expressed via a type guard or more specific typing.
This double cast makes `a` effectively `any`, weakening type safety and hiding regressions. Instead, either narrow the declared type of `a` so it includes `currentState` (e.g. `{ metadata?: { currentState?: string }; currentState?: string }`) or add a type guard to safely access `currentState` without bypassing the type system.
Suggested implementation:
```typescript
const { metadata, currentState } = a as {
metadata?: { currentState?: string };
currentState?: string;
};
const st = (metadata?.currentState ?? currentState) as string | undefined;
```
If possible in your codebase, consider tightening the type of `a` (or the generic `T`) where it is declared so it structurally includes `metadata?: { currentState?: string }` and `currentState?: string`. That would allow you to avoid the cast entirely and have the compiler enforce the presence and shape of these fields.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const st = (a.metadata?.currentState ?? | ||
| (a as unknown as Record<string, unknown>).currentState) as string | undefined; |
There was a problem hiding this comment.
suggestion: The double cast through unknown could hide real type issues and might be better expressed via a type guard or more specific typing.
This double cast makes a effectively any, weakening type safety and hiding regressions. Instead, either narrow the declared type of a so it includes currentState (e.g. { metadata?: { currentState?: string }; currentState?: string }) or add a type guard to safely access currentState without bypassing the type system.
Suggested implementation:
const { metadata, currentState } = a as {
metadata?: { currentState?: string };
currentState?: string;
};
const st = (metadata?.currentState ?? currentState) as string | undefined;If possible in your codebase, consider tightening the type of a (or the generic T) where it is declared so it structurally includes metadata?: { currentState?: string } and currentState?: string. That would allow you to avoid the cast entirely and have the compiler enforce the presence and shape of these fields.
![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 cleans up unused functions, updates the VnextInstance interface, and refactors logic in ChatManagement.tsx to ensure empty arrays are returned instead of booleans. Feedback suggests improving the type casting in Dashboard.tsx by refining the VnextInstance interface or using type guards to avoid verbose double casting.
| const st = (a.metadata?.currentState ?? | ||
| (a as unknown as Record<string, unknown>).currentState) as string | undefined; |
There was a problem hiding this comment.
The type casting (a as unknown as Record<string, unknown>) is a bit verbose. While it fixes the TypeScript error, consider if a could be more accurately typed in the interface VnextInstance or if a type guard could be used to avoid this double casting.
2 participants
Summary by Sourcery
Adjust type assertions and response handling to fix TypeScript build issues in admin and advisor pages.
Bug Fixes: