Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the existing perps application into a monorepo structure with three main packages: a common package containing shared code, a widget package (the original application), and a new dashboard package. The refactoring introduces Turbo for build orchestration, TypeScript project references for better build performance, and a pnpm catalog for dependency management.
Changes:
- Converted single-app to monorepo with common, widget, and dashboard packages
- Migrated all shared code (atoms, components, services, domain logic) to @yieldxyz/perps-common
- Updated all imports from path aliases (@/*) to relative imports or package imports
- Added new dashboard application with trading interface and positions table
- Introduced Turbo for coordinated builds and development
Reviewed changes
Copilot reviewed 194 out of 221 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| turbo.json | Turbo configuration for monorepo task orchestration |
| tsconfig.json, tsconfig.base.json | TypeScript project references setup for monorepo |
| pnpm-workspace.yaml | Workspace configuration with centralized dependency catalog |
| packages/common/* | Shared code extracted into reusable package with proper exports |
| packages/widget/* | Original app refactored to use common package |
| packages/dashboard/* | New dashboard application for trading interface |
| package.json | Root package.json updated for monorepo scripts |
Comments suppressed due to low confidence (3)
packages/common/src/services/config.ts:34
- The
ConfigServiceeffect has removed the.orDieat the end. This means configuration errors will now be propagated as errors rather than causing the program to die. Ensure that all consumers ofConfigServiceproperly handle configuration errors, as this is a behavior change.
packages/common/src/atoms/actions-atoms.ts:12 - The
actionAtomtype signature changed fromAtom.writable<ActionDto | null, ActionDto>toAtom.writable<ActionDto | null, ActionDto | null>. This allows setting the atom tonull, but verify that all usages ofsetAction(null)are intentional and that consumers can handle null values properly.
packages/common/src/hooks/use-position-actions.ts:8 - The removal of the
leveragefield from theUpdateLeverageSchemaargs seems incorrect. This schema is used for updating leverage on a position, but without the leverage value in the args, how would the update know what leverage to set? This appears to be a bug unless the leverage is being passed through a different mechanism.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const formatSnakeCase = (network: string) => { | ||
| let str = network[0]?.toUpperCase() ?? ""; | ||
| for (let i = 1; i < network.length; i++) { | ||
| str += | ||
| network[i] === "_" ? ` ${network[++i]?.toUpperCase() ?? ""}` : network[i]; | ||
| } | ||
| return str; | ||
| }; |
There was a problem hiding this comment.
The formatSnakeCase function has a potential issue with accessing array indices that may be undefined. When incrementing i with ++i, the subsequent access network[++i] could go out of bounds. Consider adding bounds checking or using optional chaining.
6b03315 to
212acc0
Compare
212acc0 to
1285465
Compare
1285465 to
6e0ce87
Compare
No description provided.