Skip to content

Latest commit

 

History

History
975 lines (744 loc) · 25.4 KB

File metadata and controls

975 lines (744 loc) · 25.4 KB

SimpleRouter - Production Readiness Analysis

Executive Summary

SimpleRouter is a lightweight and well-architected routing library for .NET applications, particularly designed for MVVM desktop applications using Avalonia UI. The library provides core navigation functionality with a clean API and solid foundation. However, to be considered production-ready, several improvements and enhancements are needed across code quality, features, documentation, and testing.

Current State: ✅ Functional core library with good test coverage
Production Ready: ⚠️ Needs improvements in several areas
Recommendation: Address critical issues first, then incrementally add missing features


Table of Contents

  1. Library Overview
  2. Strengths
  3. Critical Issues
  4. Code Quality Issues
  5. Missing Core Features
  6. Missing Documentation
  7. Testing Gaps
  8. DevOps & Infrastructure
  9. Implementation Checklist
  10. Priority Matrix

Library Overview

What is SimpleRouter?

SimpleRouter is a navigation/routing library that provides:

  • Stack-based navigation with forward/back support
  • Route lifecycle events (OnRouteChanging, OnRouteChanged)
  • Factory-based route creation with parameter support
  • Platform-agnostic core with Avalonia-specific extensions
  • View resolution and hosting for Avalonia applications

Architecture

Core Layer (SimpleRouter):
├── IRoute - Route interface
├── IRouter - Router interface  
├── IRouterHost - Host interface
├── Router - Main router implementation
└── Events - Navigation event arguments

Avalonia Layer (SimpleRouter.Avalonia):
├── RouteViewHost - Content control for displaying routes
└── ViewLocatorBase - Abstract view locator

Target Audience

  • Desktop application developers using MVVM pattern
  • Avalonia UI developers (primary)
  • WPF developers (with custom implementation)
  • Framework-agnostic usage possible

Strengths

Clean Architecture

  • Well-defined interfaces with single responsibility
  • Good separation of concerns between core and UI layers
  • Framework-agnostic core design

Good Test Coverage

  • 26 unit tests covering main scenarios
  • Uses mocking appropriately
  • Tests pass consistently

Simple API

  • Intuitive navigation methods (NavigateTo, NavigateBack, NavigateToAndReset)
  • Generic and type-based overloads
  • Event-driven design

NuGet Package Ready

  • Properly configured .csproj files
  • Multi-targeting (netstandard2.1, net8.0)
  • Package metadata configured

CI/CD Setup

  • GitHub Actions for testing
  • Automated builds on PR and push

Critical Issues

🔴 Priority: CRITICAL

1. Memory Leak in RouteViewHost

Location: SimpleRouter.Avalonia/RouteViewHost.cs:30

Issue: Event handler subscribed but never unsubscribed when Router changes or control is disposed.

router.OnRouteChanged += Router_OnRouteChanged;

Impact: Memory leaks in applications with dynamic router changes.

Fix Required:

protected override void OnPropertyChanged(AvaloniaPropertyChangedEventArgs change)
{
    base.OnPropertyChanged(change);
    switch (change.Property.Name)
    {
        case nameof(Router):
            // Unsubscribe from old router
            if (change.OldValue is IRouter oldRouter)
            {
                oldRouter.OnRouteChanged -= Router_OnRouteChanged;
            }
            
            // Subscribe to new router
            if (change.NewValue is IRouter newRouter)
            {
                newRouter.OnRouteChanged += Router_OnRouteChanged;
                NavigateToRoute(newRouter.Current);
            }
            break;
    }
}

2. Stack Limit Not Applied

Location: SimpleRouter/Router.cs:12-15

Issue: Constructor accepts stackLimit parameter but never assigns it to _stackLimit field.

public Router(RouteFactory createRoute, int stackLimit = 50)
{
    _createRoute = createRoute ?? throw new ArgumentNullException(nameof(createRoute));
    // BUG: stackLimit parameter is ignored!
}

Impact: Stack limit is always 50 regardless of constructor parameter.

Fix Required:

public Router(RouteFactory createRoute, int stackLimit = 50)
{
    _createRoute = createRoute ?? throw new ArgumentNullException(nameof(createRoute));
    _stackLimit = stackLimit;
}

Code Quality Issues

🟡 Priority: HIGH

1. Unused Import

Location: SimpleRouter/Router.cs:1

Issue:

using System.Net.Sockets; // Not used anywhere

Fix: Remove the unused import.

2. Nullable Reference Type Warnings

Locations: Test files have 3 warnings for null literal conversions

Files:

  • AvaloniaRouteViewHostTests.cs:17,53
  • RouterTests.cs:303

Fix: Use null! or proper nullable annotations in tests.

3. Missing XML Documentation

Issue: Many public APIs lack XML documentation comments.

Examples:

  • IRoute.RouteName - No description
  • IRoute.RouterHost - No description
  • Router methods - Minimal documentation
  • Events.cs - No documentation on event args properties

Impact: IntelliSense provides limited help to library users.

4. Magic Numbers

Location: Throughout Router.cs

Examples:

  • Stack limit default of 50 (what's the rationale?)
  • Stack index calculations using literals

Fix: Add constants with descriptive names and documentation.


Missing Core Features

🟡 Priority: HIGH

1. Navigation Guards/Middleware

Description: No way to prevent or intercept navigation.

Use Cases:

  • Authorization checks before navigating
  • Unsaved changes confirmation
  • Conditional navigation based on application state

Proposed API:

public interface INavigationGuard
{
    Task<bool> CanNavigateFrom(IRoute? current, IRoute? next);
    Task<bool> CanNavigateTo(IRoute? current, IRoute? next);
}

2. Route Parameters & Query Strings

Description: No structured way to pass parameters beyond constructor args.

Use Cases:

  • Deep linking
  • Bookmarkable routes
  • Passing optional parameters

Proposed API:

public interface IRoute
{
    string RouteName { get; }
    IRouterHost RouterHost { get; }
    IReadOnlyDictionary<string, object>? Parameters { get; } // NEW
}

// Usage:
router.NavigateTo<UserProfileRoute>(new { userId = "123", tab = "settings" });

3. Navigation Cancellation

Description: No way to cancel in-progress navigation.

Use Cases:

  • Cancel navigation if user navigates away quickly
  • Cancel if validation fails asynchronously

Proposed API:

public sealed class RouteChangingEventArgs(IRoute? previous, IRoute? next) : EventArgs
{
    public IRoute? Previous { get; } = previous;
    public IRoute? Next { get; } = next;
    public bool Cancel { get; set; } // NEW
}

4. Async Navigation Support

Description: All navigation is synchronous.

Use Cases:

  • Load data before navigating
  • Async validation
  • Async guards/middleware

Proposed API:

Task<IRoute> NavigateToAsync<T>() where T : IRoute;
Task<IRoute?> NavigateBackAsync();

5. Route State/Data Passing

Description: No way to pass transient state between routes without constructor parameters.

Use Cases:

  • Passing selected item from list to detail view
  • Returning results from modal routes
  • Wizard-style navigation with accumulated state

Proposed API:

router.NavigateTo<DetailRoute>(state: selectedItem);

🟢 Priority: MEDIUM

6. Deep Linking Support

Description: No URI-based routing or route registration.

Use Cases:

  • Restore application state from URL
  • External navigation (e.g., from email links)
  • Bookmarking specific views

Proposed API:

router.RegisterRoute<HomeRoute>("home");
router.RegisterRoute<UserProfileRoute>("users/{userId}");
router.NavigateToUri("users/123");

7. Route Lifecycle Hooks

Description: No hooks for route entry/exit logic.

Use Cases:

  • Initialize data when entering route
  • Cleanup resources when leaving route
  • Track analytics

Proposed API:

public interface IRoutable : IRoute
{
    Task OnNavigatedTo(NavigationContext context);
    Task OnNavigatedFrom(NavigationContext context);
}

8. Navigation History Management

Description: Basic stack only, no advanced history manipulation.

Missing:

  • Clear history
  • Remove specific items from stack
  • Insert items at specific positions
  • History length limits per route type

9. Logging & Diagnostics

Description: No built-in logging or diagnostic information.

Needed:

  • Navigation event logging
  • Performance metrics
  • Error tracking
  • Debug mode with detailed traces

Proposed:

public interface IRouterLogger
{
    void LogNavigation(IRoute? from, IRoute? to);
    void LogNavigationFailed(IRoute? from, IRoute? to, Exception ex);
}

10. Route Caching Strategies

Description: New route instance created on every navigation.

Use Cases:

  • Reuse existing route instances
  • Preserve scroll position and UI state
  • Reduce memory allocations

Proposed:

public enum RouteCachingStrategy
{
    AlwaysCreate,      // Current behavior
    ReuseIfExists,     // Cache and reuse
    Singleton          // Only one instance ever
}

11. Source Generators for Performance

Description: No compile-time code generation for route registration and factory optimization.

Current Problem:

  • RouteFactory delegate relies on reflection or manual factory implementations
  • ViewLocatorBase.TryDeduceControl() uses reflection (Type.GetType, Activator.CreateInstance)
  • Route creation has runtime overhead from reflection
  • No compile-time validation of route configurations

Use Cases:

  • Auto-generate optimized route factory implementations at compile time
  • Eliminate reflection overhead in route and view creation
  • Generate strongly-typed navigation extension methods
  • Create compile-time route validation and diagnostics
  • Reduce startup time and memory allocations
  • Improve AOT (Ahead-of-Time) compilation compatibility

Benefits:

  • Zero runtime reflection - All route types known at compile time
  • Type safety - Compile-time errors for invalid routes or missing views
  • Performance - 10-100x faster route creation (no Activator.CreateInstance)
  • AOT-friendly - Works with Native AOT compilation
  • Intellisense support - Better IDE experience with generated code
  • Startup time - Faster application initialization

Real-World Example: Similar to StaticViewLocator by Wiesław Sołtes, which generates static view resolution code for Avalonia, eliminating reflection from view locators.

Proposed API:

// 1. Mark router for generation
[GeneratedRouter]
public partial class AppRouter : IRouter
{
    // Source generator will implement the factory
}

// 2. Mark routes for registration
[Route]
public class HomeRoute : IRoute
{
    public HomeRoute(IRouterHost host) { ... }
}

[Route]
public class UserProfileRoute : IRoute
{
    public UserProfileRoute(IRouterHost host, string userId, int? tab = null) { ... }
}

// 3. Auto-generated code (example output)
public partial class AppRouter
{
    // Generated static factory dictionary
    private static readonly Dictionary<Type, Func<IRouterHost, object[], IRoute>> s_routeFactories = new()
    {
        [typeof(HomeRoute)] = (host, args) => new HomeRoute(host),
        [typeof(UserProfileRoute)] = (host, args) => new UserProfileRoute(
            host, 
            (string)args[0], 
            args.Length > 1 ? (int?)args[1] : null),
    };
    
    // Generated strongly-typed navigation extensions
    public static class Extensions
    {
        public static HomeRoute NavigateToHome(this IRouter router)
            => (HomeRoute)router.NavigateTo<HomeRoute>();
            
        public static UserProfileRoute NavigateToUserProfile(
            this IRouter router, 
            string userId, 
            int? tab = null)
            => (UserProfileRoute)router.NavigateTo<UserProfileRoute>(userId, tab);
    }
}

// 4. Usage in application
router.NavigateToUserProfile("user123", tab: 2); // Strongly-typed, no reflection!

Generated ViewLocator Enhancement:

// Mark view locator for generation
[GeneratedViewLocator]
public partial class AppViewLocator : ViewLocatorBase
{
    // Generated view resolution (similar to StaticViewLocator)
    private static readonly Dictionary<Type, Func<Control>> s_views = new()
    {
        [typeof(HomeRoute)] = () => new HomeView(),
        [typeof(UserProfileRoute)] = () => new UserProfileView(),
    };
    
    public override Control? ResolveControl(IRoute? route)
    {
        if (route is null) return null;
        
        var type = route.GetType();
        return s_views.TryGetValue(type, out var factory) 
            ? factory() 
            : null;
    }
}

Implementation Notes:

  • Use Roslyn IIncrementalSourceGenerator (C# 9.0+)
  • Generate partial classes and extension methods
  • Support incremental generation for fast builds
  • Provide diagnostics for missing routes/views
  • Convention-based: *Route*View mapping
  • Generate XML documentation for generated methods
  • Support for nullable reference types
  • Compatible with Native AOT compilation

Performance Impact:

  • Route creation: ~100x faster (static instantiation vs reflection)
  • View resolution: ~10-50x faster (dictionary lookup vs Type.GetType)
  • Memory: Lower allocation (no runtime type resolution)
  • Startup: Faster (no reflection scanning)
  • AOT: Full support (no reflection metadata needed)

🟢 Priority: LOW

12. Route Transitions/Animations

Description: Basic content switching, no transition control.

Note: Avalonia's TransitioningContentControl provides some support, but no programmatic control.

13. Modal/Dialog Route Support

Description: No concept of modal vs. page routes.

Use Cases:

  • Display route as modal dialog
  • Block navigation while modal is active
  • Return values from modal routes

14. Nested Routers

Description: No explicit support for nested routing hierarchies.

Use Cases:

  • Master-detail layouts with independent navigation
  • Tab-based navigation
  • Multi-pane applications

15. Route Templates/Conventions

Description: No way to define route naming conventions or patterns.


Missing Documentation

🟡 Priority: HIGH

1. CHANGELOG.md

Status: ❌ Missing

Needed: Track version changes, breaking changes, and migration guides.

Template:

# Changelog

## [1.0.3] - 2024-XX-XX
### Added
- New feature X

### Changed
- Breaking change Y

### Fixed
- Bug Z

2. API Documentation (XML Comments)

Status: ⚠️ Incomplete

Needed:

  • XML comments on all public APIs
  • Code examples in comments
  • Remarks for complex behavior
  • See also references

3. Architecture Documentation

Status: ❌ Missing

Needed:

  • Design decisions and rationale
  • Extension points
  • Performance characteristics
  • Threading model

🟢 Priority: MEDIUM

4. CONTRIBUTING.md

Status: ❌ Missing

Needed:

  • How to contribute
  • Code style guide
  • PR process
  • Development setup

5. CODE_OF_CONDUCT.md

Status: ❌ Missing

Needed: Community guidelines (standard for open source).

6. SECURITY.md

Status: ❌ Missing

Needed:

  • Security policy
  • How to report vulnerabilities
  • Supported versions

7. Migration Guides

Status: ❌ Missing

Needed:

  • Migration from ReactiveUI routing
  • Breaking changes between versions

8. Advanced Usage Examples

Status: ⚠️ Limited

Current: Basic sample app exists.

Needed:

  • Nested navigation example
  • Parameter passing example
  • Event handling example
  • Custom route factory example
  • Error handling example

9. Performance Guidelines

Status: ❌ Missing

Needed:

  • Stack limit recommendations
  • Memory usage patterns
  • Performance best practices

Testing Gaps

🟡 Priority: HIGH

1. Thread Safety Tests

Status: ❌ Missing

Needed:

  • Concurrent navigation from multiple threads
  • Race condition testing
  • Thread-safe event subscription

2. Memory Leak Tests

Status: ❌ Missing

Needed:

  • Verify event handlers are cleaned up
  • Long-running navigation stress tests
  • Dispose pattern verification

3. Edge Case Coverage

Status: ⚠️ Partial

Missing:

  • NavigateBack when stack has only one item (test exists but limited)
  • NavigateTo same route multiple times
  • Rapid navigation (spam navigation calls)
  • Stack overflow scenarios
  • Route factory returning null (test exists but could be expanded)

🟢 Priority: MEDIUM

4. Integration Tests

Status: ❌ Missing

Needed:

  • End-to-end navigation flows
  • Avalonia RouteViewHost integration tests
  • View locator integration tests

5. Performance Tests

Status: ❌ Missing

Needed:

  • Navigation speed benchmarks
  • Memory usage benchmarks
  • Stack size impact tests

6. Mutation Testing

Status: ❌ Missing

Purpose: Verify test quality by introducing code mutations.


DevOps & Infrastructure

🟡 Priority: HIGH

1. Code Coverage Reporting

Status: ❌ Missing

Needed:

  • Coverage badge in README
  • Coverage reports in CI
  • Minimum coverage threshold

Tools: Coverlet, Codecov, or Coveralls

2. Release Automation

Status: ⚠️ Partial

Current: Manual version bumps and NuGet publishing.

Needed:

  • Automated version tagging
  • Automated NuGet package publishing
  • Automated release notes generation

🟢 Priority: MEDIUM

3. Semantic Versioning Enforcement

Status: ❌ Missing

Needed:

  • Git hooks or CI checks for version increments
  • Breaking change detection

4. Pre-commit Hooks

Status: ❌ Missing

Needed:

  • Format code automatically
  • Run fast unit tests
  • Lint commit messages

5. Dependabot/Dependency Updates

Status: ❌ Missing

Needed:

  • Automated dependency update PRs
  • Security vulnerability scanning

6. Benchmarking Suite

Status: ❌ Missing

Needed:

  • Performance regression detection
  • Benchmark results tracking over time

Tool: BenchmarkDotNet


Implementation Checklist

Phase 1: Critical Fixes (Required for Production)

  • Fix memory leak in RouteViewHost

    • Unsubscribe from old router events
    • Add disposal test
  • Fix stack limit bug in Router constructor

    • Assign stackLimit parameter to field
    • Add test to verify custom stack limits work
  • Remove unused import in Router.cs

    • Delete using System.Net.Sockets;
  • Fix nullable reference warnings in tests

    • Update test code to use proper null handling
  • Add thread safety to Router

    • Add locking around stack operations
    • Add concurrent access tests

Phase 2: Essential Documentation

  • Create CHANGELOG.md

    • Document all versions since 1.0.0
    • Establish changelog format
  • Add XML documentation comments

    • Document all public APIs in SimpleRouter
    • Document all public APIs in SimpleRouter.Avalonia
    • Include code examples where helpful
  • Create CONTRIBUTING.md

    • Document contribution process
    • Add code style guidelines
    • Include development setup instructions
  • Create SECURITY.md

    • Define security policy
    • Add vulnerability reporting process

Phase 3: Core Features (High Priority)

  • Add navigation guards

    • Define INavigationGuard interface
    • Integrate guards into Router
    • Add guard tests
    • Document usage with examples
  • Add navigation cancellation

    • Add Cancel property to RouteChangingEventArgs
    • Implement cancellation logic
    • Add cancellation tests
  • Add route parameters support

    • Extend IRoute with Parameters property
    • Update NavigateTo methods
    • Add parameter tests
    • Document parameter patterns
  • Add async navigation support

    • Add async NavigateTo methods
    • Add async NavigateBack
    • Add async guard support
    • Add async tests

Phase 4: Testing & Quality

  • Add thread safety tests

    • Concurrent navigation tests
    • Race condition tests
  • Add integration tests

    • End-to-end navigation tests
    • Avalonia integration tests
  • Set up code coverage

    • Add Coverlet to tests
    • Configure coverage reporting
    • Add coverage badge to README
    • Set minimum coverage threshold (80%+)

Phase 5: Advanced Features (Medium Priority)

  • Add deep linking support

    • Design URI routing API
    • Implement route registration
    • Add URI parsing and matching
    • Add deep linking tests
  • Add route lifecycle hooks

    • Define IRoutable interface
    • Implement lifecycle method calls
    • Add lifecycle tests
  • Add logging support

    • Define IRouterLogger interface
    • Add logging integration points
    • Add logging tests
  • Add route caching

    • Define caching strategies
    • Implement route cache
    • Add cache tests
  • Implement source generators for performance

    • Create source generator project
    • Generate route factory implementations
    • Generate strongly-typed navigation extensions
    • Add compile-time route validation
    • Add generator tests and documentation

Phase 6: Infrastructure & DevOps

  • Set up release automation

    • Automate NuGet publishing
    • Generate release notes automatically
    • Add version tagging workflow
  • Add pre-commit hooks

    • Set up Husky or similar
    • Configure format check
    • Configure test run
  • Add dependency automation

    • Enable Dependabot
    • Configure update schedule
  • Add benchmarking

    • Create BenchmarkDotNet project
    • Add navigation benchmarks
    • Track benchmark results

Phase 7: Polish & Additional Documentation

  • Create CODE_OF_CONDUCT.md

    • Use Contributor Covenant or similar
  • Add architecture documentation

    • Document design decisions
    • Add sequence diagrams
    • Document extension points
  • Add migration guides

    • ReactiveUI migration guide
    • Version migration guides
  • Expand examples

    • Add advanced usage examples
    • Add error handling examples
    • Add nested navigation example

Priority Matrix

Must Have (Production Blockers)

  1. Fix memory leak in RouteViewHost ⚠️
  2. Fix stack limit bug ⚠️
  3. Add thread safety
  4. XML documentation for all public APIs
  5. CHANGELOG.md

Should Have (Production Ready)

  1. Navigation guards
  2. Navigation cancellation
  3. Route parameters
  4. Async navigation
  5. Code coverage reporting
  6. CONTRIBUTING.md
  7. SECURITY.md

Nice to Have (Enhanced Production)

  1. Deep linking
  2. Route lifecycle hooks
  3. Logging support
  4. Route caching
  5. Source generators for performance
  6. Integration tests
  7. Performance tests
  8. Release automation
  9. Advanced examples

Future Considerations

  1. Modal/dialog routes
  2. Nested routers
  3. Route templates
  4. Animation control
  5. Benchmarking suite

Recommendations

Immediate Actions (This Week)

  1. ✅ Fix the memory leak - this is a critical bug
  2. ✅ Fix the stack limit bug - breaks documented API
  3. ✅ Remove unused import - code cleanliness
  4. ✅ Fix nullable warnings - clean builds
  5. ✅ Add comprehensive XML docs - developer experience

Short Term (This Month)

  1. Add thread safety with proper locking
  2. Create CHANGELOG.md and maintain it going forward
  3. Add navigation guards - commonly requested feature
  4. Add navigation cancellation - important for UX
  5. Set up code coverage reporting

Medium Term (This Quarter)

  1. Implement route parameters
  2. Add async navigation support
  3. Create comprehensive documentation (CONTRIBUTING, SECURITY)
  4. Add integration and performance tests
  5. Implement deep linking

Long Term (Ongoing)

  1. Maintain high test coverage (>80%)
  2. Regular dependency updates
  3. Monitor and respond to community issues
  4. Consider additional platform support (WPF, MAUI)
  5. Performance optimization based on benchmarks

Conclusion

SimpleRouter has a solid foundation with clean architecture and good core functionality. The library successfully delivers on its promise of being "simple" while providing essential routing capabilities.

To be production-ready, the library needs:

  1. Critical bug fixes (memory leak, stack limit)
  2. Thread safety for concurrent usage
  3. Complete documentation (XML comments, CHANGELOG)
  4. Essential features (guards, cancellation, parameters)
  5. Quality assurance (coverage reporting, integration tests)

Estimated Effort:

  • Critical fixes: 1-2 days
  • Essential documentation: 2-3 days
  • Core features: 1-2 weeks
  • Testing & quality: 1 week
  • Advanced features: 2-4 weeks (ongoing)

Current Status: 7/10 - Good foundation, needs refinement for production use. With Phase 1-3 Complete: 9/10 - Production-ready with room for enhancement.

The library is already usable for internal projects and prototypes. With the critical fixes and documentation improvements, it will be ready for production use in commercial applications.