Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughMigrates linting to a flat ESLint config, updates dependency manifests (Next/React bumps, server deps moved), replaces several fetch calls with typed axios usage, tightens TypeScript includes, and applies multiple client-side typing, promise-handling, and state-parsing refinements across components. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/client/package.json (1)
55-79:⚠️ Potential issue | 🟠 MajorResolutions override the newer React type versions and violate peer dependencies.
The
resolutionsblock pins@types/reactto 19.0.7 and@types/react-domto 19.0.3. However,@types/react-dom@19.2.3declares a peer dependency on@types/react@^19.2.0, so forcing 19.0.7 violates that constraint. This can result in missing or incorrect type definitions.Update resolutions to match the devDependencies versions:
Suggested fix
"resolutions": { - "@types/react": "19.0.7", - "@types/react-dom": "19.0.3" + "@types/react": "19.2.10", + "@types/react-dom": "19.2.3" }
🤖 Fix all issues with AI agents
In `@packages/client/package.json`:
- Around line 55-64: Update the dependencies in package.json to align with
Next.js 16.1.6 by changing the "eslint-config-next" entry to "16.1.6" and
bumping "eslint" to a version that satisfies its peer dependency (upgrade
"eslint" from "8.57.0" to a compatible v9 release, e.g., ">=9.0.0" or a specific
9.x like "9.8.0"); ensure the package.json handles peer requirements and run
npm/yarn install and lint to verify no peer-compatibility errors for
"eslint-config-next" and "eslint".
In `@packages/client/src/components/tabs/RaceTab.tsx`:
- Around line 124-134: fetchDriverNames() returns the data array directly, so
the current code incorrectly checks response.data and always sets an empty list;
change the promise handler to treat the resolved value as the drivers array
(e.g., rename response → drivers), check if the array is falsy/empty, then map
over drivers (each of type IDriverData) to build { Rfid, driver } and call
setDriverData(mappedArray); update the handler attached to fetchDriverNames()
accordingly and keep error handling (or add a .catch) if needed.
In `@packages/server/package.json`:
- Around line 53-56: The package.json start script currently includes the
runtime hook "ts-node/register" which is provided by the devDependency "ts-node"
and will be missing in production; update the "start" script to remove
"ts-node/register" and keep only "tsconfig-paths/register" so production uses
the compiled dist/server.js (i.e., ensure start invokes node -r
tsconfig-paths/register dist/server.js instead of loading ts-node/register), and
verify "ts-node" remains a devDependency.
justin-phxm
left a comment
justin-phxm
left a comment
There was a problem hiding this comment.
good if you address coderabbit comments
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| @@ -1,4 +1,4 @@ | |||
| import axios from "axios"; | |||
| import axios, { AxiosResponse } from "axios"; | |||
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/client/src/components/molecules/LogoStatusMolecules/Settings/DriverNameUpdate.tsx (1)
76-106:⚠️ Potential issue | 🟡 Minor
setLoading(false)is never called whenvalidateInputs()fails.If
validateInputs()returnsfalse(Line 81), execution falls through to the end ofhandleSubmitwithout resettingloading. The spinner will stay visible indefinitely. This appears to be a pre-existing bug, but it's adjacent to the changed code and easy to fix.Proposed fix
} + } else { + setLoading(false); } };packages/client/src/components/molecules/LogoStatusMolecules/PlaybackDatePicker.tsx (1)
180-188:⚠️ Potential issue | 🟠 MajorStale closure:
fetchPlaybackDatacaptures the oldplaybackDateTime.On Line 185,
setPlaybackDateTimeenqueues a state update, butfetchPlaybackData(called immediately on Line 186) still closes over the previousplaybackDateTimevalue. The data fetched may not correspond to the date just restored from local storage.Consider restructuring so the fetch runs after the state is committed (e.g., in a separate
useEffectthat depends onplaybackDateTime), or pass the date values as arguments tofetchPlaybackData.
🤖 Fix all issues with AI agents
In @.prettierrc.cjs:
- Line 15: Update the tailwindConfig path in .prettierrc.cjs: change the
tailwindConfig value (currently "tailwind.config.ts") to point to the actual
config at ./packages/client/tailwind.config.ts so Prettier's Tailwind plugin can
resolve the file and re-enable class sorting; locate the tailwindConfig property
in the existing .prettierrc.cjs and replace the string with the correct relative
path.
In `@eslint.config.mjs`:
- Around line 79-103: The react-hooks plugin is registered but its rules aren't
enabled; update the rules block for the files entry that registers reactHooks
(the object containing plugins: { ..., "react-hooks": reactHooks }) to either
spread reactHooks.configs.recommended.rules into rules or explicitly add
"react-hooks/rules-of-hooks": "error" and "react-hooks/exhaustive-deps": "warn"
so the Hook lints are enforced for the rules-of-hooks and exhaustive-deps
checks.
In `@package.json`:
- Around line 33-36: The package.json has a peer dependency mismatch:
"@eslint/js@10.0.1" requires "eslint@^10.0.0" but the project pins "eslint" to
"9.0.0"; fix by either upgrading "eslint" to a ^10.x version (ensure Node >=
20.19.0) and run tests/CI to validate, or downgrade "@eslint/js" to a 9.x
compatible version (e.g., ^9.0.0) so it aligns with "eslint" 9.x; update the
dependency entry for the chosen package ("eslint" or "@eslint/js") in
package.json and run install to verify no peer dependency warnings remain.
In `@packages/client/src/components/containers/MLContainer.tsx`:
- Around line 30-32: The code calls JSON.parse on axios' response.data which is
already parsed JSON, causing a SyntaxError; in MLContainer.tsx change the axios
call to return parsed type directly (use axios.get<PlotParams>(plotType) and
assign const data = response.data) and remove JSON.parse; if the API truly
returns a JSON-encoded string instead, call axios.get<string>(plotType, {
responseType: 'text' }) and then JSON.parse(response.data) — update the types
accordingly so data is a PlotParams.
In
`@packages/client/src/components/molecules/LogoStatusMolecules/PlaybackDatePicker.tsx`:
- Around line 170-173: The catch block inside fetchPlaybackData currently
re-throws the error which leads to unhandled promise rejections because callers
use void fetchPlaybackData(); instead, remove the throw in the catch of
fetchPlaybackData and handle the error locally: log it, set a local error state
(e.g., setPlaybackError or use existing state), and clear any loading flags so
the UI can show an error/notification; update fetchPlaybackData's catch to call
that state setter or notification helper and return gracefully so callers
(including DatePickerColumn and the void fetchPlaybackData() call) no longer
receive a rejected promise.
In `@packages/client/src/components/tabs/RaceTab.tsx`:
- Around line 90-93: The code mixes await with .then inside the async function
handleDropdown; change the fetch call to use await consistently by assigning
const response = await fetchFilteredLaps(Number(newRFID)), then compute const
formattedData = response.map(formatLapData) and call
setFilteredLaps(formattedData); keep references to fetchFilteredLaps,
formatLapData, and setFilteredLaps so the logic stays the same but uses await
instead of .then chaining.
In `@packages/client/src/pages/api/getPacketCorrelationMatrix.ts`:
- Around line 13-15: The axios call currently uses axios.get<string>(API_ROUTE)
which misleadingly types response.data as string while Axios parses JSON at
runtime; update the call in getPacketCorrelationMatrix.ts to use a proper
response type (e.g., axios.get<PlotParams>(API_ROUTE) or
axios.get<unknown>(API_ROUTE)) so that the local variable graph is correctly
typed, or explicitly pass { responseType: 'text' } to axios.get if you truly
need a raw string; ensure all uses of response and graph reflect the corrected
type.
🧹 Nitpick comments (6)
packages/client/src/components/molecules/LogoStatusMolecules/Settings/DriverNameUpdate.tsx (1)
1-1: UnusedAxiosResponseimport.
AxiosResponseis imported but never referenced in this file. This was likely left over from an intermediate refactor.-import axios, { AxiosResponse } from "axios"; +import axios from "axios";eslint.config.mjs (2)
71-72: Duplicatesort-keysrules — built-in and plugin may conflict.Line 71 enables the built-in ESLint
sort-keysrule (which reports but cannot auto-fix), while Line 72 enablessort-keys/sort-keys-fixfrom the plugin (which auto-fixes). Having both active can produce duplicate diagnostics for the same violation.Consider disabling the built-in rule and relying solely on the plugin's fixable variant:
Proposed fix
"sort-destructure-keys/sort-destructure-keys": "error", - "sort-keys": "error", + "sort-keys": "off", "sort-keys/sort-keys-fix": "error",
4-11:eslint-plugin-importis declared inpackage.jsonbut not used in this config.The root
package.jsonlistseslint-plugin-importas a devDependency, but it's neither imported nor configured here. Either integrate it or remove the unused dependency.packages/client/src/components/tabs/RaceTab.tsx (2)
100-108: Inconsistent error return type —fetchDriverNamesreturns{ error: string }on failure, notIDriverData[].The success path returns
IDriverData[]but the catch block returns{ error: "Error fetching drivers" }. While the caller at Line 129 guards withArray.isArray(response), a cleaner approach is to return a consistent type (e.g., return[]on error, matchingfetchFilteredLaps's pattern on Line 117), or throw and let the caller'scatchhandle it.Proposed fix — return empty array on error (consistent with fetchFilteredLaps)
const fetchDriverNames = async () => { try { const response = await axios.get<IDriverData[]>(`${prodURL}/drivers`); - return response.data; } catch (error) { - return { error: "Error fetching drivers" }; + return []; } };
64-69:handleCopyis not memoized and has no error handling for the Clipboard API.
Unlike
handleDropdownandonCopied,handleCopyisn't wrapped inuseCallbackbut is passed as a prop to<DriverFilter>, which can cause unnecessary re-renders. (With the React Compiler this may be auto-optimized, but explicit memoization is still good practice for clarity.)
navigator.clipboard.writeTextcan reject (e.g., non-HTTPS, permission denied). Consider wrapping in try/catch.packages/client/src/components/global/AppStateEffectsManager.tsx (1)
7-8: Fragile type extraction — consider exportingAppStatefrom the store directly.The conditional type
AppStateStore extends { currentAppState: infer T } ? T : neversilently resolves toneverif the store shape changes (e.g.,currentAppStateis renamed). Exporting the type from the store module is more robust and maintainable.For example, in
useAppState.ts:export type AppState = { /* ... existing shape ... */ };Then here:
-type AppStateStore = ReturnType<typeof useAppState>; -type AppState = AppStateStore extends { currentAppState: infer T } ? T : never; +import { type AppState } from "@/stores/useAppState";
| require.resolve("prettier-plugin-tailwindcss"), | ||
| ], | ||
| tailwindConfig: "./packages/client/tailwind.config.ts", | ||
| tailwindConfig: "tailwind.config.ts", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Searching for tailwind.config.ts ==="
fd -t f 'tailwind\.config\.ts'
echo ""
echo "=== Checking file structure around packages/client ==="
ls -la packages/client/ 2>/dev/null | grep -i tailwind || echo "No tailwind files in packages/client/"
echo ""
echo "=== Checking repo root ==="
ls -la tailwind.config.ts 2>/dev/null || echo "No tailwind.config.ts at repo root"Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 337
Fix the tailwindConfig path — it points to a non-existent file.
The config file is at packages/client/tailwind.config.ts, not the repo root. The current path "tailwind.config.ts" will cause Prettier's Tailwind plugin to silently fail to resolve the config, disabling class sorting. Revert to "./packages/client/tailwind.config.ts".
🤖 Prompt for AI Agents
In @.prettierrc.cjs at line 15, Update the tailwindConfig path in
.prettierrc.cjs: change the tailwindConfig value (currently
"tailwind.config.ts") to point to the actual config at
./packages/client/tailwind.config.ts so Prettier's Tailwind plugin can resolve
the file and re-enable class sorting; locate the tailwindConfig property in the
existing .prettierrc.cjs and replace the string with the correct relative path.
| { | ||
| files: ["packages/client/**/*.{ts,tsx}"], | ||
| plugins: { | ||
| react: reactPlugin, | ||
| "@next/next": nextPlugin, | ||
| "react-hooks": reactHooks, | ||
| }, | ||
| settings: { | ||
| react: { | ||
| version: "detect", | ||
| }, | ||
| next: { | ||
| rootDir: "packages/client", | ||
| }, | ||
| }, | ||
| rules: { | ||
| ...reactPlugin.configs.recommended.rules, | ||
| ...nextPlugin.configs["core-web-vitals"].rules, | ||
|
|
||
| "react/jsx-sort-props": "error", | ||
| "react/sort-comp": "off", | ||
| "react/react-in-jsx-scope": "off", | ||
| "@next/next/no-html-link-for-pages": ["error", "packages/client/src/pages"], | ||
| }, | ||
| }, |
There was a problem hiding this comment.
React Hooks rules are not enforced — plugin is registered but no rules are enabled.
The react-hooks plugin is added on Line 84, but its recommended rules (rules-of-hooks and exhaustive-deps) are never spread or individually listed. Without them, hooks usage won't be linted at all.
Proposed fix
rules: {
...reactPlugin.configs.recommended.rules,
...nextPlugin.configs["core-web-vitals"].rules,
+ ...reactHooks.configs.recommended.rules,
"react/jsx-sort-props": "error",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| files: ["packages/client/**/*.{ts,tsx}"], | |
| plugins: { | |
| react: reactPlugin, | |
| "@next/next": nextPlugin, | |
| "react-hooks": reactHooks, | |
| }, | |
| settings: { | |
| react: { | |
| version: "detect", | |
| }, | |
| next: { | |
| rootDir: "packages/client", | |
| }, | |
| }, | |
| rules: { | |
| ...reactPlugin.configs.recommended.rules, | |
| ...nextPlugin.configs["core-web-vitals"].rules, | |
| "react/jsx-sort-props": "error", | |
| "react/sort-comp": "off", | |
| "react/react-in-jsx-scope": "off", | |
| "@next/next/no-html-link-for-pages": ["error", "packages/client/src/pages"], | |
| }, | |
| }, | |
| { | |
| files: ["packages/client/**/*.{ts,tsx}"], | |
| plugins: { | |
| react: reactPlugin, | |
| "@next/next": nextPlugin, | |
| "react-hooks": reactHooks, | |
| }, | |
| settings: { | |
| react: { | |
| version: "detect", | |
| }, | |
| next: { | |
| rootDir: "packages/client", | |
| }, | |
| }, | |
| rules: { | |
| ...reactPlugin.configs.recommended.rules, | |
| ...nextPlugin.configs["core-web-vitals"].rules, | |
| ...reactHooks.configs.recommended.rules, | |
| "react/jsx-sort-props": "error", | |
| "react/sort-comp": "off", | |
| "react/react-in-jsx-scope": "off", | |
| "@next/next/no-html-link-for-pages": ["error", "packages/client/src/pages"], | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In `@eslint.config.mjs` around lines 79 - 103, The react-hooks plugin is
registered but its rules aren't enabled; update the rules block for the files
entry that registers reactHooks (the object containing plugins: { ...,
"react-hooks": reactHooks }) to either spread
reactHooks.configs.recommended.rules into rules or explicitly add
"react-hooks/rules-of-hooks": "error" and "react-hooks/exhaustive-deps": "warn"
so the Hook lints are enforced for the rules-of-hooks and exhaustive-deps
checks.
| "@eslint/js": "^10.0.1", | ||
| "@next/eslint-plugin-next": "^16.1.6", | ||
| "@types/eslint": "^9.6.0", | ||
| "@typescript-eslint/eslint-plugin": "^7.4.0", | ||
| "@typescript-eslint/parser": "^7.4.0", | ||
| "clsx": "^2.1.1", | ||
| "eslint": "8.57.0", | ||
| "eslint-config-prettier": "^9.1.0", | ||
| "eslint-config-turbo": "latest", | ||
| "eslint": "9.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@eslint/js version 10 compatibility with eslint version 9
💡 Result:
@eslint/js v10 is not intended to be used with ESLint v9.
@eslint/js@10.0.1declares a (optional) peer dependency oneslint: ^10.0.0—so with ESLint 9 you’ll get a peer-deps mismatch (and you’re outside supported compatibility). [1]- If you’re on ESLint v9, use
@eslint/js@9.xinstead; upgrade to ESLint v10 only if you can meet its requirements (notably Node >= 20.19.0). [2]
Sources: [1] [2]
Fix version mismatch: @eslint/js@10.0.1 requires eslint@^10.0.0, not 9.0.0.
@eslint/js@10.0.1 declares a peer dependency on eslint@^10.0.0, creating an incompatibility with the pinned eslint: "9.0.0". This configuration is outside the supported compatibility range and will result in a peer dependency mismatch.
Resolve by either:
- Upgrade
eslintto^10.0.0(requires Node >= 20.19.0), or - Downgrade
@eslint/jsto^9.0.0to align with ESLint v9
🤖 Prompt for AI Agents
In `@package.json` around lines 33 - 36, The package.json has a peer dependency
mismatch: "@eslint/js@10.0.1" requires "eslint@^10.0.0" but the project pins
"eslint" to "9.0.0"; fix by either upgrading "eslint" to a ^10.x version (ensure
Node >= 20.19.0) and run tests/CI to validate, or downgrade "@eslint/js" to a
9.x compatible version (e.g., ^9.0.0) so it aligns with "eslint" 9.x; update the
dependency entry for the chosen package ("eslint" or "@eslint/js") in
package.json and run install to verify no peer dependency warnings remain.
| // TODO: check to see if data is being parsed correctly. | ||
| const response = await axios.get<string>(plotType); | ||
| const data = JSON.parse(response.data) as PlotParams; |
There was a problem hiding this comment.
JSON.parse(response.data) will throw at runtime — axios already parses JSON.
Since axios defaults to responseType: 'json', response.data is already a parsed object. Calling JSON.parse() on an object coerces it to "[object Object]" first, then throws SyntaxError. This means the plot will never load — the catch block will always fire, showing "Error loading data".
Fix by removing JSON.parse and typing the generic properly:
Proposed fix
- const response = await axios.get<string>(plotType);
- const data = JSON.parse(response.data) as PlotParams;
+ const response = await axios.get<PlotParams>(plotType);
+ const data = response.data;Alternatively, if the upstream API truly returns a JSON-encoded string (double-serialized), add { responseType: 'text' } to the axios config to prevent auto-parsing.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // TODO: check to see if data is being parsed correctly. | |
| const response = await axios.get<string>(plotType); | |
| const data = JSON.parse(response.data) as PlotParams; | |
| // TODO: check to see if data is being parsed correctly. | |
| const response = await axios.get<PlotParams>(plotType); | |
| const data = response.data; |
🤖 Prompt for AI Agents
In `@packages/client/src/components/containers/MLContainer.tsx` around lines 30 -
32, The code calls JSON.parse on axios' response.data which is already parsed
JSON, causing a SyntaxError; in MLContainer.tsx change the axios call to return
parsed type directly (use axios.get<PlotParams>(plotType) and assign const data
= response.data) and remove JSON.parse; if the API truly returns a JSON-encoded
string instead, call axios.get<string>(plotType, { responseType: 'text' }) and
then JSON.parse(response.data) — update the types accordingly so data is a
PlotParams.
| } catch (error) { | ||
| throw new Error(`Error fetching playback data: ${error}`); | ||
| throw new Error( | ||
| `Error fetching playback data: ${error instanceof Error ? error.message : String(error)}`, | ||
| ); |
There was a problem hiding this comment.
Re-throwing in fetchPlaybackData causes unhandled promise rejections.
fetchPlaybackData is called via void fetchPlaybackData() (Line 186) and likely via a similar fire-and-forget pattern in DatePickerColumn. The void operator discards the promise, so the re-thrown error on Line 171 becomes an unhandled rejection — no caller ever .catch()es it.
Either handle the error locally (e.g., show a notification or set error state) instead of re-throwing, or ensure every call site catches the rejection.
Proposed fix — handle error locally
} catch (error) {
- throw new Error(
- `Error fetching playback data: ${error instanceof Error ? error.message : String(error)}`,
- );
+ notifications.show({
+ color: "red",
+ message: `Error fetching playback data: ${error instanceof Error ? error.message : String(error)}`,
+ title: "Error",
+ });
} finally {🤖 Prompt for AI Agents
In
`@packages/client/src/components/molecules/LogoStatusMolecules/PlaybackDatePicker.tsx`
around lines 170 - 173, The catch block inside fetchPlaybackData currently
re-throws the error which leads to unhandled promise rejections because callers
use void fetchPlaybackData(); instead, remove the throw in the catch of
fetchPlaybackData and handle the error locally: log it, set a local error state
(e.g., setPlaybackError or use existing state), and clear any loading flags so
the UI can show an error/notification; update fetchPlaybackData's catch to call
that state setter or notification helper and return gracefully so callers
(including DatePickerColumn and the void fetchPlaybackData() call) no longer
receive a rejected promise.
| await fetchFilteredLaps(Number(newRFID)).then((response) => { | ||
| const formattedData = response.data.map(formatLapData); | ||
| const formattedData = response.map(formatLapData); | ||
| setFilteredLaps(formattedData); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Anti-pattern: mixing await with .then() in an async function.
handleDropdown is async (Line 82) and uses await on Line 90, but then chains .then() on the same call. This is confusing and unnecessary — use await consistently.
Proposed fix
} else {
- await fetchFilteredLaps(Number(newRFID)).then((response) => {
- const formattedData = response.map(formatLapData);
- setFilteredLaps(formattedData);
- });
+ const response = await fetchFilteredLaps(Number(newRFID));
+ const formattedData = response.map(formatLapData);
+ setFilteredLaps(formattedData);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await fetchFilteredLaps(Number(newRFID)).then((response) => { | |
| const formattedData = response.data.map(formatLapData); | |
| const formattedData = response.map(formatLapData); | |
| setFilteredLaps(formattedData); | |
| }); | |
| const response = await fetchFilteredLaps(Number(newRFID)); | |
| const formattedData = response.map(formatLapData); | |
| setFilteredLaps(formattedData); |
🤖 Prompt for AI Agents
In `@packages/client/src/components/tabs/RaceTab.tsx` around lines 90 - 93, The
code mixes await with .then inside the async function handleDropdown; change the
fetch call to use await consistently by assigning const response = await
fetchFilteredLaps(Number(newRFID)), then compute const formattedData =
response.map(formatLapData) and call setFilteredLaps(formattedData); keep
references to fetchFilteredLaps, formatLapData, and setFilteredLaps so the logic
stays the same but uses await instead of .then chaining.
| // TODO: check to see if this is parsing correctly (json vs string) | ||
| const response = await axios.get<string>(API_ROUTE as string); | ||
| const graph = response.data; |
There was a problem hiding this comment.
axios.get<string> is misleading — axios auto-parses JSON by default.
Axios's default responseType is 'json', so response.data will already be a parsed object, not a raw string. The <string> generic only affects the TypeScript type — it doesn't change runtime behavior. This means the TODO concern is valid: graph is typed as string but is likely an object at runtime.
Either:
- Set
{ responseType: 'text' }if a raw string is truly needed downstream, or - Type the generic to match the actual response shape (e.g.,
axios.get<PlotParams>(...)oraxios.get<unknown>(...)).
Since res.status(200).json(graph) serializes whatever graph is back to JSON for the client, option 2 is likely correct here.
🤖 Prompt for AI Agents
In `@packages/client/src/pages/api/getPacketCorrelationMatrix.ts` around lines 13
- 15, The axios call currently uses axios.get<string>(API_ROUTE) which
misleadingly types response.data as string while Axios parses JSON at runtime;
update the call in getPacketCorrelationMatrix.ts to use a proper response type
(e.g., axios.get<PlotParams>(API_ROUTE) or axios.get<unknown>(API_ROUTE)) so
that the local variable graph is correctly typed, or explicitly pass {
responseType: 'text' } to axios.get if you truly need a raw string; ensure all
uses of response and graph reflect the corrected type.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/server/package.json (1)
16-16:⚠️ Potential issue | 🟠 Major
@faker-js/fakershould be a devDependency or removed from this package.
@faker-js/fakeris not used anywhere in the server package code. The library is only used inpackages/sharedandpackages/client, which should declare it in their own dependencies. Keeping it here unnecessarily bloats the production server install.Suggested move
"dependencies": { ... - "@faker-js/faker": "^9.0.3", ... }, "devDependencies": { + "@faker-js/faker": "^9.0.3", ... }
🤖 Fix all issues with AI agents
In `@eslint.config.mjs`:
- Around line 71-72: The config currently enables both ESLint's built-in rule
"sort-keys" and the plugin rule "sort-keys/sort-keys-fix", causing duplicate
diagnostics; remove the built-in rule entry "sort-keys": "error" and keep only
"sort-keys/sort-keys-fix": "error" in the rules object (ensure the plugin
providing sort-keys-fix is present in the plugins list/installed). This ensures
only the auto-fixable plugin rule runs.
In `@packages/client/src/components/tabs/RaceTab.tsx`:
- Around line 64-68: Wrap the navigator.clipboard.writeText call inside a
try/catch in the handleCopy function (referencing handleCopy, Rfid, setCopy, and
onCopied) so clipboard failures don't cause unhandled rejections; on success run
setCopy(1), onCopied(), and clear after timeout, and on failure catch the error
(console.error or processLogger), set a distinct error state (e.g., setCopy(-1)
or similar) so the UI can show feedback, clear that state after a timeout, and
optionally attempt a legacy fallback copy method if you want broader browser
support.
In `@packages/client/src/pages/api/getLapCorrelationMatrix.ts`:
- Around line 16-20: The status check "if (!response.status || response.status
!== 200)" in the getLapCorrelationMatrix handler is dead code because Axios
throws for non-2xx by default; update the request handling to either pass a
validateStatus option to the Axios call used in getLapCorrelationMatrix so the
response is returned for non-200 statuses, or remove the status check and handle
all error differentiation inside the existing catch block (inspect
error.response?.status and error.message there). Locate the Axios call and the
response variable in the getLapCorrelationMatrix function and apply one of these
fixes so error paths are reachable and correctly logged/responded.
In `@packages/client/src/pages/api/getPacketCorrelationMatrix.ts`:
- Around line 16-20: The if-check that inspects response.status inside
getPacketCorrelationMatrix is dead code because axios rejects non-2xx responses
and response.status is always a number (truthy); remove the entire conditional
branch that returns res.status(response.status).json({ error: ... }) and instead
rely on the existing try/catch error handling (or, if you need to handle non-2xx
responses inline, reconfigure the axios call with validateStatus to allow those
through and then handle them using response.status). Locate the axios call
(response variable) and the getPacketCorrelationMatrix handler to implement the
change and update any tests expecting that branch.
🧹 Nitpick comments (3)
packages/client/src/components/tabs/RaceTab.tsx (2)
100-108: Mixed return type (IDriverData[] | { error: string }) is fragile.
fetchDriverNamesreturnsresponse.data(anIDriverData[]) on success but{ error: string }on failure. This untyped union forces the caller to useArray.isArrayguards. Since the caller already has a try/catch (Lines 124-140), just let the error propagate:Suggested fix
const fetchDriverNames = async () => { - try { - const response = await axios.get<IDriverData[]>(`${prodURL}/drivers`); - - return response.data; - } catch (error) { - return { error: "Error fetching drivers" }; - } + const response = await axios.get<IDriverData[]>(`${prodURL}/drivers`); + return response.data; };Then in the useEffect you can simplify the guard:
try { const response = await fetchDriverNames(); - - if (!Array.isArray(response)) { - setDriverData([]); - return; - } - - const driverData = response.map((driver: IDriverData) => ({ + const driverData = response.map((driver) => ({ Rfid: driver.Rfid, driver: driver.driver, }));
12-12: Use barrel export for consistency.Line 12 imports from
@shared/helios-types/src/typeswhile line 11 already imports from the barrel export@shared/helios-types. SinceIDriverDataandILapDataare re-exported from the barrel, consolidate to avoid unnecessary coupling to the internal file structure.-import { IDriverData, ILapData } from "@shared/helios-types/src/types"; +import { IDriverData, ILapData } from "@shared/helios-types";eslint.config.mjs (1)
108-114: Server config block is an empty placeholder — consider removing or adding rules.An empty
rulesobject adds config noise without effect. If there are no server-specific overrides yet, you can remove this block entirely and add it back when needed. If you plan to add rules soon, a TODO comment would clarify intent.
| "sort-keys": "error", | ||
| "sort-keys/sort-keys-fix": "error", |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicate sort-keys enforcement — built-in rule + plugin rule both active.
Line 71 enables ESLint's built-in sort-keys rule (report-only, not auto-fixable), and Line 72 enables the plugin's sort-keys/sort-keys-fix (auto-fixable). Both will flag the same unsorted keys, producing duplicate diagnostics. Drop the built-in rule and keep only the fixable plugin version.
Proposed fix
"sort-destructure-keys/sort-destructure-keys": "error",
- "sort-keys": "error",
"sort-keys/sort-keys-fix": "error",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "sort-keys": "error", | |
| "sort-keys/sort-keys-fix": "error", | |
| "sort-destructure-keys/sort-destructure-keys": "error", | |
| "sort-keys/sort-keys-fix": "error", |
🤖 Prompt for AI Agents
In `@eslint.config.mjs` around lines 71 - 72, The config currently enables both
ESLint's built-in rule "sort-keys" and the plugin rule
"sort-keys/sort-keys-fix", causing duplicate diagnostics; remove the built-in
rule entry "sort-keys": "error" and keep only "sort-keys/sort-keys-fix": "error"
in the rules object (ensure the plugin providing sort-keys-fix is present in the
plugins list/installed). This ensures only the auto-fixable plugin rule runs.
| const handleCopy = async () => { | ||
| await navigator.clipboard.writeText(String(Rfid)); | ||
| setCopy(1); | ||
| setTimeout(() => setCopy(0), 5000); | ||
| onCopied(); |
There was a problem hiding this comment.
Missing error handling for clipboard API.
navigator.clipboard.writeText can reject (e.g., permissions denied, non-secure context). Without a try/catch, the rejection propagates as an unhandled promise rejection, and setCopy(1) / onCopied() never run — giving the user no feedback.
Suggested fix
const handleCopy = async () => {
- await navigator.clipboard.writeText(String(Rfid));
- setCopy(1);
- setTimeout(() => setCopy(0), 5000);
- onCopied();
+ try {
+ await navigator.clipboard.writeText(String(Rfid));
+ setCopy(1);
+ setTimeout(() => setCopy(0), 5000);
+ onCopied();
+ } catch {
+ notifications.show({
+ color: "red",
+ message: "Failed to copy to clipboard",
+ title: "Error",
+ });
+ }
};🤖 Prompt for AI Agents
In `@packages/client/src/components/tabs/RaceTab.tsx` around lines 64 - 68, Wrap
the navigator.clipboard.writeText call inside a try/catch in the handleCopy
function (referencing handleCopy, Rfid, setCopy, and onCopied) so clipboard
failures don't cause unhandled rejections; on success run setCopy(1),
onCopied(), and clear after timeout, and on failure catch the error
(console.error or processLogger), set a distinct error state (e.g., setCopy(-1)
or similar) so the UI can show feedback, clear that state after a timeout, and
optionally attempt a legacy fallback copy method if you want broader browser
support.
| if (!response.status || response.status !== 200) { | ||
| return res.status(response.status).json({ | ||
| error: "Failed to fetch correlation matrix data", | ||
| }); | ||
| } |
There was a problem hiding this comment.
Same dead-code status check as getPacketCorrelationMatrix.ts.
Axios throws on non-2xx by default, making this branch unreachable. The same fix applies here — either use validateStatus or move error differentiation into the catch block. See the detailed suggestion on getPacketCorrelationMatrix.ts.
🤖 Prompt for AI Agents
In `@packages/client/src/pages/api/getLapCorrelationMatrix.ts` around lines 16 -
20, The status check "if (!response.status || response.status !== 200)" in the
getLapCorrelationMatrix handler is dead code because Axios throws for non-2xx by
default; update the request handling to either pass a validateStatus option to
the Axios call used in getLapCorrelationMatrix so the response is returned for
non-200 statuses, or remove the status check and handle all error
differentiation inside the existing catch block (inspect error.response?.status
and error.message there). Locate the Axios call and the response variable in the
getLapCorrelationMatrix function and apply one of these fixes so error paths are
reachable and correctly logged/responded.
| if (!response.status || response.status !== 200) { | ||
| return res.status(response.status).json({ | ||
| error: "Failed to fetch correlation matrix data", | ||
| }); | ||
| } |
There was a problem hiding this comment.
Status check is unreachable — axios throws on non-2xx responses by default.
Axios rejects the promise for any response with a status outside the 2xx range, so control will jump to the catch block instead of reaching this if. This entire branch is dead code. Additionally, !response.status is always false since status is a number (e.g., 200 is truthy).
If you want to handle specific non-2xx statuses gracefully, configure validateStatus:
Option: use validateStatus to allow non-2xx through
- const response = await axios.get<string>(BACKEND_ROUTE);
+ const response = await axios.get<string>(BACKEND_ROUTE, {
+ validateStatus: () => true,
+ });Otherwise, remove the dead branch and rely on the catch block for error handling:
Option: remove dead code
try {
- // TODO: check to see if this is parsing correctly (json vs string)
- const response = await axios.get<string>(BACKEND_ROUTE);
-
- if (!response.status || response.status !== 200) {
- return res.status(response.status).json({
- error: "Failed to fetch correlation matrix data",
- });
- }
-
- const graph = response.data;
+ const response = await axios.get(BACKEND_ROUTE);
+ const graph = response.data;
res.status(200).json(graph);
} catch (err) {
+ const status = axios.isAxiosError(err) ? (err.response?.status ?? 500) : 500;
+ const message = axios.isAxiosError(err)
+ ? "Failed to fetch correlation matrix data"
+ : `Failed to load data: ${err instanceof Error ? err.message : String(err)}`;
- res.status(500).json({
- error: `Failed to load data: ${err instanceof Error ? err.message : String(err)}`,
+ res.status(status).json({
+ error: message,
});
}🤖 Prompt for AI Agents
In `@packages/client/src/pages/api/getPacketCorrelationMatrix.ts` around lines 16
- 20, The if-check that inspects response.status inside
getPacketCorrelationMatrix is dead code because axios rejects non-2xx responses
and response.status is always a number (truthy); remove the entire conditional
branch that returns res.status(response.status).json({ error: ... }) and instead
rely on the existing try/catch error handling (or, if you need to handle non-2xx
responses inline, reconfigure the axios call with validateStatus to allow those
through and then handle them using response.status). Locate the axios call
(response variable) and the getPacketCorrelationMatrix handler to implement the
change and update any tests expecting that branch.
2 participants
Summary by CodeRabbit
New Features
Bug Fixes
Chores