Skip to content

feat(lvt): migrate templates to method dispatch pattern#6

Merged
adnaan merged 1 commit intomainfrom
remove-store-interface
Dec 4, 2025
Merged

feat(lvt): migrate templates to method dispatch pattern#6
adnaan merged 1 commit intomainfrom
remove-store-interface

Conversation

@adnaan
Copy link
Contributor

@adnaan adnaan commented Dec 4, 2025

Summary

  • Migrate all generator templates from Store.Change() switch-case pattern to automatic method dispatch
  • Update all kit templates (simple, single, multi) to use the same pattern
  • Migrate E2E tests to method dispatch
  • Update golden files and README documentation

This aligns with the livetemplate core v0.6.0 changes that removed the Store interface in favor of method dispatch (PR #67).

Changes

Generator Templates

  • resource/handler.go.tmpl: 13 actions converted to individual methods (Add, Edit, Update, CancelEdit, View, Back, Delete, Search, Sort, NextPage, PrevPage, GotoPage, LoadMore)
  • view/handler.go.tmpl: Removed Change(), added comment with example method
  • app/home.go.tmpl: Removed Change() (static page, no actions)

Kit Templates

  • simple: main.go.tmpl - Increment, Decrement, Reset methods
  • single: All templates mirrored from generator
  • multi: All templates mirrored from generator + auth handler

Auth Handler

  • Register(), Login(), MagicLink(), ForgotPassword(), SwitchView() methods

E2E Tests

  • LoadingTestState: Removed empty Change() method
  • FocusTestState: Converted to Increment() method

Test plan

  • Run go test -v ./... -short - all unit tests pass
  • Run focus/loading e2e tests - pass
  • Golden file tests updated and passing

Breaking Change

Generated code will use method dispatch instead of Change() switch statements. Existing apps need to migrate manually.

🤖 Generated with Claude Code

Remove Store interface with Change() method in favor of automatic
method dispatch. Actions now route to methods by name (e.g., action
"increment" -> Increment() method).

Changes:
- Generator templates: Convert Change() switch to individual methods
- Kit templates (simple, single, multi): Same pattern
- Auth handler: Register, Login, MagicLink, ForgotPassword, SwitchView
- E2E tests: LoadingTestState, FocusTestState migrated
- Golden files and README updated

BREAKING CHANGE: Generated code no longer uses Change() method.
Existing apps must migrate to method dispatch pattern.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings December 4, 2025 08:36
@claude
Copy link

claude bot commented Dec 4, 2025

Code Review: Migration to Method Dispatch Pattern

I've completed a comprehensive review of PR #6. Overall, this is an excellent refactoring that modernizes the codebase and aligns with livetemplate core v0.6.0. Here's my detailed analysis:


Strengths

1. Clean Architecture Migration

  • Successfully migrated from switch-case Change() method to automatic method dispatch
  • All 13 CRUD actions now have dedicated methods with clear responsibilities
  • Method names follow proper Go naming conventions (e.g., Add, Edit, Update, CancelEdit)

2. Excellent Code Organization

  • Each action method is self-contained and well-documented with clear godoc comments
  • Consistent structure across all templates (resource, view, auth handlers)
  • Methods properly update LastUpdated field consistently

3. Strong Test Coverage

  • E2E tests properly migrated (LoadingTestState, FocusTestState)
  • Golden files updated to reflect new patterns
  • Tests demonstrate the new method dispatch approach clearly

4. Documentation Quality

  • README.md updated with clear examples of the new pattern
  • Breaking change clearly documented
  • Good inline code examples showing migration path

🔍 Code Quality Observations

Positive Patterns:

  1. Context Handling: Good use of context.Background() for database operations
  2. Error Handling: Proper error wrapping with fmt.Errorf and error propagation
  3. Validation: Consistent use of ctx.BindAndValidate with validator tags
  4. Action Comments: Each method has helpful godoc explaining its purpose

Example of Clean Implementation:

// Add handles the "add" action to create a new resource
func (s *UserState) Add(ctx *livetemplate.ActionContext) error {
    var input AddInput
    if err := ctx.BindAndValidate(&input, validate); err != nil {
        return err
    }
    // ... clean implementation
    s.LastUpdated = formatTime()
    return nil
}

🎯 Potential Improvements (Minor)

1. Context Parameter Usage

Some methods accept ctx *livetemplate.ActionContext but ignore it with _:

func (s *State) CancelEdit(_ *livetemplate.ActionContext) error
func (s *State) Back(_ *livetemplate.ActionContext) error
func (s *State) NextPage(_ *livetemplate.ActionContext) error
func (s *State) PrevPage(_ *livetemplate.ActionContext) error

Suggestion: This is acceptable but consider if these could be simplified to not require the context parameter at all in the livetemplate core. (This may be a core library design decision, so not a blocker.)

2. Duplicate Database Context Creation

Every method creates dbCtx := context.Background(). Consider:

// Helper method
func (s *State) dbContext() context.Context {
    return context.Background()
}

This would make it easier to add context timeouts or tracing later. However, this is very minor and current approach is fine.

3. Auth Handler - Action Name Consistency

In auth handler, action names use underscores (switch_view) but method names use camelCase (SwitchView). This is correct per Go conventions, but ensure the routing layer properly converts action names.


🔒 Security Analysis

✅ Secure Practices:

  1. Password Hashing: Proper use of bcrypt in auth handlers
  2. SQL Injection: Protected via sqlc generated queries
  3. Input Validation: Consistent validation with struct tags
  4. Context Usage: Proper context passing for database operations

⚠️ Minor Security Note:

The auth handler logs errors that could contain sensitive info:

log.Printf("Password hash error: %v", err)

Recommendation: Consider using structured logging that can filter sensitive data in production.


🐛 Potential Bugs

✅ No Critical Bugs Found

The migration is clean and consistent. All patterns are applied uniformly across:

  • Generator templates (resource, view, app)
  • Kit templates (simple, single, multi)
  • Auth handlers
  • E2E tests

🧪 Test Coverage

✅ Excellent Test Migration:

  1. E2E Tests:

    • FocusTestState.Increment() properly migrated from switch-case
    • LoadingTestState correctly simplified (no actions needed)
  2. Golden Files:

    • Updated to reflect new method dispatch pattern
    • Proper validation of generated code
  3. Test Plan Completed:

    • ✅ Unit tests passing
    • ✅ E2E tests passing
    • ✅ Golden file tests updated

Recommendation: The PR description mentions tests pass. Consider adding a CI check output to the PR for verification.


📊 Performance Considerations

✅ Neutral to Positive Impact:

  1. Method Dispatch vs Switch:

    • Go's method dispatch is highly optimized
    • Slight improvement in readability and maintainability
    • No measurable performance difference expected
  2. Memory Allocation:

    • No additional allocations introduced
    • Same patterns for database operations
  3. Code Size:

    • Net increase of ~246 lines (1049 additions, 803 deletions)
    • Increase is due to better organization, not bloat
    • More methods = better testability

📝 Code Style & Best Practices

✅ Follows Go Best Practices:

  1. Naming: Clear, idiomatic Go names
  2. Documentation: Good godoc comments on all public methods
  3. Error Handling: Proper error wrapping and propagation
  4. Package Organization: Logical structure maintained

Style Consistency:

  • All methods follow same pattern: validate → process → update state → return
  • Consistent use of formatTime() for timestamp updates
  • Proper use of pointer receivers

🎓 Breaking Change Assessment

Well-Documented Breaking Change:

The PR correctly identifies this as a breaking change:

  • Clear migration path provided in README
  • Example code shows before/after
  • Commit message properly marked with BREAKING CHANGE:

Migration Guide for Users:
The README example is clear:

// Before (switch-case)
func (s *State) Change(ctx *livetemplate.ActionContext) error {
    switch ctx.Action {
    case "add": // ...
    }
}

// After (method dispatch)
func (s *State) Add(ctx *livetemplate.ActionContext) error {
    // ...
}

🎯 Final Verdict

APPROVED

This is a high-quality refactoring that:

  • ✅ Successfully achieves its migration goals
  • ✅ Maintains code quality and consistency
  • ✅ Includes comprehensive test coverage
  • ✅ Properly documents breaking changes
  • ✅ No security vulnerabilities introduced
  • ✅ No performance regressions expected
  • ✅ Follows Go best practices throughout

Recommendation:

Merge with confidence. This PR represents a significant architectural improvement that will make the codebase more maintainable and easier to understand.

Minor Follow-ups (Optional, Post-Merge):

  1. Consider adding context timeout examples for database operations
  2. Consider structured logging for auth errors
  3. Monitor for any edge cases in method dispatch routing

Great work on this migration! The consistency across all templates and the thorough test coverage demonstrates careful attention to detail. 🎉

Copy link

Copilot AI left a 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 migrates all LiveTemplate generator templates from the Store.Change() switch-case pattern to the new method dispatch pattern introduced in livetemplate core v0.6.0. The migration removes the Store interface requirement and enables automatic action routing based on method names, making the code more idiomatic Go with better IDE support.

Key Changes

  • Handler pattern migration: All 13 resource actions (Add, Edit, Update, CancelEdit, View, Back, Delete, Search, Sort, NextPage, PrevPage, GotoPage, LoadMore) converted from switch cases to individual methods
  • API updates: Adapted to v0.6.0 where livetemplate.New() no longer returns an error
  • Simplified initialization: Removed defensive Init() logic since EditingID/EditingResource are now set after initialization in page mode
  • Documentation: Updated README examples and inline comments to demonstrate the new method dispatch pattern

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.

Show a summary per file
File Description
testdata/golden/view_handler.go.golden Golden file updated: removed empty Change(), added example comment for action methods
testdata/golden/resource_handler.go.golden Golden file updated: 13 resource actions converted to methods, livetemplate.New() error handling removed
internal/generator/templates/view/handler.go.tmpl Generator template updated: helpful example comment for adding action methods
internal/generator/templates/resource/handler.go.tmpl Generator template updated: all CRUD and pagination actions converted to methods
internal/generator/templates/app/home.go.tmpl Home page template updated: removed empty Change() method
internal/kits/system/simple/templates/app/main.go.tmpl Simple kit updated: Increment/Decrement/Reset as individual methods
internal/kits/system/single/templates/* Single-page kit templates mirror generator changes
internal/kits/system/multi/templates/* Multi-page kit templates mirror generator changes, including auth handler (Register, Login, MagicLink, ForgotPassword, SwitchView methods)
e2e/livetemplate_core_test.go E2E tests updated: removed empty Change() from LoadingTestState, FocusTestState uses Increment() method
README.md Documentation updated: shows method dispatch pattern in example code

@adnaan adnaan merged commit 78c60da into main Dec 4, 2025
7 of 8 checks passed
@adnaan adnaan deleted the remove-store-interface branch December 4, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants