-
Notifications
You must be signed in to change notification settings - Fork 4
Fix Docker build issues and improve chain-agnostic architecture #52
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
Conversation
👷 Deploy request for transaction-form-builder pending review.A Netlify team Owner will need to approve the deploy before you can run your build. Are you a team Owner? Visit the deploys page to approve it → Need more help? Learn more in the Netlify docs →
|
🚨 BugBot couldn't runSomething went wrong. Try again by commenting "bugbot run", or contact support (requestId: serverGenReqId_dc7b0435-a231-42ef-9f87-0dc56f5bac76). |
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 resolves Docker build issues when building from a clean repository clone and improves the chain-agnostic architecture by removing EVM-specific dependencies from shared packages.
- Docker build process now compiles all workspace packages in dependency order using
pnpm -r build
- EVM-specific
viem
imports removed from chain-agnostic types package - New
TypedEvmNetworkConfig
interface added to EVM adapter for proper type safety while maintaining base compatibility
Reviewed Changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
Dockerfile | Changed build command to compile all workspace packages in dependency order |
packages/types/src/networks/config.ts | Removed viem import and changed viemChain type to unknown for chain-agnostic compatibility |
packages/adapter-evm/src/types.ts | Added TypedEvmNetworkConfig interface with proper viem Chain typing |
packages/adapter-evm/src/adapter.ts | Updated to use TypedEvmNetworkConfig instead of EvmNetworkConfig |
packages/builder/src/core/ecosystemManager.ts | Updated import and type usage for TypedEvmNetworkConfig |
Multiple package.json files | Updated react-hook-form versions and added missing dependencies |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
packages/adapter-evm/src/networks/testnet.ts:193
- [nitpick] The variable name
zksyncSepoliaTestnet
is inconsistent with theexportConstName
property value'zksyncSepoliaTestnet'
but inconsistent with the previous naming pattern. The original namezkSyncEraSepolia
was more descriptive and consistent with other network names.
export const zksyncSepoliaTestnet: TypedEvmNetworkConfig = {
packages/adapter-evm/src/networks/testnet.ts:195
- [nitpick] The
exportConstName
should match the variable name for consistency. If the variable iszksyncSepoliaTestnet
, the export name should also bezksyncSepoliaTestnet
, but this creates inconsistency with other networks that use camelCase without the 'testnet' suffix.
exportConstName: 'zksyncSepoliaTestnet',
bugbot run |
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.
✅ BugBot reviewed your changes and found no bugs!
BugBot free trial expires on July 22, 2025
Learn more in the Cursor dashboard.
Was this report helpful? Give feedback by reacting with 👍 or 👎
Summary
Resolves Docker build failures when building from a clean repository clone and improves the separation between chain-agnostic and EVM-specific code.
Key Changes
pnpm -r build
)viem
imports from chain-agnostic@openzeppelin/contracts-ui-builder-types
packageTypedEvmNetworkConfig
interface in EVM adapter for properviem.Chain
typing while maintaining base type compatibilitylucide-react
andreact-hook-form
dependencies to packages that were importing them