-
Notifications
You must be signed in to change notification settings - Fork 45
Add component wrapper #315
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
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 introduces a component wrapper system to organize services into a nested hierarchy, replacing the previous pattern where components directly appended services to the manifest. Each component now explicitly creates a Component object containing services and other components, enabling reusable component instantiation with different namespaces.
Changes:
- Refactored
Applymethods across all components and recipes to return*Componentinstead of modifying a manifest directly - Updated
Manifestcreation to accept a rootComponentand flatten its hierarchy into services - Changed recipe and component interfaces to work with
*ExContextinstead of*Manifest
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| playground/manifest.go | Introduces Component struct and ComponentGen interface, refactors manifest construction to accept and flatten component hierarchies |
| playground/components.go | Updates all component Apply methods to create and return Component instances instead of modifying manifest |
| playground/recipe_opstack.go | Refactors OpRecipe to build and return component hierarchy |
| playground/recipe_l1.go | Refactors L1Recipe to build and return component hierarchy |
| playground/recipe_buildernet.go | Refactors BuilderNetRecipe to compose components |
| playground/manifest_test.go | Updates test to use new component-based manifest construction |
| playground/components_test.go | Updates test framework to work with new component pattern |
| playground/docs.go | Updates documentation generation to use new component approach |
| playground/catalog.go | Changes component registration type from Component to ComponentGen |
| main.go | Updates main execution flow to create manifest from returned components |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
playground/components.go
Outdated
| func (r *RollupBoost) Apply(manifest *Manifest) { | ||
| service := manifest.NewService("rollup-boost"). | ||
| func (r *RollupBoost) Apply(ctx *ExContext) *Component { | ||
| component := NewComponent("bproxy") |
Copilot
AI
Jan 16, 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 component name "bproxy" appears to be a copy-paste error - this is the RollupBoost component, not BProxy. The component name should be "rollup-boost" to match the service name and component purpose.
| component := NewComponent("bproxy") | |
| component := NewComponent("rollup-boost") |
playground/components.go
Outdated
| func (o *OpRbuilder) Apply(manifest *Manifest) { | ||
| service := manifest.NewService("op-rbuilder"). | ||
| func (o *OpRbuilder) Apply(ctx *ExContext) *Component { | ||
| component := NewComponent("bproxy") |
Copilot
AI
Jan 16, 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 component name "bproxy" is incorrect for OpRbuilder. The component name should be "op-rbuilder" to match the service name and component purpose.
| component := NewComponent("bproxy") | |
| component := NewComponent("op-rbuilder") |
playground/components.go
Outdated
|
|
||
| func (f *FlashblocksRPC) Apply(manifest *Manifest) { | ||
| func (f *FlashblocksRPC) Apply(ctx *ExContext) *Component { | ||
| component := NewComponent("bproxy") |
Copilot
AI
Jan 16, 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 component name "bproxy" is incorrect for FlashblocksRPC. The component name should be "flashblocks-rpc" to match the service name and component purpose.
| component := NewComponent("bproxy") | |
| component := NewComponent("flashblocks-rpc") |
| func (w *WebsocketProxy) Apply(manifest *Manifest) { | ||
| manifest.NewService("websocket-proxy"). | ||
| func (w *WebsocketProxy) Apply(ctx *ExContext) *Component { | ||
| component := NewComponent("webproxy") |
Copilot
AI
Jan 16, 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 component name "webproxy" should be "websocket-proxy" to match the service name and maintain consistency with component naming conventions.
| component := NewComponent("webproxy") | |
| component := NewComponent("websocket-proxy") |
This PR is a followup on #304 and a way to fix #305. Before, the components would append their services to the manifest. Now, each component explicitely creates a
Componentobject which containsServicesand otherComponentobjects. We create a nested hierarchy of Components and Services.This is useful because we can instantiate the same Component with a different namespace later on like:
or