feat(core): refactor commands to use handler functions (Magek v2 Phase 1)#733
feat(core): refactor commands to use handler functions (Magek v2 Phase 1)#733javiertoledo wants to merge 2 commits intomainfrom
Conversation
…sses This is the first phase of Magek v2's data-oriented refactoring. Commands now use handler functions directly instead of class references, making the framework more composable and enabling a DSL-first approach. Changes: - Add `command()` DSL function in packages/core/src/dsl/command.ts - Update CommandMetadata to use `handler` function instead of `class` - Add `name` property to CommandMetadata - Update CommandDispatcher to call handler directly - Reimplement @command decorator as thin wrapper around DSL - Update GraphQL infrastructure to work with new metadata format - Add `generateGraphQLTypeForMetadata()` for metadata-based type generation Breaking changes: - CommandMetadata.class replaced with CommandMetadata.handler - CommandMetadata now requires `name` property Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b144b777e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const handler = async (input: unknown, register: Register): Promise<unknown> => { | ||
| // Create an instance from the input (mirroring old createInstance behavior) | ||
| const instance = Object.assign(Object.create(commandClass.prototype), input) | ||
| return commandClass.handle(instance, register) |
There was a problem hiding this comment.
Preserve constructor side effects when wrapping @command classes
The decorator now builds the handler by Object.assign(Object.create(commandClass.prototype), input) and skips calling the class constructor, whereas the old path used createInstance() which does new instanceClass() and therefore runs constructor logic. Any existing command class that sets defaults, validates, or derives fields in its constructor will now receive an instance without those side effects, which can change command handling behavior at runtime. If constructors are intentionally supported today, consider instantiating with new commandClass() (or reusing createInstance) to keep parity with the previous behavior.
Useful? React with 👍 / 👎.
- Commit the pnpm-lock.yaml which is required for CI/CD - Update CLAUDE.md with reminders to always run `rush update` and commit the lockfile before pushing changes Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
This PR implements the first phase of Magek v2's data-oriented refactoring, focusing on commands. The key architectural change is moving from class-based command handlers to direct handler functions, enabling a DSL-first approach where decorators become thin wrappers.
Changes
command()inpackages/core/src/dsl/command.tscreates command metadata with handler functionsCommandMetadata: Useshandlerfunction instead ofclassreference, addsnamepropertyCommandDispatcher: Callshandler(input, register)directly instead ofclass.handle()@Commanddecorator: Now a thin wrapper around the DSLcommand()functionTargetTypeMetadatausesnameinstead ofclassgenerateGraphQLTypeForMetadata()for metadata-based type generationBreaking Changes
CommandMetadata.classreplaced withCommandMetadata.handlerCommandMetadatanow requiresnamepropertyArchitecture (Before → After)
Test plan
@magek/corepassrush build)rush lint:check)rush test)Next Steps (Future PRs)
Following the incremental migration plan:
event()DSL, updateEventMetadata🤖 Generated with Claude Code