Repository navigation
chore: use TypeScript project references - #1056
Conversation
WalkthroughThe workspace migrates to shared TypeScript project configurations and direct project-build checks. Desktop React compiler integration now uses explicit React and Rolldown Babel plugins. Core AI tool schemas and return types are explicit. Vitest and module-resolution settings receive small updates. ChangesWorkspace project-check configuration
React compiler integration
Explicit AI tool contracts
Test and module adjustments
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
ocavue
changed the title
@ocavue/tsconfig with project references and tsc -b
9d8e144 to
9f8110c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/desktop/vitest.shared.ts (1)
6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the exported
defineDesktopProjectcontract.
defineDesktopProjectis exported and used byapps/desktop/vitest.browser.config.ts, but its plugin and test-configuration behavior is not documented. The parameter object is also defined inline. Add aDesktopProjectOptionsinterface and document that shared React/Babel plugins are prepended andproject.testoverrides shared defaults.As per coding guidelines, exported APIs must be documented and object definitions should prefer interfaces.
Proposed refactor
+interface DesktopProjectOptions { + readonly plugins?: ViteUserConfig['plugins'] + readonly test: NonNullable<ViteUserConfig['test']> +} + +/** + * Creates the shared desktop Vitest configuration. + * Project plugins are appended after the React Compiler plugins. + * Project test options override shared defaults. + */ -export function defineDesktopProject(project: { - plugins?: ViteUserConfig['plugins'] - test: NonNullable<ViteUserConfig['test']> -}): ViteUserConfig { +export function defineDesktopProject( + project: DesktopProjectOptions, +): ViteUserConfig {🤖 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 `@apps/desktop/vitest.shared.ts` around lines 6 - 9, Introduce an exported DesktopProjectOptions interface for the defineDesktopProject parameter, preserving its plugins and required test configuration types. Add API documentation for defineDesktopProject describing that shared React/Babel plugins are prepended and project.test overrides shared defaults, and update the function signature to use the interface.Source: Coding guidelines
packages/core/src/ai/chat/tools.ts (2)
269-275: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an interface for
NoteTools.
NoteToolsis a public object-shaped contract. Declare it asexport interface NoteToolsinstead of a type literal.Proposed change
-export type NoteTools = { +export interface NoteTools { ... -} +}As per coding guidelines, “Prefer interfaces for object definitions; use type aliases for unions, intersections, and mapped types.”
🤖 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 `@packages/core/src/ai/chat/tools.ts` around lines 269 - 275, Change the public object-shaped NoteTools contract from an exported type alias to an exported interface, preserving all existing tool properties and their types.Source: Coding guidelines
91-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the newly exported schemas.
searchNotesInput,listRecentNotesInput, andlistDailyNotesInputare now public APIs, but they have no JSDoc. Add a short comment before each export. Keep.describe()for runtime schema metadata.Proposed documentation
+/** Validates arguments for the `search_notes` tool. */ export const searchNotesInput = ... +/** Validates arguments for the `list_recent_notes` tool. */ export const listRecentNotesInput = ... +/** Validates arguments for the `list_daily_notes` tool. */ export const listDailyNotesInput = ...As per coding guidelines, “Always document public APIs.”
Also applies to: 102-116, 121-123
🤖 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 `@packages/core/src/ai/chat/tools.ts` around lines 91 - 99, Add concise JSDoc comments immediately before the exported schemas searchNotesInput, listRecentNotesInput, and listDailyNotesInput to document their public API purpose. Keep the existing Zod .describe() metadata unchanged.Source: Coding guidelines
🤖 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 `@apps/desktop/vite.config.ts`:
- Around line 68-74: Update the Vite server configuration around the
host-conditional options so server.port remains 1420 and the WebSocket settings
are nested under server.ws. Preserve the existing conditional behavior for host,
including protocol, host, and port 1421 values.
---
Nitpick comments:
In `@apps/desktop/vitest.shared.ts`:
- Around line 6-9: Introduce an exported DesktopProjectOptions interface for the
defineDesktopProject parameter, preserving its plugins and required test
configuration types. Add API documentation for defineDesktopProject describing
that shared React/Babel plugins are prepended and project.test overrides shared
defaults, and update the function signature to use the interface.
In `@packages/core/src/ai/chat/tools.ts`:
- Around line 269-275: Change the public object-shaped NoteTools contract from
an exported type alias to an exported interface, preserving all existing tool
properties and their types.
- Around line 91-99: Add concise JSDoc comments immediately before the exported
schemas searchNotesInput, listRecentNotesInput, and listDailyNotesInput to
document their public API purpose. Keep the existing Zod .describe() metadata
unchanged.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56b35f98-e8e8-4210-b5f9-0ae6261a1be9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
apps/desktop/package.jsonapps/desktop/react-compiler-plugin.tsapps/desktop/tsconfig.jsonapps/desktop/vite.config.tsapps/desktop/vitest.browser.config.tsapps/desktop/vitest.shared.tsapps/extension/package.jsonapps/extension/tsconfig.jsonapps/extension/wxt.config.tspackage.jsonpackages/core/package.jsonpackages/core/src/ai/chat/tools.tspackages/core/tsconfig.jsonpackages/core/vitest.browser.config.tspackages/core/vitest.node.config.tspackages/db/package.jsonpackages/db/tsconfig.jsonpackages/utils/package.jsonpackages/utils/tsconfig.jsontsconfig.base.jsontsconfig.jsonturbo.json
💤 Files with no reviewable changes (5)
- packages/core/vitest.node.config.ts
- turbo.json
- apps/desktop/react-compiler-plugin.ts
- packages/core/vitest.browser.config.ts
- tsconfig.base.json
| ...(host | ||
| ? { | ||
| protocol: 'ws', | ||
| host, | ||
| port: 1421, | ||
| } | ||
| : undefined, | ||
| : {}), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
config='apps/desktop/vite.config.ts'
rg -n 'server:|ws:|hmr:|protocol:|port: 1420|port: 1421' "$config"
if rg -q -P -U '\?\s*\{\s*\n\s*protocol:' "$config"; then
echo "WebSocket options are still flattened into server" >&2
exit 1
fi
rg -q -P -U 'ws:\s*\{\s*\n\s*protocol:' "$config"Repository: team-reflect/reflect-open
Length of output: 301
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- vite config ---'
cat -n apps/desktop/vite.config.ts
printf '%s\n' '--- package metadata containing Vite/Tauri ---'
rg -n -C 3 '"(vite|`@vitejs/plugin-`|`@tauri-apps/cli`|`@tauri-apps/api`)"' \
package.json apps/desktop/package.json pnpm-lock.yaml 2>/dev/null || true
printf '%s\n' '--- related host and port references ---'
rg -n -C 3 'TAURI_DEV_HOST|1420|1421|server\.hmr|server\.ws|hmr:|ws:' \
apps package.json 2>/dev/null || true
printf '%s\n' '--- tracked config files ---'
git ls-files '*vite*' '*tauri*' | head -80Repository: team-reflect/reflect-open
Length of output: 40099
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from collections import OrderedDict
base_server = OrderedDict([
("port", 1420),
("host", False),
])
host = "dev.example.test"
result = OrderedDict(base_server)
if host:
result.update({
"protocol": "ws",
"host": host,
"port": 1421,
})
print("flattened server:", dict(result))
print("HTTP server port:", result["port"])
print("top-level protocol:", result.get("protocol"))
print("nested ws:", result.get("ws"))
print("nested hmr:", result.get("hmr"))
nested_ws = OrderedDict(base_server)
if host:
nested_ws["ws"] = {
"protocol": "ws",
"host": host,
"port": 1421,
}
print("nested ws server:", dict(nested_ws))
print("nested ws HTTP port:", nested_ws["port"])
nested_hmr = OrderedDict(base_server)
if host:
nested_hmr["hmr"] = {
"protocol": "ws",
"host": host,
"port": 1421,
}
print("nested hmr server:", dict(nested_hmr))
print("nested hmr HTTP port:", nested_hmr["port"])
PYRepository: team-reflect/reflect-open
Length of output: 608
🌐 Web query:
Vite 8.2 server hmr protocol host port server.ws configuration options
💡 Result:
In Vite 8.2, the WebSocket configuration options that were previously located under server.hmr have been migrated to a new server.ws configuration object [1][2][3]. While the previous options under server.hmr (including protocol, host, port, path, clientPort, timeout, and server) remain supported for backward compatibility, they are now deprecated [1][4][3]. You should update your configuration to use the server.ws namespace [1][3]. The available configuration options under server.ws are [3]: - protocol: Sets the WebSocket protocol used for the HMR connection (e.g., 'ws' or 'wss'). - host: Specifies the WebSocket server host. - port: Specifies the WebSocket server port. - clientPort: An advanced option that overrides the port on the client side, allowing the WebSocket to be served on a different port than the one the client code expects [5][3]. - path: Sets the WebSocket path. - timeout: Defines the connection timeout in milliseconds (default: 30000). - server: Allows providing a custom HTTP server for WebSocket connections [5][3]. The server.hmr object still exists but is now primarily used for HMR-specific behavior, such as the overlay option (e.g., server.hmr.overlay: false) [1][3]. If you continue using the deprecated options, they will be automatically synced with server.ws, but you will receive deprecation warnings [1][6][3].
Citations:
- 1: https://vite.dev/config/server-options
- 2: feat: rename
server.hmroptions toserver.wsoptions vitejs/vite#21357 - 3: https://github.com/vitejs/vite/blob/main/docs/config/server-options.md
- 4: https://github.com/vitejs/vite/blob/main/packages/vite/src/node/server/hmr.ts
- 5: https://github.com/vitejs/vite/blob/v8.0.8/docs/config/server-options.md
- 6: test: avoid warnings vitejs/vite#22851
Nest the WebSocket options under server.ws.
When TAURI_DEV_HOST is set, the current spread changes the HTTP port from 1420 to 1421 and leaves protocol at an unsupported level. Preserve server.port: 1420 and move these options into server.ws.
🤖 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 `@apps/desktop/vite.config.ts` around lines 68 - 74, Update the Vite server
configuration around the host-conditional options so server.port remains 1420
and the WebSocket settings are nested under server.ws. Preserve the existing
conditional behavior for host, including protocol, host, and port 1421 values.
Replace the hand-rolled
tsconfig.base.jsonwith@ocavue/tsconfigbases and TypeScript project references, sopnpm typecheckis a single incrementaltsc -b(meowdown's setup).Summary by CodeRabbit