feat: migrate frontend from JavaScript to TypeScript - #37
Merged
Merged
Conversation
Convert the entire React SPA from JSX/JS to TSX/TS with strict TypeScript configuration (strict: true, noUnusedLocals, noUnusedParameters). Includes new types.ts for shared interfaces, eslint.config.js for TS-aware linting, tsconfig.json with project references, and vite-env.d.ts. Updates CI (make check, GitHub Actions) to run tsc --noEmit and eslint . alongside existing tests. All component props, API return types, and state variables are now typed; runtime patterns like vi.mocked() and null-safe access via ??/?. are adopted to match the stricter compilation.
|
Warning Review limit reached
More reviews will be available in 34 minutes and 28 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR migrates the entire NutWatch frontend from untyped JavaScript/JSX to strict TypeScript/TSX. It adds ChangesFrontend TypeScript Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/frontend/src/components/HistoryChart.tsx (2)
84-98:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse an instance-scoped clipPath id instead of a hardcoded global id.
id="chart-clip"is document-global in SVG. MultipleHistoryChartinstances can collide and reference the wrong clip path.💡 Proposed fix
- const defs = document.createElementNS(ns, 'defs'); + const defs = document.createElementNS(ns, 'defs'); + const clipId = svg.dataset.clipId ?? `chart-clip-${Math.random().toString(36).slice(2)}`; + svg.dataset.clipId = clipId; const clip = document.createElementNS(ns, 'clipPath'); - clip.setAttribute('id', 'chart-clip'); + clip.setAttribute('id', clipId); @@ const chartArea = document.createElementNS(ns, 'g'); - chartArea.setAttribute('clip-path', 'url(`#chart-clip`)'); + chartArea.setAttribute('clip-path', `url(#${clipId})`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/src/components/HistoryChart.tsx` around lines 84 - 98, The clipPath id is hardcoded as 'chart-clip' which is document-global and causes collisions when multiple HistoryChart instances render. Generate a unique identifier per component instance (such as using React's useId hook) and replace the hardcoded id in both the clip.setAttribute('id', 'chart-clip') call and the corresponding chartArea.setAttribute('clip-path', 'url(`#chart-clip`)') reference so each instance uses its own unique clip path identifier.
291-330:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReset variable/data state on UPS switch to prevent stale history fetches.
When
upsNamechanges, Line 322 can fire with the previousavailableVarsbefore the new variables request resolves, causing a wrong/history/...request and transiently rendering stale series for the new UPS context.💡 Proposed fix
useEffect(() => { let cancelled = false; setLoading(true); + setAvailableVars([]); + setSelectedVars({}); + setData(null); api<{ variables?: string[] }>(API.historyVariables(upsName)) .then(res => { if (cancelled) return; const vars = (res.variables ?? []).filter(isDynamicVar); setAvailableVars(vars);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/src/components/HistoryChart.tsx` around lines 291 - 330, When upsName changes, the second useEffect can execute with stale availableVars from the previous UPS context before the first useEffect completes and updates availableVars, causing incorrect history requests. To fix this, reset availableVars and selectedVars to their initial empty/default state at the start of the first useEffect (the one that calls API.historyVariables) when upsName changes, before the new API request is made. This ensures the second useEffect either doesn't fire yet or uses the reset state while waiting for the new variables to arrive. Alternatively, consider clearing availableVars, selectedVars, and data immediately in a separate effect that depends only on upsName to synchronously reset state before any other effects process the change.src/frontend/src/components/UserModal.tsx (1)
28-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate username before submit.
trimmedNamecan be empty in add mode, so the POST can send an invalid user payload and surface only a generic failure message. Add an explicit non-empty username check before sending.Proposed patch
async function handleSave() { if (savePending.current) return; const trimmedName = name.trim(); + if (!trimmedName) { + await alert('Username is required', 'Validation Error'); + return; + } if (!isEdit && !password) { await alert('Password is required for new users', 'Validation Error'); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/src/components/UserModal.tsx` around lines 28 - 44, The code validates that a password is required for new users but does not validate that the username is not empty before submission. Add an explicit non-empty check for trimmedName in the validation section (after the password validation check) that uses the alert function to show a validation error message if the trimmedName is empty, similar to how the password validation is handled. This ensures that both in add mode and edit mode, the username is never empty when making the API call.src/frontend/src/components/Notifications.tsx (1)
19-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSeparate “loading” from “failed-to-load” state.
config === nullcurrently represents both in-flight and failed fetch, which can leave the page stuck on a loading message after request failure. Track load completion separately (or use a distinct error sentinel) so failure can render a recoverable/error state.Proposed patch
- const [config, setConfig] = useState<UpsmonConfig | null>(null); + const [config, setConfig] = useState<UpsmonConfig | null>(null); + const [configLoaded, setConfigLoaded] = useState(false); useEffect(() => { let cancelled = false; Promise.all([ api<UpsmonConfig>(API.UPSMON_CONFIG).catch(() => null), api<UpsDevice[]>(API.UPS).catch(() => [] as UpsDevice[]), ]).then(([cfg, ups]) => { if (cancelled) return; setConfig(cfg); setUpsNames(ups.map(u => u.name)); + setConfigLoaded(true); if (cfg) { ... } }); return () => { cancelled = true; }; }, []); - if (!config) { + if (!configLoaded) { return <div className="empty">Loading notifications configuration...</div>; } + if (!config) { + return <div className="empty">Notifications configuration unavailable.</div>; + }Also applies to: 190-192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/src/components/Notifications.tsx` around lines 19 - 45, The config state currently uses null to represent both the loading state (data in-flight) and the failed-to-load state (fetch error), which prevents proper error handling. Add a separate state variable to track whether the config has finished loading (success or failure) independently from its value. Update the Promise.all callback to set both the config value and the load completion state, ensuring the catch handler for the config API call sets the appropriate error or failure indicator. Then update the UI rendering logic (referenced at lines 190-192) to check the load completion state rather than just the config nullability, so it can distinguish between displaying a loading message, an error message for failed requests, or the actual config data when available.src/frontend/src/components/UpsDevices.tsx (1)
65-90:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the delete lock active until restart/refresh completes.
deletePendingis released right after the DELETE phase, but restart calls still run afterward. That allows a second click to trigger duplicate non-idempotent operations during the same logical delete flow.Proposed fix
async function handleDelete(name: string) { const key = 'ups:' + name; if (deletePending.current[key]) return; deletePending.current[key] = true; try { const ok = await dangerConfirm('Delete UPS "' + name + '"? This will stop the driver and remove all configuration.'); if (!ok) return; await api(API.ups(name), { method: 'DELETE' }); - } catch (e) { - await alert('Failed to delete UPS:\n' + (e as Error).message, 'Error'); - return; - } finally { - delete deletePending.current[key]; - } - try { - const list = await api<UpsDevice[]>(API.UPS); - const r = list.length === 0 - ? await api<CommandResult>(API.SERVICE_RESTART_MONITOR, { method: 'POST' }) - : await api<CommandResult>(API.SERVICE_RESTART_ALL, { method: 'POST' }); - if (r.returncode !== 0) { - await alert('Service restart warning:\n' + (r.stderr ?? r.stdout ?? ''), 'Restart Warning'); + try { + const list = await api<UpsDevice[]>(API.UPS); + const r = list.length === 0 + ? await api<CommandResult>(API.SERVICE_RESTART_MONITOR, { method: 'POST' }) + : await api<CommandResult>(API.SERVICE_RESTART_ALL, { method: 'POST' }); + if (r.returncode !== 0) { + await alert('Service restart warning:\n' + (r.stderr ?? r.stdout ?? ''), 'Restart Warning'); + } + } catch (e) { + await alert('Restart failed — changes may not be fully applied:\n' + (e as Error).message, 'Restart Error'); } - } catch (e) { - await alert('Restart failed — changes may not be fully applied:\n' + (e as Error).message, 'Restart Error'); + void loadUps(); + } catch (e) { + await alert('Failed to delete UPS:\n' + (e as Error).message, 'Error'); + return; + } finally { + delete deletePending.current[key]; } - void loadUps(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/src/components/UpsDevices.tsx` around lines 65 - 90, The delete lock in the handleDelete function is released too early. Currently, the `delete deletePending.current[key];` statement is in the finally block of the first try-catch (which handles the DELETE API call), allowing a second click to trigger duplicate restart operations that execute after the lock is released. Move the `delete deletePending.current[key];` statement from the inner finally block to a new outer finally block that wraps all subsequent operations including the restart API calls and the loadUps() invocation, ensuring the lock remains active throughout the entire delete flow until all async operations complete.src/frontend/src/main.tsx (1)
2-8:⚠️ Potential issue | 🔴 CriticalImport
createRootas a named export.
react-dom/clientdoes not expose a default export. The current import will fail at both compile time (TypeScript strict mode) and runtime. Use the named export instead.🔧 Proposed fix
import React from 'react'; -import ReactDOM from 'react-dom/client'; +import { createRoot } from 'react-dom/client'; import App from './App'; import './styles/variables.css'; import './styles/base.css'; import './styles/components.css'; -ReactDOM.createRoot(document.getElementById('root')!).render( +createRoot(document.getElementById('root')!).render( <React.StrictMode> <App /> </React.StrictMode> );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/src/main.tsx` around lines 2 - 8, The import statement in main.tsx is attempting to import ReactDOM as a default export from 'react-dom/client', but this module only provides named exports. Change the import statement to use named export syntax: import { createRoot } from 'react-dom/client', and then update the ReactDOM.createRoot call to simply use createRoot directly, removing the ReactDOM prefix from the method invocation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Line 23: The check target executes tsc-check and lint-frontend before frontend
dependencies are installed, causing failures on clean environments. Ensure that
a frontend dependency installation step (typically npm ci) runs before tsc-check
and lint-frontend by either adding it as a prerequisite target that these
commands depend on, or by reorganizing the dependency chain in the check target
to install dependencies first. This fix should also be applied to any other
affected targets mentioned in the related locations (lines 46-47) that have
similar dependency ordering issues.
In `@src/frontend/package.json`:
- Line 13: The tsc-check script in the tsc-check command uses the --noEmit flag
which does not traverse TypeScript project references, leaving files like
vite.config.ts and vitest.config.ts unchecked. Replace the --noEmit flag with
the --build flag (or -b shorthand) to enable TypeScript build mode, which will
properly traverse and type-check all referenced projects including those in
tsconfig.node.json.
In `@src/frontend/src/__tests__/components/ConfirmDialog.test.tsx`:
- Around line 47-48: The non-null assertion operator used with confirmBtn on
line 48 will produce unclear error messages if the button finder logic fails.
Replace the non-null assertion (the ! after confirmBtn) with an explicit guard
that checks if confirmBtn exists before calling user.click. This could be done
with an if statement or explicit assertion that provides a clear error message
indicating the button was not found with the specified criteria.
In `@src/frontend/src/__tests__/components/Modal.test.tsx`:
- Around line 51-52: The querySelector call for the modal-overlay selector is
being unsafely cast to Element without verifying it exists first. Before the
user.click(overlay) call, add an assertion or conditional check to ensure the
overlay element is not null. This prevents a runtime crash if the selector fails
to find the element and makes the error path clear if the modal-overlay is
missing from the DOM.
In `@src/frontend/src/components/UpsDetail.tsx`:
- Line 74: The decodeURIComponent call on the name parameter can throw a
URIError if name contains malformed percent-encoded sequences, causing the
component to crash before error UI can display. Wrap the decodeURIComponent(name
?? '') call in a try-catch block to handle the potential URIError exception.
When a URIError is caught, assign a safe default value to upsname (such as an
empty string or the raw name value) so the component can continue to render and
display appropriate error messaging to the user.
---
Outside diff comments:
In `@src/frontend/src/components/HistoryChart.tsx`:
- Around line 84-98: The clipPath id is hardcoded as 'chart-clip' which is
document-global and causes collisions when multiple HistoryChart instances
render. Generate a unique identifier per component instance (such as using
React's useId hook) and replace the hardcoded id in both the
clip.setAttribute('id', 'chart-clip') call and the corresponding
chartArea.setAttribute('clip-path', 'url(`#chart-clip`)') reference so each
instance uses its own unique clip path identifier.
- Around line 291-330: When upsName changes, the second useEffect can execute
with stale availableVars from the previous UPS context before the first
useEffect completes and updates availableVars, causing incorrect history
requests. To fix this, reset availableVars and selectedVars to their initial
empty/default state at the start of the first useEffect (the one that calls
API.historyVariables) when upsName changes, before the new API request is made.
This ensures the second useEffect either doesn't fire yet or uses the reset
state while waiting for the new variables to arrive. Alternatively, consider
clearing availableVars, selectedVars, and data immediately in a separate effect
that depends only on upsName to synchronously reset state before any other
effects process the change.
In `@src/frontend/src/components/Notifications.tsx`:
- Around line 19-45: The config state currently uses null to represent both the
loading state (data in-flight) and the failed-to-load state (fetch error), which
prevents proper error handling. Add a separate state variable to track whether
the config has finished loading (success or failure) independently from its
value. Update the Promise.all callback to set both the config value and the load
completion state, ensuring the catch handler for the config API call sets the
appropriate error or failure indicator. Then update the UI rendering logic
(referenced at lines 190-192) to check the load completion state rather than
just the config nullability, so it can distinguish between displaying a loading
message, an error message for failed requests, or the actual config data when
available.
In `@src/frontend/src/components/UpsDevices.tsx`:
- Around line 65-90: The delete lock in the handleDelete function is released
too early. Currently, the `delete deletePending.current[key];` statement is in
the finally block of the first try-catch (which handles the DELETE API call),
allowing a second click to trigger duplicate restart operations that execute
after the lock is released. Move the `delete deletePending.current[key];`
statement from the inner finally block to a new outer finally block that wraps
all subsequent operations including the restart API calls and the loadUps()
invocation, ensuring the lock remains active throughout the entire delete flow
until all async operations complete.
In `@src/frontend/src/components/UserModal.tsx`:
- Around line 28-44: The code validates that a password is required for new
users but does not validate that the username is not empty before submission.
Add an explicit non-empty check for trimmedName in the validation section (after
the password validation check) that uses the alert function to show a validation
error message if the trimmedName is empty, similar to how the password
validation is handled. This ensures that both in add mode and edit mode, the
username is never empty when making the API call.
In `@src/frontend/src/main.tsx`:
- Around line 2-8: The import statement in main.tsx is attempting to import
ReactDOM as a default export from 'react-dom/client', but this module only
provides named exports. Change the import statement to use named export syntax:
import { createRoot } from 'react-dom/client', and then update the
ReactDOM.createRoot call to simply use createRoot directly, removing the
ReactDOM prefix from the method invocation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 814cc777-5300-4ad9-aa8b-01715b823b8b
⛔ Files ignored due to path filters (1)
src/frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (62)
.github/workflows/lint.ymlMakefileREADME.mdsrc/frontend/eslint.config.jssrc/frontend/index.htmlsrc/frontend/package.jsonsrc/frontend/src/App.tsxsrc/frontend/src/__tests__/components/Badge.test.tsxsrc/frontend/src/__tests__/components/ConfirmDialog.test.tsxsrc/frontend/src/__tests__/components/Dashboard.test.tsxsrc/frontend/src/__tests__/components/ErrorBoundary.test.tsxsrc/frontend/src/__tests__/components/Gauge.test.tsxsrc/frontend/src/__tests__/components/HistoryChart.test.tsxsrc/frontend/src/__tests__/components/Modal.test.tsxsrc/frontend/src/__tests__/components/UpsCard.test.tsxsrc/frontend/src/__tests__/components/UpsDetail.test.tsxsrc/frontend/src/__tests__/components/theme.test.tsxsrc/frontend/src/__tests__/setup.tssrc/frontend/src/__tests__/utils/api.test.tssrc/frontend/src/__tests__/utils/directives.test.tssrc/frontend/src/__tests__/utils/format.test.tssrc/frontend/src/__tests__/utils/logs.test.tssrc/frontend/src/__tests__/utils/service.test.tssrc/frontend/src/api.tssrc/frontend/src/components/Badge.jsxsrc/frontend/src/components/Badge.tsxsrc/frontend/src/components/ConfigFiles.tsxsrc/frontend/src/components/ConfirmDialog.tsxsrc/frontend/src/components/Dashboard.tsxsrc/frontend/src/components/ErrorBoundary.tsxsrc/frontend/src/components/Gauge.tsxsrc/frontend/src/components/HistoryChart.tsxsrc/frontend/src/components/HookEditor.tsxsrc/frontend/src/components/HooksSection.tsxsrc/frontend/src/components/Logs.tsxsrc/frontend/src/components/Modal.tsxsrc/frontend/src/components/Notifications.tsxsrc/frontend/src/components/ServiceStatus.tsxsrc/frontend/src/components/Sidebar.tsxsrc/frontend/src/components/ThemeSettings.tsxsrc/frontend/src/components/UpsCard.tsxsrc/frontend/src/components/UpsDetail.tsxsrc/frontend/src/components/UpsDevices.tsxsrc/frontend/src/components/UpsModal.tsxsrc/frontend/src/components/UserModal.tsxsrc/frontend/src/components/Users.tsxsrc/frontend/src/components/WakeOnLan.tsxsrc/frontend/src/constants/index.tssrc/frontend/src/main.tsxsrc/frontend/src/theme.tsxsrc/frontend/src/types.tssrc/frontend/src/utils/directives.tssrc/frontend/src/utils/format.tssrc/frontend/src/utils/logs.tssrc/frontend/src/utils/metrics.tssrc/frontend/src/utils/service.jssrc/frontend/src/utils/service.tssrc/frontend/src/vite-env.d.tssrc/frontend/tsconfig.jsonsrc/frontend/tsconfig.node.jsonsrc/frontend/vite.config.tssrc/frontend/vitest.config.ts
💤 Files with no reviewable changes (2)
- src/frontend/src/components/Badge.jsx
- src/frontend/src/utils/service.js
- Switch tsc-check from `tsc --noEmit` to `tsc -b` (project references build mode) with `declaration: true` in tsconfig.node.json, generating .tsbuildinfo and .d.ts/.js build artifacts. - Add `npm ci` to lint-frontend and tsc-check Makefile targets to ensure dependencies are installed before running those checks. - Fix Notifications component: add separate `configLoaded` state to distinguish loading from a null config (failed to load), preventing an infinite "Loading..." spinner on error. - Fix UpsDetail: wrap `decodeURIComponent(name)` in try-catch to handle malformed URI-encoded UPS names without crashing. - Fix UpsDevices: move service-restart logic inside the delete try block so `loadUps()` is called even if the restart fails after a successful delete. - Fix UserModal: validate that the username is non-empty after trimming. - Fix HistoryChart: use React `useId()` for SVG clip-path IDs to avoid DOM ID conflicts when multiple charts are rendered. - Refactor Dashboard tests: replace sequential `.mockResolvedValueOnce()` chains with a single `mockImplementation` in `beforeEach`, simplifying test setup and maintenance.
Replace the non-null assertion operator (!) and type casts (as Element) with proper null guards that throw descriptive Error messages when expected DOM elements are missing. This makes test failures more informative and avoids brittle type-level assertions that silently mask missing elements at runtime. Affected files: - ConfirmDialog.test.tsx: confirm button lookup - Modal.test.tsx: modal overlay lookup
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/frontend/tsconfig.node.json (1)
1-12:⚠️ Potential issue | 🟠 MajorAdd TypeScript build artifacts to
.gitignore.The TypeScript build artifacts are currently in version control and should be gitignored to prevent merge conflicts, staleness, and repository bloat. The repository contains
vite.config.js,vitest.config.js,vite.config.d.ts,vitest.config.d.ts,tsconfig.node.tsbuildinfo, andtsconfig.tsbuildinfoinsrc/frontend/, but these compiled outputs are not excluded by.gitignore. Add rules to.gitignoreto exclude.js,.d.ts, and.tsbuildinfofiles from the frontend directory.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/frontend/tsconfig.node.json` around lines 1 - 12, TypeScript build artifacts in the src/frontend/ directory are currently tracked in version control and should be excluded. Update the repository's .gitignore file to add patterns that prevent `.js`, `.d.ts`, and `.tsbuildinfo` files from the src/frontend/ directory from being committed. This will exclude the compiled outputs like vite.config.js, vitest.config.js, vite.config.d.ts, vitest.config.d.ts, tsconfig.node.tsbuildinfo, and tsconfig.tsbuildinfo while allowing the source TypeScript files to remain tracked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Makefile`:
- Around line 43-47: Create a new frontend-install target that executes npm ci
in the src/frontend directory, then update the lint-frontend, tsc-check, and
test-frontend targets to depend on frontend-install as a prerequisite instead of
each running npm ci independently. This eliminates the redundant npm ci
invocations while maintaining the ability to invoke each target independently
since they will all depend on the shared frontend-install prerequisite.
---
Outside diff comments:
In `@src/frontend/tsconfig.node.json`:
- Around line 1-12: TypeScript build artifacts in the src/frontend/ directory
are currently tracked in version control and should be excluded. Update the
repository's .gitignore file to add patterns that prevent `.js`, `.d.ts`, and
`.tsbuildinfo` files from the src/frontend/ directory from being committed. This
will exclude the compiled outputs like vite.config.js, vitest.config.js,
vite.config.d.ts, vitest.config.d.ts, tsconfig.node.tsbuildinfo, and
tsconfig.tsbuildinfo while allowing the source TypeScript files to remain
tracked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 88ce6fd5-6ff1-4244-b4e4-78bb2ebf285b
📒 Files selected for processing (17)
Makefilesrc/frontend/package.jsonsrc/frontend/src/__tests__/components/ConfirmDialog.test.tsxsrc/frontend/src/__tests__/components/Dashboard.test.tsxsrc/frontend/src/__tests__/components/Modal.test.tsxsrc/frontend/src/components/HistoryChart.tsxsrc/frontend/src/components/Notifications.tsxsrc/frontend/src/components/UpsDetail.tsxsrc/frontend/src/components/UpsDevices.tsxsrc/frontend/src/components/UserModal.tsxsrc/frontend/tsconfig.node.jsonsrc/frontend/tsconfig.node.tsbuildinfosrc/frontend/tsconfig.tsbuildinfosrc/frontend/vite.config.d.tssrc/frontend/vite.config.jssrc/frontend/vitest.config.d.tssrc/frontend/vitest.config.js
Extract the repeated `npm ci` calls from test-frontend, lint-frontend, and tsc-check targets into a shared frontend-install dependency. This avoids running npm ci three separate times when running multiple frontend targets in sequence (e.g. make tsc-check && make test-frontend), relying on npm's own caching instead. Also gitignore six frontend build artifacts (vite/vitest config outputs and tsbuildinfo files) to keep the working tree clean.
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.
Convert the entire React SPA from JSX/JS to TSX/TS with strict TypeScript configuration (strict: true, noUnusedLocals, noUnusedParameters). Includes new types.ts for shared interfaces, eslint.config.js for TS-aware linting, tsconfig.json with project references, and vite-env.d.ts. Updates CI (make check, GitHub Actions) to run tsc --noEmit and eslint . alongside existing tests. All component props, API return types, and state variables are now typed; runtime patterns like vi.mocked() and null-safe access via ??/?. are adopted to match the stricter compilation.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Chores
Tests