feat: add monorepo dev workflow with concurrently and fix TypeScript #51#323
feat: add monorepo dev workflow with concurrently and fix TypeScript #51#323vinisha1014 wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughThis pull request introduces typing improvements, refactors variable naming to mark unused/reserved code paths, adds optional component props for upcoming features, and establishes a monorepo structure with root-level package.json configuration and concurrent build/dev scripts. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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: 1
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 6-15: The root package.json’s current scripts leave frontend deps
uninstalled causing npm run dev (which invokes dev:frontend) to fail on a fresh
clone; update package.json to automatically install frontend deps after root
install by adding a postinstall script that runs the existing "install:all" (or
directly "npm run install:frontend"), so that "npm install" will bootstrap both
root and frontend, and ensure the change references the existing script names
"install:all", "install:frontend", "dev", and "dev:frontend".
🧹 Nitpick comments (7)
frontend/src/Pages/TeamDebateRoom.tsx (1)
741-742: Dead-code placeholder adds noise without value.
_currentPhaseis assigned and immediately voided, contributing nothing at runtime. If this is scaffolding for future phase-aware message handling, consider adding it when the logic is actually implemented. Leavingvoidno-ops in production code adds cognitive overhead for readers.If you want to keep the reminder, a short
// TODO:comment (without the variable) would be cleaner:Suggested cleanup
- const _currentPhase = debatePhaseRef.current; - void _currentPhase; // Available for phase-aware message handling + // TODO: use debatePhaseRef.current for phase-aware message handlingpackage.json (1)
9-9:dev:backendwill fail silently if Go is not installed.
go runwill exit with an error if Go isn't onPATH, andconcurrentlyby default won't stop the other process. This is acceptable for a dev script, but consider adding--kill-others-on-failto thedevscript so contributors get a clear signal when one side fails rather than seeing a half-running environment.frontend/src/Pages/OnlineDebateRoom.tsx (1)
2491-2497: The newliveTranscriptprop is not passed toSpeechTranscripts.The
SpeechTranscriptscomponent now accepts an optionalliveTranscriptprop (added in this PR), but it's not being supplied here. If the intent is to show live transcription to the user during the current phase, consider passing the current interim transcript. Since the prop is optional, this isn't a build error—just noting the gap.frontend/src/Pages/Game.tsx (1)
35-48: Consider removing unused code instead ofvoid-casting placeholders.Underscore-prefixing +
voidcasts work to suppress warnings, but they add noise. If these are reserved for a future feature, a brief TODO comment without the actual declarations would be cleaner. Alternatively, ifnoUnusedLocalsis the concern, the conventional approach is to prefix with_alone (TypeScript recognizes the_prefix convention whennoUnusedLocalsis enabled).The
voidstatements are unnecessary if the variables are already underscore-prefixed—TypeScript will not flag_-prefixed variables as unused.♻️ Simplified placeholder approach
- const _lastTypingStateRef = useRef<boolean>(false); - const _lastSpeakingStateRef = useRef<boolean>(false); - void _lastTypingStateRef; // Reserved for typing indicator feature - void _lastSpeakingStateRef; // Reserved for speaking indicator feature + // TODO: Reserved for typing/speaking indicator features + const _lastTypingStateRef = useRef<boolean>(false); + const _lastSpeakingStateRef = useRef<boolean>(false); - const _sendWebSocketMessage = useCallback((payload: Record<string, unknown>) => { + // TODO: Reserved for future WebSocket messaging + const _sendWebSocketMessage = useCallback((payload: Record<string, unknown>) => { const ws = websocketRef.current; if (ws?.readyState === WebSocket.OPEN) { ws.send(JSON.stringify(payload)); } else { console.warn("Attempted to send message while WebSocket was not open.", payload); } }, []); - void _sendWebSocketMessage; // Reserved for future WebSocket messagingfrontend/src/components/SpeechTranscripts.tsx (1)
82-86:animate-pulseon the text container may reduce readability.The pulsing animation applies to the entire text block including the transcript content, which can make it harder to read during active speech. Consider applying the animation only to the cursor indicator (
▋) instead.♻️ Pulse only the cursor, not the text
- <div className='text-sm text-gray-800 bg-white p-2 rounded border animate-pulse'> + <div className='text-sm text-gray-800 bg-white p-2 rounded border'> {liveTranscript} - <span className='text-orange-500 ml-1'>▋</span> + <span className='text-orange-500 ml-1 animate-pulse'>▋</span> </div>frontend/src/components/CommentTree.tsx (2)
231-233: Unused atom hook call still subscribes to state.
useAtom(addCommentToTranscriptAtom(transcriptId))still creates and subscribes to the atom even though the setter is voided. If it's truly not needed now, consider removing the hook call entirely to avoid the subscription overhead and the import.♻️ Remove unused atom hook
- const [, _addCommentAtom] = useAtom(addCommentToTranscriptAtom(transcriptId)); const [, removeCommentAtom] = useAtom(removeCommentFromTranscriptAtom(transcriptId)); - void _addCommentAtom; // Reserved for future useAnd remove
addCommentToTranscriptAtomfrom the import on line 9 if no other usage exists in this file.
394-396: Dead assignment — simplify by removing the variable.
_newCommentis assigned and immediately voided. SincefetchComments()on the next line handles the state update, just drop the variable.♻️ Remove dead variable
- const _newComment: Comment = result.comment; - void _newComment; // Result used via fetchComments refresh + // Result consumed via fetchComments refresh below
| "scripts": { | ||
| "dev": "concurrently -n \"FE,BE\" -c \"cyan,yellow\" \"npm run dev:frontend\" \"npm run dev:backend\"", | ||
| "dev:frontend": "cd frontend && npm run dev", | ||
| "dev:backend": "cd backend && go run ./cmd/server/", | ||
| "build": "concurrently \"npm run build:frontend\" \"npm run build:backend\"", | ||
| "build:frontend": "cd frontend && npm run build", | ||
| "build:backend": "cd backend && go build ./cmd/server/", | ||
| "install:frontend": "cd frontend && npm install", | ||
| "install:all": "npm install && npm run install:frontend" | ||
| }, |
There was a problem hiding this comment.
npm install alone won't bootstrap frontend — contributors hitting a broken npm run dev on fresh clone.
The PR states the usage is npm install; npm run dev, but npm install only installs root devDependencies (concurrently). Frontend dependencies in frontend/package.json are never installed, so dev:frontend will fail on a fresh clone.
Consider adding a postinstall hook to automate this, or at minimum update the documented workflow to use install:all:
Proposed fix
"scripts": {
"dev": "concurrently -n \"FE,BE\" -c \"cyan,yellow\" \"npm run dev:frontend\" \"npm run dev:backend\"",
"dev:frontend": "cd frontend && npm run dev",
"dev:backend": "cd backend && go run ./cmd/server/",
"build": "concurrently \"npm run build:frontend\" \"npm run build:backend\"",
"build:frontend": "cd frontend && npm run build",
"build:backend": "cd backend && go build ./cmd/server/",
"install:frontend": "cd frontend && npm install",
- "install:all": "npm install && npm run install:frontend"
+ "install:all": "npm install && npm run install:frontend",
+ "postinstall": "npm run install:frontend"
},📝 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.
| "scripts": { | |
| "dev": "concurrently -n \"FE,BE\" -c \"cyan,yellow\" \"npm run dev:frontend\" \"npm run dev:backend\"", | |
| "dev:frontend": "cd frontend && npm run dev", | |
| "dev:backend": "cd backend && go run ./cmd/server/", | |
| "build": "concurrently \"npm run build:frontend\" \"npm run build:backend\"", | |
| "build:frontend": "cd frontend && npm run build", | |
| "build:backend": "cd backend && go build ./cmd/server/", | |
| "install:frontend": "cd frontend && npm install", | |
| "install:all": "npm install && npm run install:frontend" | |
| }, | |
| "scripts": { | |
| "dev": "concurrently -n \"FE,BE\" -c \"cyan,yellow\" \"npm run dev:frontend\" \"npm run dev:backend\"", | |
| "dev:frontend": "cd frontend && npm run dev", | |
| "dev:backend": "cd backend && go run ./cmd/server/", | |
| "build": "concurrently \"npm run build:frontend\" \"npm run build:backend\"", | |
| "build:frontend": "cd frontend && npm run build", | |
| "build:backend": "cd backend && go build ./cmd/server/", | |
| "install:frontend": "cd frontend && npm install", | |
| "install:all": "npm install && npm run install:frontend", | |
| "postinstall": "npm run install:frontend" | |
| }, |
🤖 Prompt for AI Agents
In `@package.json` around lines 6 - 15, The root package.json’s current scripts
leave frontend deps uninstalled causing npm run dev (which invokes dev:frontend)
to fail on a fresh clone; update package.json to automatically install frontend
deps after root install by adding a postinstall script that runs the existing
"install:all" (or directly "npm run install:frontend"), so that "npm install"
will bootstrap both root and frontend, and ensure the change references the
existing script names "install:all", "install:frontend", "dev", and
"dev:frontend".
There was a problem hiding this comment.
Pull request overview
This PR adds a root-level monorepo developer workflow for running the frontend (Vite) and backend (Go) together, and resolves several existing TypeScript compilation issues across the frontend.
Changes:
- Added a root
package.jsonwithconcurrently-powered scripts to run/build FE+BE from one command. - Fixed TypeScript type errors and unused imports/variables across multiple frontend pages/components.
- Added support for displaying a live transcript in
SpeechTranscripts.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Introduces root dev/build/install scripts using concurrently for FE+BE workflow. |
| frontend/src/services/teamDebateService.ts | Fixes response typing for matchmaking pool API call. |
| frontend/src/components/SpeechTranscripts.tsx | Adds optional liveTranscript display for the current phase. |
| frontend/src/components/JudgementPopup.tsx | Fixes props destructuring to include botDesc. |
| frontend/src/components/Header.tsx | Removes unused lucide icon imports. |
| frontend/src/components/CommentTree.tsx | Removes an unused atom import and adjusts variables to satisfy TS unused checks. |
| frontend/src/Pages/TeamDebateRoom.tsx | Fixes null vs undefined ref assignment and addresses an unused phase variable. |
| frontend/src/Pages/OnlineDebateRoom.tsx | Adds an explicit type annotation in String.replace callback. |
| frontend/src/Pages/Game.tsx | Fixes points type and suppresses several unused refs/helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const result = await response.json(); | ||
| const newComment: Comment = result.comment; | ||
| const _newComment: Comment = result.comment; | ||
| void _newComment; // Result used via fetchComments refresh |
There was a problem hiding this comment.
_newComment is assigned from the POST response but never used (only void’d). If the comment isn’t needed because you refresh via fetchComments(), drop the unused assignment and just await the refresh; otherwise, use the returned comment to update state/atoms directly to avoid an extra network round-trip.
| const _currentPhase = debatePhaseRef.current; | ||
| void _currentPhase; // Available for phase-aware message handling |
There was a problem hiding this comment.
_currentPhase is introduced but unused (only referenced via void). If phase-aware handling isn’t implemented yet, it’s clearer to remove the unused variable until it’s needed, rather than keeping placeholder code in the message handler.
| const _currentPhase = debatePhaseRef.current; | |
| void _currentPhase; // Available for phase-aware message handling |
| const _lastTypingStateRef = useRef<boolean>(false); | ||
| const _lastSpeakingStateRef = useRef<boolean>(false); | ||
| void _lastTypingStateRef; // Reserved for typing indicator feature | ||
| void _lastSpeakingStateRef; // Reserved for speaking indicator feature |
There was a problem hiding this comment.
These refs are renamed and then intentionally unused (only referenced via void). This leaves dead code paths in a core page and makes it harder to understand what is actually used. Prefer removing them until the typing/speaking indicator feature is implemented, or wire them into the existing TYPING_* / SPEAKING_* handlers if that feature already exists in this component.
| const _sendWebSocketMessage = useCallback((payload: Record<string, unknown>) => { | ||
| const ws = websocketRef.current; | ||
| if (ws?.readyState === WebSocket.OPEN) { | ||
| ws.send(JSON.stringify(payload)); | ||
| } else { | ||
| console.warn("Attempted to send message while WebSocket was not open.", payload); | ||
| } | ||
| }, []); | ||
| void _sendWebSocketMessage; // Reserved for future WebSocket messaging |
There was a problem hiding this comment.
_sendWebSocketMessage is defined but unused (then void’d). If this helper isn’t currently needed, remove it to reduce noise; otherwise, use it in the message-sending call sites so it provides value and stays covered by compiler/lint checks.
| "build:frontend": "cd frontend && npm run build", | ||
| "build:backend": "cd backend && go build ./cmd/server/", | ||
| "install:frontend": "cd frontend && npm install", | ||
| "install:all": "npm install && npm run install:frontend" |
There was a problem hiding this comment.
npm install at the repo root will only install root devDependencies (e.g., concurrently) and will not install frontend/ dependencies. As written, the PR description’s “npm install && npm run dev” flow will fail unless users manually run npm install in frontend/ first. Consider adding a root postinstall (or using npm workspaces / npm --prefix frontend install) so a single root install is sufficient, or adjust scripts so dev ensures frontend deps are installed.
| "install:all": "npm install && npm run install:frontend" | |
| "install:all": "npm install && npm run install:frontend", | |
| "postinstall": "npm --prefix frontend install" |
| "private": true, | ||
| "description": "DebateAI Monorepo - Run frontend and backend together", | ||
| "scripts": { | ||
| "dev": "concurrently -n \"FE,BE\" -c \"cyan,yellow\" \"npm run dev:frontend\" \"npm run dev:backend\"", |
There was a problem hiding this comment.
The dev script starts two long-running processes, but it doesn’t configure concurrently to stop the other process when one exits/fails. This can mask backend crashes while the frontend keeps running (or vice versa). Consider enabling concurrently’s kill-others-on-fail behavior so the overall npm run dev clearly fails when either service fails.
| "dev": "concurrently -n \"FE,BE\" -c \"cyan,yellow\" \"npm run dev:frontend\" \"npm run dev:backend\"", | |
| "dev": "concurrently --kill-others-on-fail -n \"FE,BE\" -c \"cyan,yellow\" \"npm run dev:frontend\" \"npm run dev:backend\"", |
| const [commentsAtom] = useAtom(getCommentsForTranscriptAtom(transcriptId)); | ||
| const [, setCommentsAtom] = useAtom(setCommentsForTranscriptAtom(transcriptId)); | ||
| const [, addCommentAtom] = useAtom(addCommentToTranscriptAtom(transcriptId)); | ||
| const [, _addCommentAtom] = useAtom(addCommentToTranscriptAtom(transcriptId)); | ||
| const [, removeCommentAtom] = useAtom(removeCommentFromTranscriptAtom(transcriptId)); | ||
| void _addCommentAtom; // Reserved for future use |
There was a problem hiding this comment.
_addCommentAtom is initialized via useAtom(...) but intentionally unused (only referenced via void). This adds dead code and an extra atom subscription; if the add-comment atom isn’t needed here, remove the useAtom(addCommentToTranscriptAtom(...)) call (and any related import) until it’s actually used.
### Description
This PR improves the developer experience by introducing a monorepo development workflow that allows running both the frontend and backend simultaneously using a single command.
A root package.json has been added with scripts powered by concurrently to start both services together.
While implementing this, several pre-existing TypeScript build errors were resolved to ensure the project builds successfully.
### Changes
### TypeScript Fixes (Pre-existing errors)
These fixes were necessary to allow successful compilation:
Game.tsx — corrected points type (boolean → number)
OnlineDebateRoom.tsx — added explicit type to regex callback
TeamDebateRoom.tsx — fixed null vs undefined mismatch
CommentTree.tsx, Header.tsx, JudgementPopup.tsx — removed unused imports
SpeechTranscripts.tsx — added liveTranscript prop support
teamDebateService.ts — corrected type name typo
### Screenshots:
### How to Use
npm install
npm run dev
Starts:
Frontend (Vite)
Backend (Go server)
Impact
### Checklist
Summary by CodeRabbit
New Features
Chores