-
Notifications
You must be signed in to change notification settings - Fork 0
Fix TypeScript build errors, CSS scoping issues, and CI workflow failures #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: aurelianware <194855645+aurelianware@users.noreply.github.com>
Co-authored-by: aurelianware <194855645+aurelianware@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes 35 TypeScript build errors that were blocking CI, primarily caused by styled-jsx syntax incompatibilities with Vite and various type mismatches.
Changes:
- Converted
<style jsx>to standard<style>tags in React components (removes scoped styling) - Added missing TypeScript type definitions for server-side dependencies (
@types/express,@types/ws,@types/uuid) - Fixed type issues in services (ImageData constructor, Promise types, stream callbacks, unused parameters)
- Updated Stripe API version and commented out incomplete FlightOrchestrator functionality
- Removed genuinely unused code (dead methods, unused variables/imports)
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/VideoStreamHandler.ts | Fixed WriteStream callback signature with proper error handling and Promise typing |
| src/services/VideoPrivacyPipeline.ts | Fixed ImageData constructor to use Uint8ClampedArray, prefixed unused parameter |
| src/services/ThermalAnalyzer.ts | Prefixed unused parameter with underscore |
| src/services/FlightOrchestrator.ts | Removed unused imports, methods (awaitMissionCompletion, delay), and variables (currentMission, PreFlightCheckResult) |
| src/services/EventDispatcher.ts | Removed unused threatThresholds object |
| src/services/AutelDroneSDK.ts | Prefixed unused parameters with underscores |
| src/routes/webhookRoutes.ts | Commented out FlightOrchestrator usage with TODO notes, added type guards, prefixed unused request parameters |
| src/mvp/server.ts | Added const assertion to waypoint action for type narrowing |
| src/lib/stripe.ts | Updated Stripe API version from 2025-11-17.clover to 2025-12-15.clover |
| src/components/MissionDashboard.tsx | Converted styled-jsx to standard style tag, prefixed unused missionId parameter |
| src/components/LaunchApprovalCard.tsx | Converted styled-jsx to standard style tag |
| package.json | Added @types/express, @types/ws, @types/uuid dev dependencies |
| package-lock.json | Lockfile updates for new type definition packages |
| const { eventId } = req.params; | ||
|
|
||
| // Ensure eventId is a string (route params can be string | string[]) | ||
| const eventIdStr = Array.isArray(eventId) ? eventId[0] : eventId; |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type guard for eventId is unnecessary. Express route parameters (from req.params) are always of type string, not string | string[]. The array type comes from query parameters (req.query), not route parameters.
This code can be simplified to:
const { eventId } = req.params;
const event = eventDispatcher.getEvent(eventId);
const droneResponse = eventDispatcher.getDroneResponse(eventId);However, this change doesn't cause any issues and the code functions correctly as-is.
| <p className="hint" aria-label="Offline status">{offline ? 'Offline: request will be queued' : 'Online approval'}</p> | ||
|
|
||
| <style jsx>{` | ||
| <style>{` |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing from <style jsx> to <style> removes CSS scoping and makes these styles global. This could cause naming conflicts with other components if they use the same class names (e.g., .header, .pill, .actions, button, select, etc.).
Consider using one of these alternatives:
- CSS Modules with
.module.cssfiles for automatic scoping - Tailwind classes inline (already used elsewhere in the project)
- A CSS-in-JS library compatible with Vite (e.g., styled-components, emotion)
If keeping regular <style> tags, namespace the class names to avoid conflicts (e.g., .launch-approval-card-header instead of .header).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 4df6a16. All CSS class names in LaunchApprovalCard.tsx are now namespaced with launch-approval- prefix to prevent global scope conflicts (e.g., .launch-approval-card, .launch-approval-header, .launch-approval-pill, etc.).
| </div> | ||
|
|
||
| <style jsx>{` | ||
| <style>{` |
Copilot
AI
Feb 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing from <style jsx> to <style> removes CSS scoping and makes these styles global. This could cause naming conflicts with other components if they use the same class names (e.g., .card, .grid, .header, button, etc.).
Consider using one of these alternatives:
- CSS Modules with
.module.cssfiles for automatic scoping - Tailwind classes inline (already used elsewhere in the project)
- A CSS-in-JS library compatible with Vite (e.g., styled-components, emotion)
If keeping regular <style> tags, namespace the class names to avoid conflicts (e.g., .mission-dashboard-card instead of .card).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 4df6a16. All CSS class names in MissionDashboard.tsx are now namespaced with mission-dashboard- prefix to prevent global scope conflicts (e.g., .mission-dashboard-container, .mission-dashboard-card, .mission-dashboard-grid, etc.).
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: aurelianware <194855645+aurelianware@users.noreply.github.com>
Co-authored-by: aurelianware <194855645+aurelianware@users.noreply.github.com>
Applied all changes from the review comments in commits 4df6a16 and 8de0210:
|
|
@copilot csn you fix the failing checks? |
Co-authored-by: aurelianware <194855645+aurelianware@users.noreply.github.com>
Fixed in commit 1c136ab. Updated Node.js version from 18 to 20 in all security workflow jobs (.github/workflows/security.yml) to fix the Prisma dependency error that requires Node.js 20.19+, 22.12+, or 24.0+. The Security Scanning workflow should now pass. The Azure ACR push failure is due to missing Azure credentials (AZURE_CLIENT_ID, AZURE_TENANT_ID) which are environment secrets and need to be configured in the repository settings - this cannot be fixed in code. |
Summary
TypeScript build was failing with 35 errors, blocking CI. Primary issue:
<style jsx>syntax used without styled-jsx configuration in LaunchApprovalCard.tsx. Additionally addressed CSS scoping concerns by namespacing all class names to prevent global conflicts. Fixed CI workflow failures caused by Node.js version incompatibility with Prisma dependencies.What changed
<style jsx>to standard<style>tags in LaunchApprovalCard.tsx (line 145) and MissionDashboard.tsx (line 223)launch-approval-prefix to all classes (e.g.,.launch-approval-card,.launch-approval-header,.launch-approval-pill)mission-dashboard-prefix to all classes (e.g.,.mission-dashboard-container,.mission-dashboard-card,.mission-dashboard-grid)@types/express,@types/ws,@types/uuidfor server-side code2025-11-17.clover→2025-12-15.cloveras constin mvp/server.tsnoUnusedLocals/noUnusedParametersgenerateMission/executeMissiondon't exist).github/workflows/security.ymlfor all security jobs (CodeQL Security Analysis, Dependency Vulnerability Scan, TypeScript Security Checks) to fix Prisma dependency compatibility (requires Node.js 20.19+, 22.12+, or 24.0+)Testing
npm run buildcompletes successfullySecurity/Compliance
Screenshots
N/A - Build fix, CSS scoping improvements, and CI workflow updates
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.