Merged
Conversation
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the frontend's type safety by enabling TypeScript's strict mode and enforcing stricter null and property access checks. The changes primarily involve explicit handling of potentially null or undefined values throughout the codebase, ensuring more robust and predictable code behavior without altering any existing application logic. This extensive refactoring addresses numerous type-related issues, making the codebase more maintainable and less prone to common programming errors. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Contributor
Author
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces significant improvements to type safety by enabling stricter TypeScript compiler options. The changes are extensive and mostly correct in handling potential null and undefined values. However, I've identified several issues, including a few critical bugs that could lead to incorrect calculations and runtime errors. Specifically, there's an operator precedence issue in a calorie calculation, a syntax error in a component, and a few places where API calls could be made with undefined parameters. I've also noted a regression in type safety in the central apiCall function. Please review the detailed comments for each issue.
Note: Security Review did not run due to the size of the PR.
SparkyFitnessFrontend/src/tests/components/MealManagement.test.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Code Review
This pull request enables strict null checks and other related TypeScript rules, which is a significant step towards improving code safety. The changes are extensive, touching many files to add null checks, optional chaining, and type assertions. While most changes are correct and improve null safety, I've identified several critical issues that need to be addressed. These include a bug in calorie calculation, incorrect error handling logic, potential for creating invalid API requests with undefined IDs, and a significant weakening of type safety in the generic apiCall function. Please review the detailed comments for each issue.
Note: Security Review is unavailable for this PR.
I am having trouble creating individual review comments. Click here to see my feedback.
SparkyFitnessFrontend/src/api/Chatbot/Chatbot_FoodHandler.ts (505)
There's a logical error here due to operator precedence. The expression quantity ?? 0 / (selectedOption.serving_size || 100) is evaluated as quantity ?? (0 / (selectedOption.serving_size || 100)), which simplifies to quantity ?? 0. This means the division by serving_size is completely lost, and the calculation will be incorrect. You should wrap quantity ?? 0 in parentheses to ensure the division happens correctly.
((quantity ?? 0) / (selectedOption.serving_size || 100))
SparkyFitnessFrontend/src/api/api.ts (19-23)
Changing apiCall to no longer be generic and return Promise<any> significantly weakens the type safety across the entire application, which contradicts the goal of this PR. This change means that callers of apiCall will lose all type information for the response data. Please revert this to use generics (async function apiCall<T>(...): Promise<T>) to maintain type safety. You can then handle the type assertions within the function where needed, for example by using [] as unknown as T for specific cases where you return an empty array.
export async function apiCall<T = any>(
endpoint: string,
options?: ApiCallOptions
): Promise<T> {SparkyFitnessFrontend/src/components/Onboarding/PersonalPlanHeader.tsx (101-103)
There appears to be a syntax error here with a misplaced closing parenthesis. This will cause a rendering issue.
{plan?.bmr &&
Math.round(convertEnergy(plan.bmr, 'kcal', localEnergyUnit))}
{getEnergyUnitString(localEnergyUnit)}
SparkyFitnessFrontend/src/api/Chatbot/Chatbot_MeasurementHandler.ts (191)
Initializing categorySearchError with an empty object {} is problematic. The subsequent check if (categorySearchError && ...) will always evaluate the first part as true, even when no error has been caught. This changes the control flow of your application. To fix this while adhering to strict null checks, you should initialize it with null and type it as ApiError | null, or use undefined and type it as ApiError | undefined.
let categorySearchError: ApiError | null = null;
SparkyFitnessFrontend/src/api/Foods/mealPlanTemplate.ts (32)
The templateData.id could be undefined because templateData is a Partial<MealPlanTemplate>. This would result in an API call to /meal-plan-templates/undefined, which is likely an error. You should add a check to ensure templateData.id exists before making the API call, or adjust the function signature to enforce it.
SparkyFitnessFrontend/src/api/Foods/meals.ts (34-35)
The mealId parameter is now optional, but if it's undefined, the API call will be made to /meals/undefined, which will likely result in an error. You should add a check to ensure mealId is not undefined before making the call, or handle this case appropriately (e.g., by disabling the query if mealId is not present).
export const getMealById = async (mealId: string): Promise<Meal> => {
return await apiCall(`/meals/${mealId}`, { method: 'GET' });
SparkyFitnessFrontend/src/pages/Reports/FastingReport.tsx (97-100)
The logic for calculating zoneData seems to have a bug. The condition && zones.Anabolic (and similar for other zones) will be false when the count is 0, preventing the count from ever being incremented. You should remove these checks to correctly count the fasts in each zone.
if (hrs < 12) zones.Anabolic += 1;
else if (hrs < 16) zones.Catabolic += 1;
else if (hrs < 20) zones.FatBurning += 1;
else zones.Ketosis += 1;
SparkyFitnessFrontend/src/tests/components/MealManagement.test.tsx (26)
There is a typo here. It should be defaultValue instead of deefaultValue. This will cause tests to fail or behave unexpectedly.
return defaultValueOrOpts['defaultValue'] as string;
Sim-sat
force-pushed
the
strict-null-checks
branch
from
e2f3b4e to
006ef2c
Compare
Sim-sat
force-pushed
the
strict-null-checks
branch
from
006ef2c to
c549305
Compare
CodeWithCJ
approved these changes
Mar 6, 2026
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
2 participants
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.
Description
Adds and fixes the following typescript rules
This will significantly increase runtime safety and preventing almost all runtime errors. This also enables use to use zod validation since it requires strict typescript.
There were about 200 errors each so the pr is quite large. The logic was not changed.
Related Issue
PR type [ ] Issue [ ] New Feature [ ] Documentation
Linked Issue: #
Checklist
Please check all that apply:
pnpm run validate(especially for Frontend).en) translation file (if applicable).rls_policies.sqlfor any new user-specific tables.