-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Move out dead adapters #5747
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: master
Are you sure you want to change the base?
Move out dead adapters #5747
Conversation
📝 WalkthroughWalkthroughThis change introduces a system for managing "dead" adapters—modules marked with a deadFrom marker. A new migration CLI script scans adapter modules, identifies dead adapters, and relocates them to a dead/ directory with a registry. Integration points in the factory registry and module builder incorporate dead adapters into the runtime module map during adapter lookup and module generation. Changes
Sequence DiagramssequenceDiagram
participant CLI as Migration CLI
participant FS as File System
participant Registry as deadAdapters.json
participant ModuleMap as Module Builder
CLI->>FS: Scan adapter modules for deadFrom markers
CLI->>CLI: Identify dead adapters & resolve dependencies
CLI->>CLI: Sort by dependency order
CLI->>FS: Create dead/ directories
CLI->>FS: Move dead adapter files
CLI->>Registry: Write deadAdapters.json with module paths
ModuleMap->>Registry: Load deadAdapters on startup
ModuleMap->>ModuleMap: Merge into dimensionsImports
sequenceDiagram
participant Client as Client Code
participant Registry as factory/registry.ts
participant DeadReg as deadAdapters.json
participant Factory as Adapter Factory
Client->>Registry: getAdapterFromHelpers(adapterType, protocolName)
Registry->>DeadReg: Check deadAdapters[adapterType][protocolName]
alt Dead Adapter Found
DeadReg-->>Registry: Return dead adapter entry
Registry-->>Client: Return {factoryName: 'deadAdapter', adapter}
else Not Found
Registry->>Factory: Load via dynamic factory
Factory-->>Registry: Return adapter
Registry-->>Client: Return adapter
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@factory/registry.ts`:
- Around line 51-57: The returned object is supplying metadata
(deadAdapters[adapterType][protocolName]) where the signature expects a
functional SimpleAdapter (AdapterBase & { adapter?: BaseAdapter }), and the
stored metadata's module field is corrupted by mockFunctions; fix by changing
the registry return to provide either a real adapter loader or a properly-typed
adapter: locate the deadAdapters map and instead of returning the raw metadata
object, use its modulePath/codePath to import/require and reconstruct a valid
SimpleAdapter instance (or return a wrapper that lazily imports and instantiates
the adapter) so callers like importAdapter.ts receive an AdapterBase-compatible
object; also update the types for deadAdapters and the return type of the
registry function to reflect metadata vs adapter (or convert metadata to a
proper SimpleAdapter before returning), and ensure migrateDeadProjects.ts does
not leave the module field with mockFunctions='_f' or provide a deserialization
step to restore real functions.
🧹 Nitpick comments (3)
cli/migrateDeadProjects.ts (3)
63-97: Inconsistent fs module usage.Line 67 uses
require('fs').statSyncwhileexistsSyncandreadFileSyncare already imported fromfsat line 2. Consider using the already-importedstatSyncfor consistency.Suggested fix
-import { existsSync, readFileSync } from "fs"; +import { existsSync, readFileSync, statSync } from "fs";- const fileStat = existsSync(filePath) ? require('fs').statSync(filePath) : null + const fileStat = existsSync(filePath) ? statSync(filePath) : null
99-151: Silent error swallowing may hide legitimate issues.Lines 147-149 silently catch and ignore all errors during adapter move operations. While some failures are expected (e.g., import errors for incompatible modules), completely silent failure could mask real problems like permission errors, disk space issues, or partially moved files.
Consider logging at least a debug message when errors occur.
Suggested improvement
} catch (error: any) { - // Skip modules that fail to import + // Log non-import errors for debugging + if (error.code && error.code !== 'MODULE_NOT_FOUND') { + console.log(` Warning: Failed to move ${moduleKey}: ${error.message}`) + } }
214-217: Minor: Unused loop variable.The
_variable is unused in the for-of loop. This is a common convention for ignored values, but you could destructure directly.Suggested fix
- for (const [_, info] of deadAdapterInfos) { + for (const info of deadAdapterInfos.values()) {
|
|
||
| if ((deadAdapters as any)[adapterType]?.[protocolName]) { | ||
| return { | ||
| factoryName: 'deadAdapter', | ||
| adapter: (deadAdapters as any)[adapterType][protocolName] | ||
| } | ||
| } |
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.
🧩 Analysis chain
🏁 Script executed:
rg -n -C3 'getAdapterFromHelpers'Repository: DefiLlama/dimension-adapters
Length of output: 1282
🏁 Script executed:
cat -n factory/registry.ts | sed -n '40,70p'Repository: DefiLlama/dimension-adapters
Length of output: 1269
🏁 Script executed:
rg -n -B5 -A10 'deadAdapters' factory/registry.ts | head -40Repository: DefiLlama/dimension-adapters
Length of output: 1047
🏁 Script executed:
fd migrateDeadProjects.tsRepository: DefiLlama/dimension-adapters
Length of output: 98
🏁 Script executed:
cat factory/deadAdapters.json | head -100Repository: DefiLlama/dimension-adapters
Length of output: 74
🏁 Script executed:
cat -n cli/migrateDeadProjects.ts | sed -n '120,145p'Repository: DefiLlama/dimension-adapters
Length of output: 1132
🏁 Script executed:
rg -n -B5 -A10 'function mockFunctions' cli/migrateDeadProjects.tsRepository: DefiLlama/dimension-adapters
Length of output: 483
🏁 Script executed:
# Check how the adapter field from getAdapterFromHelpers is used
rg -n -A5 'const result = getAdapterFromHelpers' adapters/utils/importAdapter.tsRepository: DefiLlama/dimension-adapters
Length of output: 384
🏁 Script executed:
# Check what SimpleAdapter looks like to confirm the incompatibility
rg -n -A10 'interface SimpleAdapter' adapters/typesRepository: DefiLlama/dimension-adapters
Length of output: 126
🏁 Script executed:
# Check if there are any other usages of result.adapter in importAdapter.ts
cat -n adapters/utils/importAdapter.tsRepository: DefiLlama/dimension-adapters
Length of output: 1881
🏁 Script executed:
fd types.ts | grep -v node_modulesRepository: DefiLlama/dimension-adapters
Length of output: 238
🏁 Script executed:
# Find SimpleAdapter definition
rg -n 'interface SimpleAdapter|type SimpleAdapter' --type tsRepository: DefiLlama/dimension-adapters
Length of output: 402
🏁 Script executed:
cat -n adapters/types.ts | sed -n '115,160p'Repository: DefiLlama/dimension-adapters
Length of output: 1847
Fix type mismatch: deadAdapters stores metadata objects, not SimpleAdapter instances.
The function returns deadAdapters[adapterType][protocolName] (a { modulePath, codePath, module } object created in migrateDeadProjects.ts lines 128-132) as the adapter field, but the return type signature declares adapter: SimpleAdapter. Callers like importAdapter.ts (line 41) expect a SimpleAdapter conforming to AdapterBase & { adapter?: BaseAdapter }, not metadata.
Additionally, the module field is corrupted by mockFunctions (lines 41-48 of migrateDeadProjects.ts), which replaces all functions with the string '_f'. When dead adapters are retrieved at runtime, the caller will receive an incompatible data structure instead of a functional adapter.
🤖 Prompt for AI Agents
In `@factory/registry.ts` around lines 51 - 57, The returned object is supplying
metadata (deadAdapters[adapterType][protocolName]) where the signature expects a
functional SimpleAdapter (AdapterBase & { adapter?: BaseAdapter }), and the
stored metadata's module field is corrupted by mockFunctions; fix by changing
the registry return to provide either a real adapter loader or a properly-typed
adapter: locate the deadAdapters map and instead of returning the raw metadata
object, use its modulePath/codePath to import/require and reconstruct a valid
SimpleAdapter instance (or return a wrapper that lazily imports and instantiates
the adapter) so callers like importAdapter.ts receive an AdapterBase-compatible
object; also update the types for deadAdapters and the return type of the
registry function to reflect metadata vs adapter (or convert metadata to a
proper SimpleAdapter before returning), and ensure migrateDeadProjects.ts does
not leave the module field with mockFunctions='_f' or provide a deserialization
step to restore real functions.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.