diff --git a/.gitignore b/.gitignore index 63ebc45..b94df65 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ build .cxx local.properties .kotlin +**/bin/ diff --git a/.serena/.gitignore b/.serena/.gitignore new file mode 100644 index 0000000..14d86ad --- /dev/null +++ b/.serena/.gitignore @@ -0,0 +1 @@ +/cache diff --git a/.serena/memories/architecture_patterns.md b/.serena/memories/architecture_patterns.md new file mode 100644 index 0000000..6c58046 --- /dev/null +++ b/.serena/memories/architecture_patterns.md @@ -0,0 +1,73 @@ +# Nibel Architecture & Design Patterns + +## Core Architecture Patterns + +### Navigation Controller Pattern + +- **NavigationController**: Central navigation component injected into Composables +- **Entry Classes**: Generated navigation entry points (e.g., `MyScreenEntry`) +- **Type-safe**: Compile-time checking for navigation arguments and types + +### Annotation-Driven Code Generation + +- **@UiEntry**: Internal screen entries (single module) +- **@UiExternalEntry**: External entries (multi-module) +- **@LegacyEntry/@LegacyExternalEntry**: Fragment integration +- **KSP Processor**: Generates entry classes and navigation boilerplate + +### Multi-Module Navigation Pattern + +``` + featureA featureB + module module + │ │ + └──► navigation ◄──┘ + module +``` + +- **Destinations**: Type-safe navigation intents in shared navigation module +- **DestinationWithNoArgs**: Simple object destinations +- **DestinationWithArgs**: Parameterized destinations + +### Result-Based Navigation Pattern + +- **Activity Result API**: Integration for type-safe result handling +- **ResultEntry**: Generated interfaces for result-returning screens +- **Callbacks**: Strongly typed result callbacks with null handling for cancellation + +## State Management Pattern (Sample App) + +```kotlin +@HiltViewModel +class MyViewModel @Inject constructor() : ViewModel() { + private val _state = MutableStateFlow(MyState()) + val state: StateFlow = _state.asStateFlow() + + private val _sideEffects = Channel() + val sideEffects: Flow = _sideEffects.receiveAsFlow() +} +``` + +## Implementation Types Strategy + +- **ImplementationType.Composable**: Preferred for performance (avoids fragment overhead) +- **ImplementationType.Fragment**: Required when navigating FROM existing fragments +- **Gradual Migration**: Start with fragments, migrate to Composable over time + +## Code Generation Patterns + +- **Entry Factories**: Generated companion objects with `newInstance()` methods +- **Type Safety**: Compile-time verification of argument types between annotations and function parameters +- **Destination Resolution**: Cached multi-module destination lookup after first access + +## Testing Patterns + +- **Compilation Tests**: Verify generated code has correct interfaces and methods +- **Integration Tests**: Test actual navigation flows end-to-end +- **Type Validation**: Ensure argument type matching between annotations and function parameters + +## Performance Considerations + +- Use `ImplementationType.Composable` when possible +- Generated entry classes optimized for minimal runtime cost +- Multi-module destination resolution cached after first lookup diff --git a/.serena/memories/code_style_conventions.md b/.serena/memories/code_style_conventions.md new file mode 100644 index 0000000..58c14f2 --- /dev/null +++ b/.serena/memories/code_style_conventions.md @@ -0,0 +1,55 @@ +# Nibel Code Style & Conventions + +## Kotlin Style Guidelines + +- **Official Kotlin code style** configured in `gradle.properties` (`kotlin.code.style=official`) +- **4-space indentation** for all code +- **PascalCase** for classes and Composable functions +- **camelCase** for functions and variables +- **UPPER_SNAKE_CASE** for constants +- **Comprehensive KDoc** required for all public APIs + +## Android-Specific Conventions + +- **Composable functions**: PascalCase (e.g., `FirstScreen`, `PhotoPickerScreen`) +- **State management**: Use `collectAsStateWithLifecycle()` for observing state +- **Hilt integration**: `@HiltViewModel` and `hiltViewModel()` for dependency injection + +## Package Structure + +- `nibel.runtime` - Core runtime components +- `com.turo.nibel.sample.featureX` - Sample feature modules +- Feature-based module organization +- Clear separation of concerns (screens, ViewModels, navigation) + +## Annotation Patterns + +```kotlin +// Internal entry (single module) +@UiEntry(type = ImplementationType.Fragment) +@Composable +fun MyScreen() { ... } + +// External entry (multi-module) +@UiExternalEntry( + type = ImplementationType.Composable, + destination = MyScreenDestination::class +) +@Composable +fun MyScreen(args: MyScreenArgs) { ... } +``` + +## Naming Conventions + +- **Entry classes**: `{ComposableName}Entry` (auto-generated) +- **Destination classes**: `{Screen}Destination` +- **Args classes**: `{Screen}Args` +- **Result classes**: `{Screen}Result` +- **ViewModels**: `{Screen}ViewModel` (in sample app) + +## File Organization + +- Keep related functionality together in feature modules +- Prefer editing existing files over creating new ones +- Use symbolic tools for precise code modifications +- Follow existing patterns when adding new code diff --git a/.serena/memories/project_overview.md b/.serena/memories/project_overview.md new file mode 100644 index 0000000..e4efbdf --- /dev/null +++ b/.serena/memories/project_overview.md @@ -0,0 +1,30 @@ +# Nibel Project Overview + +## Purpose + +Nibel is a type-safe navigation library for seamless integration of Jetpack Compose in fragment-based Android apps. It enables gradual migration from Fragment-based Android apps to Jetpack Compose while maintaining compatibility with existing codebases. + +## Key Features + +- **Type-safe navigation** with compile-time checking between fragments and Compose screens +- **Multi-directional navigation**: fragment→compose, compose→compose, compose→fragment +- **Single-module and multi-module** navigation support +- **Result-based navigation** with Activity Result API pattern +- **Annotation processing** for code generation via KSP (Kotlin Symbol Processing) +- **Seamless integration** with existing fragment-based codebases + +## Navigation Scenarios Supported + +- fragment → compose (existing fragments to new Compose screens) +- compose → compose (between Compose screens) +- compose → fragment (Compose screens back to legacy fragments) + +## Main Components + +- **nibel-runtime**: Core runtime library with NavigationController +- **nibel-compiler**: KSP processor for generating entry classes +- **nibel-annotations**: Annotations (@UiEntry, @UiExternalEntry, etc.) +- **nibel-stub**: Stub classes for compilation +- **sample**: Multi-module sample application demonstrating usage patterns +- **tests**: Integration and compilation tests +- **build-tools**: Gradle convention plugins for consistent builds diff --git a/.serena/memories/project_structure.md b/.serena/memories/project_structure.md new file mode 100644 index 0000000..371f0ec --- /dev/null +++ b/.serena/memories/project_structure.md @@ -0,0 +1,92 @@ +# Nibel Project Structure + +## Root Directory Structure + +``` +nibel/ +├── nibel-annotations/ # Annotations for code generation +├── nibel-compiler/ # KSP processor for generating entry classes +├── nibel-runtime/ # Core runtime library with NavigationController +├── nibel-stub/ # Stub classes for compilation +├── tests/ # Integration and compilation tests +├── sample/ # Multi-module sample application +├── build-tools/ # Gradle convention plugins +├── config/ # Configuration files +├── docs/ # Documentation +└── .github/ # GitHub workflows and templates +``` + +## Core Modules + +### nibel-annotations/ + +- Contains `@UiEntry`, `@UiExternalEntry`, `@LegacyEntry` annotations +- Defines annotation parameters and validation rules +- Minimal dependencies, used at compile-time only + +### nibel-compiler/ + +- KSP (Kotlin Symbol Processing) implementation +- Generates entry classes, destination factories, result handlers +- Validates annotation usage and argument type matching +- Produces type-safe navigation boilerplate code + +### nibel-runtime/ + +- Core `NavigationController` implementation +- Entry interfaces (`ComposableEntry`, `FragmentEntry`, `ResultEntry`) +- Destination resolution and factory management +- Result-based navigation infrastructure + +### nibel-stub/ + +- Stub implementations for generated classes during compilation +- Ensures clean compilation before code generation completes + +## Sample Application Structure + +``` +sample/ +├── app/ # Main application module +├── navigation/ # Shared navigation destinations +├── feature-A/ # Independent feature module +├── feature-B/ # Independent feature module +├── feature-C/ # Independent feature module +└── common/ # Shared utilities and components +``` + +### Multi-Module Dependencies + +- Features depend on `navigation/` for shared destinations +- Features do NOT depend on each other directly +- `app/` module depends on all features for final assembly + +## Testing Structure + +``` +tests/src/test/kotlin/nibel/tests/ +├── codegen/ # Code generation tests +│ ├── InternalEntryCompileTest.kt +│ ├── ExternalEntryCompileTest.kt +│ ├── ResultEntryCompileTest.kt +│ └── TestArgs.kt +└── ExternalDestinationSearchTest.kt +``` + +## Build Tools Structure + +``` +build-tools/conventions/ +├── src/main/kotlin/ +│ ├── NibelAndroidCommonPlugin.kt +│ ├── NibelKotlinJvmLibraryPlugin.kt +│ └── SampleAndroidApplicationPlugin.kt +└── build.gradle.kts +``` + +## Configuration Files + +- `.pre-commit-config.yaml`: Pre-commit hooks configuration +- `gradle.properties`: Gradle and Kotlin configuration +- `settings.gradle.kts`: Project module configuration +- `CLAUDE.md`: Development guidelines and instructions diff --git a/.serena/memories/suggested_commands.md b/.serena/memories/suggested_commands.md new file mode 100644 index 0000000..587d3bd --- /dev/null +++ b/.serena/memories/suggested_commands.md @@ -0,0 +1,92 @@ +# Nibel Essential Commands + +## Development Workflow Commands + +```bash +# After making changes - run all verification (MOST IMPORTANT) +./gradlew check + +# Full build verification (used in CI) +./gradlew build + +# Clean build to avoid stale artifacts +./gradlew clean build +``` + +## Testing Commands + +```bash +# Run all unit tests +./gradlew test + +# Run specific test variants +./gradlew testDebugUnitTest +./gradlew testReleaseUnitTest + +# Run instrumentation tests (requires device) +./gradlew connectedAndroidTest +``` + +## Linting & Formatting Commands + +```bash +# Fix lint issues automatically (IMPORTANT) +./gradlew lintFix + +# Run Kotlin static analysis +./gradlew detekt + +# Run all Android lint checks +./gradlew lint +``` + +## Module-Specific Commands + +```bash +# Build specific modules +./gradlew :nibel-runtime:build +./gradlew :nibel-compiler:build +./gradlew :sample:app:build + +# Test changes locally (for library development) +./gradlew publishToMavenLocal --no-configuration-cache +``` + +## Pre-commit & Git Commands + +```bash +# Run pre-commit hooks manually +pre-commit run --all-files + +# Standard git commands (macOS/Darwin system) +git status +git add . +git commit -m "feat: description" +git push +``` + +## Documentation Commands + +```bash +# Generate documentation +./gradlew dokkaHtml +./gradlew dokkaGfm +``` + +## System Utilities (macOS/Darwin) + +```bash +# File operations +ls -la # List files with details +find . -name "*.kt" # Find Kotlin files +grep -r "pattern" . # Search in files +cd directory # Change directory + +# Development utilities +./gradlew tasks # List all available Gradle tasks +./gradlew help # Gradle help +``` + +## Critical Workflow + +**ALWAYS run `./gradlew check` before considering any task complete!** diff --git a/.serena/memories/task_completion_checklist.md b/.serena/memories/task_completion_checklist.md new file mode 100644 index 0000000..6443f8d --- /dev/null +++ b/.serena/memories/task_completion_checklist.md @@ -0,0 +1,51 @@ +# Task Completion Checklist + +## Mandatory Steps (MUST BE DONE) + +### Code Quality Verification ✅ + +1. **Run verification**: `./gradlew check` - Must pass completely +2. **Fix lint issues**: `./gradlew lintFix` - Apply automatic fixes +3. **Verify tests**: `./gradlew test` - All tests must pass +4. **Pre-commit hooks**: `pre-commit run --all-files` - Must pass + +### Build Verification ✅ + +5. **Full build**: `./gradlew build` - Complete build success required +6. **Sample app**: `./gradlew :sample:app:build` - Test integration in context +7. **Clean build**: `./gradlew clean build` - Ensure no stale artifacts + +## For Library Changes ✅ + +8. **Local publishing**: `./gradlew publishToMavenLocal` - Test library changes locally + +## Documentation & Testing (When Applicable) ✅ + +9. **Update KDoc** for new public APIs +10. **Add unit tests** for new functionality +11. **Add integration tests** for new navigation patterns +12. **Update README.md** if public interface changes + +## Git Best Practices ✅ + +13. **Conventional commits**: Use `feat:`, `fix:`, `docs:`, `refactor:` prefixes +14. **Branch hygiene**: Keep feature branches focused and small +15. **Rebase against main** before creating PR + +## Performance & Integration ✅ + +16. **Run detekt**: `./gradlew detekt` - Static analysis must pass +17. **Verify sample works**: Test actual navigation flows in sample app +18. **Check multi-module**: Ensure cross-module navigation still works + +## Critical Rule + +**NEVER consider a task complete without running `./gradlew check` successfully!** + +This ensures: + +- Code compiles across all modules +- All tests pass +- Linting rules are satisfied +- Generated code is valid +- Integration tests pass diff --git a/.serena/memories/tech_stack.md b/.serena/memories/tech_stack.md new file mode 100644 index 0000000..ce9ddf5 --- /dev/null +++ b/.serena/memories/tech_stack.md @@ -0,0 +1,39 @@ +# Nibel Tech Stack + +## Core Technologies + +- **Language**: Kotlin with official code style (configured in gradle.properties) +- **Platform**: Android +- **Build System**: Gradle with Kotlin DSL +- **Framework**: Jetpack Compose + Android Fragments +- **Code Generation**: Kotlin Symbol Processing (KSP) +- **Architecture**: Multi-module project structure + +## Sample App Technologies + +- **Dependency Injection**: Hilt +- **State Management**: StateFlow + Compose collectAsStateWithLifecycle() +- **Navigation**: Nibel library (obviously) + +## Testing Technologies + +- **Unit Testing**: JUnit +- **Assertions**: Kotest assertions +- **Integration Testing**: Custom compilation tests for KSP code generation +- **Test Structure**: Located in `tests/` module with compilation and integration tests + +## Build & Development Tools + +- **KSP**: For annotation processing and code generation +- **Detekt**: Kotlin static analysis and linting +- **Android Lint**: Android-specific linting with lintFix capability +- **Pre-commit hooks**: Automated formatting, linting, and commit message validation +- **Prettier**: Code formatting +- **Commitlint**: Conventional commit message validation +- **Dokka**: Documentation generation + +## Gradle Configuration + +- **Convention Plugins**: Custom plugins in `build-tools/conventions/` +- **Version Catalogs**: Using `libs.*` for dependency management +- **Performance Optimizations**: Parallel builds, configuration cache, build cache enabled diff --git a/.serena/project.yml b/.serena/project.yml new file mode 100644 index 0000000..a1155f9 --- /dev/null +++ b/.serena/project.yml @@ -0,0 +1,67 @@ +# language of the project (csharp, python, rust, java, typescript, go, cpp, or ruby) +# * For C, use cpp +# * For JavaScript, use typescript +# Special requirements: +# * csharp: Requires the presence of a .sln file in the project folder. +language: kotlin + +# whether to use the project's gitignore file to ignore files +# Added on 2025-04-07 +ignore_all_files_in_gitignore: true +# list of additional paths to ignore +# same syntax as gitignore, so you can use * and ** +# Was previously called `ignored_dirs`, please update your config if you are using that. +# Added (renamed) on 2025-04-07 +ignored_paths: [] + +# whether the project is in read-only mode +# If set to true, all editing tools will be disabled and attempts to use them will result in an error +# Added on 2025-04-18 +read_only: false + +# list of tool names to exclude. We recommend not excluding any tools, see the readme for more details. +# Below is the complete list of tools for convenience. +# To make sure you have the latest list of tools, and to view their descriptions, +# execute `uv run scripts/print_tool_overview.py`. +# +# * `activate_project`: Activates a project by name. +# * `check_onboarding_performed`: Checks whether project onboarding was already performed. +# * `create_text_file`: Creates/overwrites a file in the project directory. +# * `delete_lines`: Deletes a range of lines within a file. +# * `delete_memory`: Deletes a memory from Serena's project-specific memory store. +# * `execute_shell_command`: Executes a shell command. +# * `find_referencing_code_snippets`: Finds code snippets in which the symbol at the given location is referenced. +# * `find_referencing_symbols`: Finds symbols that reference the symbol at the given location (optionally filtered by type). +# * `find_symbol`: Performs a global (or local) search for symbols with/containing a given name/substring (optionally filtered by type). +# * `get_current_config`: Prints the current configuration of the agent, including the active and available projects, tools, contexts, and modes. +# * `get_symbols_overview`: Gets an overview of the top-level symbols defined in a given file. +# * `initial_instructions`: Gets the initial instructions for the current project. +# Should only be used in settings where the system prompt cannot be set, +# e.g. in clients you have no control over, like Claude Desktop. +# * `insert_after_symbol`: Inserts content after the end of the definition of a given symbol. +# * `insert_at_line`: Inserts content at a given line in a file. +# * `insert_before_symbol`: Inserts content before the beginning of the definition of a given symbol. +# * `list_dir`: Lists files and directories in the given directory (optionally with recursion). +# * `list_memories`: Lists memories in Serena's project-specific memory store. +# * `onboarding`: Performs onboarding (identifying the project structure and essential tasks, e.g. for testing or building). +# * `prepare_for_new_conversation`: Provides instructions for preparing for a new conversation (in order to continue with the necessary context). +# * `read_file`: Reads a file within the project directory. +# * `read_memory`: Reads the memory with the given name from Serena's project-specific memory store. +# * `remove_project`: Removes a project from the Serena configuration. +# * `replace_lines`: Replaces a range of lines within a file with new content. +# * `replace_symbol_body`: Replaces the full definition of a symbol. +# * `restart_language_server`: Restarts the language server, may be necessary when edits not through Serena happen. +# * `search_for_pattern`: Performs a search for a pattern in the project. +# * `summarize_changes`: Provides instructions for summarizing the changes made to the codebase. +# * `switch_modes`: Activates modes by providing a list of their names +# * `think_about_collected_information`: Thinking tool for pondering the completeness of collected information. +# * `think_about_task_adherence`: Thinking tool for determining whether the agent is still on track with the current task. +# * `think_about_whether_you_are_done`: Thinking tool for determining whether the task is truly completed. +# * `write_memory`: Writes a named memory (for future reference) to Serena's project-specific memory store. +excluded_tools: [] + +# initial prompt for the project. It will always be given to the LLM upon activating the project +# (contrary to the memories, which are loaded on demand). +initial_prompt: "" + +project_name: "nibel" diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..1196773 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,313 @@ +# Nibel - Type-Safe Navigation Library + +## About This Project + +**Nibel** is a type-safe navigation library for seamless integration of Jetpack Compose in fragment-based Android apps. It enables gradual migration from Fragment-based Android apps to Jetpack Compose while maintaining compatibility with existing codebases. + +### Navigation Scenarios Supported + +- **fragment → compose** - Navigate from existing fragments to Compose screens +- **compose → compose** - Navigate between Compose screens +- **compose → fragment** - Navigate from Compose screens back to fragments + +### Key Features + +- **Type-safe navigation** with compile-time checking +- **Single-module and multi-module** navigation support +- **Result-based navigation** with Activity Result API pattern +- **Annotation processing** for code generation via KSP +- **Seamless integration** with existing fragment-based codebases + +## Tech Stack + +- **Language**: Kotlin with official code style +- **Platform**: Android +- **Build System**: Gradle with Kotlin DSL +- **Framework**: Jetpack Compose + Android Fragments +- **Code Generation**: Kotlin Symbol Processing (KSP) +- **Architecture**: Multi-module project structure +- **Dependency Injection**: Hilt (in sample app) +- **Testing**: JUnit + Kotest assertions + +## Project Structure + +``` +nibel/ +├── nibel-annotations/ # Annotations for code generation (@UiEntry, @UiExternalEntry, etc.) +├── nibel-compiler/ # KSP processor for generating entry classes +├── nibel-runtime/ # Core runtime library with NavigationController +├── nibel-stub/ # Stub classes for compilation +├── tests/ # Integration and compilation tests +├── sample/ # Multi-module sample application +│ ├── app/ # Main sample application +│ ├── navigation/ # Shared navigation destinations +│ ├── feature-A/ # Sample feature module A +│ ├── feature-B/ # Sample feature module B +│ └── feature-C/ # Sample feature module C +└── build-tools/ # Gradle convention plugins for consistent build + └── conventions/ +``` + +## Essential Commands + +### Development Workflow + +```bash +# After making changes - run all verification +./gradlew check + +# Full build verification (used in CI) +./gradlew build + +# Fix lint issues automatically +./gradlew lintFix + +# Run pre-commit hooks manually +pre-commit run --all-files +``` + +### Testing + +```bash +# Run all unit tests +./gradlew test + +# Run specific test variants +./gradlew testDebugUnitTest +./gradlew testReleaseUnitTest + +# Run instrumentation tests (requires device) +./gradlew connectedAndroidTest +``` + +### Module-Specific Commands + +```bash +# Build specific modules +./gradlew :nibel-runtime:build +./gradlew :nibel-compiler:build +./gradlew :sample:app:build + +# Test changes locally +./gradlew publishToMavenLocal --no-configuration-cache +``` + +## Core Components + +### Annotations + +- **@UiEntry** - Mark composable functions as internal screen entries +- **@UiExternalEntry** - Mark composable functions as external (multi-module) screen entries +- **@LegacyEntry** / **@LegacyExternalEntry** - Apply to fragments for compose→fragment navigation + +### Navigation + +- **NavigationController** - Main navigation component for screen transitions +- **Entry classes** - Generated navigation entry points (e.g., `MyScreenEntry`) +- **Destinations** - Type-safe navigation targets for multi-module setups +- **Result API** - Type-safe result handling for screen interactions + +## Code Style Guidelines + +### Kotlin Conventions + +- **Official Kotlin code style** configured in `gradle.properties` +- **4-space indentation** for all code +- **PascalCase** for classes and Composable functions +- **camelCase** for functions and variables +- **UPPER_SNAKE_CASE** for constants +- **Comprehensive KDoc** for all public APIs + +### Package Structure + +- `nibel.runtime` - Core runtime components +- `com.turo.nibel.sample.featureX` - Sample feature modules +- Feature-based module organization +- Clear separation of concerns (screens, ViewModels, navigation) + +### Android-Specific + +- **Composable functions**: PascalCase (e.g., `FirstScreen`, `PhotoPickerScreen`) +- **State management**: Use `collectAsStateWithLifecycle()` for observing state +- **Hilt integration**: `@HiltViewModel` and `hiltViewModel()` for dependency injection + +## Development Patterns + +### Entry Definition Patterns + +```kotlin +// Internal entry (single module) +@UiEntry(type = ImplementationType.Fragment) +@Composable +fun MyScreen() { ... } + +// External entry (multi-module) +@UiExternalEntry( + type = ImplementationType.Composable, + destination = MyScreenDestination::class +) +@Composable +fun MyScreen(args: MyScreenArgs) { ... } + +// Result-returning screen +@UiEntry( + type = ImplementationType.Composable, + args = MyArgs::class, + result = MyResult::class +) +@Composable +fun MyScreen( + args: MyArgs, + navigator: NavigationController +) { ... } +``` + +### Navigation Controller Usage + +```kotlin +@Composable +fun MyScreen(navigator: NavigationController) { + // Navigate to another screen + navigator.navigateTo(TargetScreenEntry.newInstance(args)) + + // Navigate for result + navigator.navigateForResult( + entry = ResultScreenEntry.newInstance(args), + callback = { result: MyResult? -> + // Handle result (null if cancelled) + } + ) + + // Return result and navigate back + navigator.setResultAndNavigateBack(result) + navigator.cancelResultAndNavigateBack() +} +``` + +### Multi-Module Destinations + +```kotlin +// No args destination +object MyScreenDestination : DestinationWithNoArgs + +// With args destination +data class MyScreenDestination( + override val args: MyScreenArgs +) : DestinationWithArgs +``` + +### State Management Pattern (Sample App) + +```kotlin +@HiltViewModel +class MyViewModel @Inject constructor() : ViewModel() { + private val _state = MutableStateFlow(MyState()) + val state: StateFlow = _state.asStateFlow() + + private val _sideEffects = Channel() + val sideEffects: Flow = _sideEffects.receiveAsFlow() + + fun onAction() { + _state.value = _state.value.copy(/* updates */) + // or emit side effect + _sideEffects.trySend(MySideEffect.Navigate) + } +} +``` + +## Task Completion Checklist + +When completing development tasks, ensure you: + +### Code Quality ✅ + +1. **Run verification**: `./gradlew check` - Must pass +2. **Fix lint issues**: `./gradlew lintFix` - Apply automatic fixes +3. **Verify tests**: `./gradlew test` - All tests must pass +4. **Pre-commit hooks**: `pre-commit run --all-files` + +### Integration Testing ✅ + +5. **Build verification**: `./gradlew build` - Full build success +6. **Sample app**: `./gradlew :sample:app:build` - Test in context +7. **Local publishing**: `./gradlew publishToMavenLocal` (for library changes) +8. **Clean build**: `./gradlew clean build` - No stale artifacts + +### Documentation & Testing ✅ + +9. **Update KDoc** for new public APIs +10. **Add unit tests** for new functionality +11. **Add integration tests** for new navigation patterns +12. **Update README.md** if public interface changes + +### Git Best Practices ✅ + +13. **Conventional commits**: Use `feat:`, `fix:`, `docs:`, `refactor:` prefixes +14. **Branch hygiene**: Keep feature branches focused and small +15. **Rebase against main** before creating PR + +## Build Tool Configuration + +### Convention Plugins (in `build-tools/`) + +- `nibel.android.library` - Standard Android library setup +- `nibel.kotlin.jvm.library` - JVM-only Kotlin library setup +- `nibel.maven.publish` - Publishing configuration +- `nibel.android.compose` - Jetpack Compose configuration + +### Dependency Management + +- **Version catalogs**: Use `libs.*` for consistent dependency versions +- **Project references**: Use `projects.*` syntax (e.g., `projects.nibelRuntime`) +- **KSP processors**: Configure with `ksp()` and `kspTest()` dependencies + +## Testing Strategy + +### Compilation Tests (`tests/src/test/kotlin/nibel/tests/codegen/`) + +- Test code generation with various annotation combinations +- Verify generated classes have correct interfaces and methods +- Validate argument type matching between annotations and function parameters + +### Integration Tests + +- Test actual navigation flows end-to-end +- Test result-based navigation callback handling +- Test multi-module destination resolution +- Located alongside compilation tests + +## Performance Considerations + +### Gradle Configuration (`gradle.properties`) + +- **Parallel builds** enabled (`org.gradle.parallel=true`) +- **Configuration cache** enabled (`org.gradle.unsafe.configuration-cache=true`) +- **Build cache** enabled (`org.gradle.caching=true`) +- **Kotlin incremental compilation** enabled +- **Resource optimization** for Android builds + +### Navigation Performance + +- Use `ImplementationType.Composable` when possible (avoids fragment overhead) +- Generated entry classes are optimized for minimal runtime cost +- Multi-module destination resolution cached after first lookup + +## Debugging Tips + +### Common Issues + +1. **"Nibel not configured"**: Ensure `Nibel.configure()` is called in `Application.onCreate()` +2. **Missing entry factory**: Destination not associated with `@UiExternalEntry` +3. **Compilation errors**: Check argument types match between annotation and function parameters +4. **Navigation not working**: Verify correct `ImplementationType` for your navigation scenario + +### Development Tools + +- **Android lint** catches common navigation issues +- **KSP error messages** provide clear guidance for annotation problems +- **Sample app** demonstrates all navigation patterns +- **Pre-commit hooks** catch formatting and basic issues early + +--- + +**Important**: Always run `./gradlew check` before considering any task complete. This ensures code quality, tests pass, and the project builds correctly across all modules. diff --git a/docs/prd-navigate-for-result.md b/docs/prd-navigate-for-result.md new file mode 100644 index 0000000..4dd5636 --- /dev/null +++ b/docs/prd-navigate-for-result.md @@ -0,0 +1,912 @@ +# Navigate-For-Result: Type-Safe Result Handling + +**Priority**: High +**Summary**: Add type-safe result handling to Nibel's navigation system, enabling screens to return data to their callers with compile-time validation +**Jira**: TBD +**Branch**: `f/result-navigation-feature` +**Status**: PLANNING + +## Subagents + +TBD - Will coordinate with technical experts for: + +- Android Architecture Review (lifecycle and process death handling) +- API Design Review (type safety and ergonomics) +- Code Generation Review (KSP implementation) +- Testing Strategy Review (coverage and test scenarios) + +## Description + +This feature adds native support for navigate-for-result patterns in Nibel, allowing screens to return typed data to their callers with compile-time validation. Similar to how arguments flow into screens (via the `args` parameter), this feature enables data to flow back out through results. + +The implementation follows Android's established Activity Result API pattern, providing a familiar developer experience while leveraging Nibel's existing type-safety guarantees through KSP code generation. + +## Details + +### Problem Statement + +Modern Android applications frequently need screens to return data to their callers, including: + +- **User selection flows**: Photo picker, contact selector, location picker +- **Form submissions**: Collecting user input across screens +- **Decision dialogs**: Approval/rejection, option selection +- **Data creation**: Creating items and returning their IDs +- **Multi-step workflows**: Gathering information across multiple screens + +Currently, developers must use workarounds: + +- **Shared ViewModels**: Couples modules, breaks encapsulation, doesn't scale across feature boundaries +- **Event buses**: Lacks type safety, difficult to track data flow +- **Navigation arguments in reverse**: Complex, error-prone, feels unnatural +- **Direct callback passing**: Doesn't survive process death or configuration changes + +These approaches are either unsafe, unmaintainable, or don't fit well with Nibel's multi-module architecture. + +### Goals + +1. **Type Safety**: Compile-time validation that result types match between caller and callee +2. **Zero Breaking Changes**: Existing code works without modifications +3. **Multi-Module Support**: Works seamlessly across feature module boundaries +4. **Consistency**: Follows Nibel's existing design patterns (similar to args handling) +5. **Opt-in**: Feature is optional, not required for simple navigation +6. **Android Integration**: Leverages Activity Result API pattern for familiar developer experience + +### Non-Goals + +1. **Process death recovery for Composable results**: Initial version won't support result delivery after process death for Composable entries (Fragment entries will support this via Fragment Result API) +2. **Activity Result Contract integration**: Won't provide wrappers for system Activity Result Contracts in initial release +3. **Streaming results**: Won't support multiple result deliveries (Flow-based results) +4. **Result validation framework**: No built-in validation beyond type checking + +### User Stories + +#### As a feature developer, I want to... + +1. **Create a photo picker screen that returns selected photos** + + - Define a result type with selected photo URIs + - Navigate to the picker and receive results + - Handle both successful selection and cancellation + +2. **Build a multi-step form that returns completed data** + + - Create multiple screens that collect information + - Return aggregate data when user completes the flow + - Handle cancellation at any step + +3. **Show a confirmation dialog and receive the user's decision** + + - Display a dialog asking for confirmation + - Return true/false based on user choice + - Distinguish between explicit "no" and cancellation + +4. **Navigate across feature modules with results** + + - Call screens in other feature modules + - Receive typed results back across module boundaries + - Maintain type safety without tight coupling + +5. **Create a user selector that returns selected user data** + - Display a list of users + - Return the selected user's information + - Handle case where no user is selected + +### Success Metrics + +#### Development Experience + +- **Compilation time**: No significant increase (< 5% impact) +- **Generated code size**: Minimal increase for entries with results +- **API discoverability**: Developers can find and use the API without extensive documentation + +#### Code Quality + +- **Type safety**: 100% compile-time validation of result types +- **Test coverage**: 90%+ coverage for new code +- **Backwards compatibility**: Zero breaking changes to existing code + +#### Adoption + +- **Migration effort**: < 30 minutes to add results to an existing screen +- **Documentation clarity**: Developers understand the feature from examples +- **Error messages**: Clear, actionable error messages when types mismatch + +## Functional Requirements + +### FR-1: Annotation Support for Results + +#### FR-1.1: @UiEntry Result Parameter + +- **MUST** add optional `result` parameter to `@UiEntry` annotation +- **MUST** default to `NoResult::class` for backwards compatibility +- **MUST** accept any `Parcelable` type as result +- **MUST** validate result type is Parcelable at compile-time + +#### FR-1.2: @UiExternalEntry Result Parameter + +- **MUST** add optional `result` parameter to `@UiExternalEntry` annotation +- **MUST** default to `NoResult::class` for backwards compatibility +- **MUST** work with multi-module destinations +- **MUST** support both `DestinationWithArgs` and `DestinationWithNoArgs` + +#### FR-1.3: NoResult Marker Class + +- **MUST** create `NoResult` marker object that implements `Parcelable` +- **MUST** use `NoResult` as default for result parameter +- **MUST** be located in `nibel-annotations` module + +### FR-2: NavigationController API + +#### FR-2.1: Navigate For Result + +- **MUST** add `navigateForResult()` method accepting an Entry +- **MUST** add `navigateForResult()` method accepting an ExternalDestination +- **MUST** require a callback parameter of type `(TResult?) -> Unit` +- **MUST** support both Fragment and Composable implementation types +- **MUST** provide type-safe callback with correct result type + +#### FR-2.2: Set Result and Navigate Back + +- **MUST** add `setResultAndNavigateBack()` method accepting result +- **MUST** validate result type matches entry's result type +- **MUST** deliver result to stored callback +- **MUST** automatically navigate back after setting result +- **MUST** clean up callback storage after delivery + +#### FR-2.3: Cancel Result and Navigate Back + +- **MUST** add `cancelResultAndNavigateBack()` method with no parameters +- **MUST** deliver `null` to callback when cancelled +- **MUST** automatically navigate back after cancellation +- **MUST** clean up callback storage after cancellation + +### FR-3: Code Generation + +#### FR-3.1: Result Entry Interface + +- **MUST** generate `ResultEntry` interface implementation +- **MUST** include `resultType: Class` property +- **MUST** extend appropriate base entry type (ComposableEntry or FragmentEntry) +- **MUST** maintain all existing entry functionality + +#### FR-3.2: Entry Factory Updates + +- **MUST** update companion object factory methods for result entries +- **MUST** return correct type (ResultEntry vs Entry) from newInstance() +- **MUST** initialize result-specific properties +- **MUST** maintain backwards compatibility for non-result entries + +#### FR-3.3: External Entry Factory Support + +- **MUST** update EntryFactoryProvider for result-aware destinations +- **MUST** associate destinations with result types +- **MUST** generate factory methods that handle results +- **MUST** support DestinationWithResult interface + +### FR-4: Multi-Module Support + +#### FR-4.1: Destination Result Interface + +- **MUST** create `DestinationWithResult` interface +- **MUST** include `resultType: Class` property +- **MUST** allow destinations to declare result types +- **MUST** work with both args and no-args destinations + +#### FR-4.2: Cross-Module Result Delivery + +- **MUST** deliver results across module boundaries +- **MUST** maintain type safety across modules +- **MUST** not require tight coupling between modules +- **MUST** support runtime destination resolution + +### FR-5: Lifecycle and State Management + +#### FR-5.1: Fragment Result Handling + +- **MUST** use Fragment Result API for Fragment implementation types +- **MUST** survive process death for Fragment results +- **MUST** survive configuration changes for Fragment results +- **MUST** clean up listeners when fragments are destroyed + +#### FR-5.2: Composable Result Handling + +- **MUST** store callbacks in NavigationController for Composables +- **MUST** survive configuration changes when NavigationController is ViewModel-scoped +- **SHOULD** document that Composable results don't survive process death +- **MUST** clean up callbacks when results are delivered + +#### FR-5.3: Callback Management + +- **MUST** generate unique request keys for each navigation +- **MUST** store callbacks with request keys +- **MUST** deliver results using request keys +- **MUST** remove callbacks after delivery or cancellation +- **MUST** prevent memory leaks from stored callbacks + +## Technical Requirements + +### TR-1: Type Safety + +#### TR-1.1: Compile-Time Validation + +- **MUST** validate result types are Parcelable at compile-time +- **MUST** validate result type in annotation matches function signature +- **MUST** generate compile errors for type mismatches +- **MUST** provide clear error messages for type issues + +#### TR-1.2: Runtime Type Safety + +- **MUST** maintain type safety at runtime through generics +- **MUST** store result types in generated classes +- **MUST** enable runtime type checking if needed +- **SHOULD** avoid unchecked casts in generated code + +### TR-2: Backwards Compatibility + +#### TR-2.1: API Compatibility + +- **MUST** maintain 100% backwards compatibility +- **MUST NOT** modify existing annotation parameters +- **MUST NOT** change existing method signatures +- **MUST NOT** deprecate any existing APIs +- **MUST NOT** require changes to existing code + +#### TR-2.2: Generated Code Compatibility + +- **MUST** generate identical code for entries without results +- **MUST** maintain existing entry class hierarchies +- **MUST** preserve existing factory method signatures +- **MUST** not break binary compatibility + +### TR-3: Performance + +#### TR-3.1: Compilation Performance + +- **MUST** add < 5% to KSP processing time +- **MUST** not significantly increase generated code size +- **SHOULD** optimize callback storage data structures +- **SHOULD** avoid reflection where possible + +#### TR-3.2: Runtime Performance + +- **MUST** add minimal overhead to navigation operations +- **MUST** use efficient callback storage (O(1) lookup) +- **MUST** clean up callbacks promptly to avoid memory leaks +- **SHOULD** minimize allocations in hot paths + +### TR-4: Testing + +#### TR-4.1: Compilation Tests + +- **MUST** test all annotation combinations with results +- **MUST** verify generated code structure and interfaces +- **MUST** validate type safety in generated code +- **MUST** test both internal and external entries + +#### TR-4.2: Integration Tests + +- **MUST** test result delivery end-to-end +- **MUST** test cancellation handling +- **MUST** test multi-module result navigation +- **MUST** test both Fragment and Composable types +- **MUST** test configuration change scenarios + +#### TR-4.3: Sample App Demonstrations + +- **MUST** add photo picker example to sample app +- **MUST** add confirmation dialog example +- **MUST** demonstrate multi-module results +- **MUST** show both implementation types + +## API Design + +### Annotation API + +```kotlin +// Add result parameter to @UiEntry +@UiEntry( + type = ImplementationType.Composable, + args = PhotoPickerArgs::class, + result = PhotoPickerResult::class // NEW +) +@Composable +fun PhotoPickerScreen( + args: PhotoPickerArgs, + navigator: NavigationController +) + +// Add result parameter to @UiExternalEntry +@UiExternalEntry( + type = ImplementationType.Fragment, + destination = PhotoPickerDestination::class, + result = PhotoPickerResult::class // NEW +) +@Composable +fun PhotoPickerScreen( + args: PhotoPickerArgs, + navigator: NavigationController +) + +// NoResult marker (default) +@Parcelize +object NoResult : Parcelable +``` + +### NavigationController API + +```kotlin +abstract class NavigationController { + + // NEW: Navigate for result + abstract fun navigateForResult( + entry: Entry, + fragmentSpec: FragmentSpec<*> = this.fragmentSpec, + composeSpec: ComposeSpec<*> = this.composeSpec, + callback: (TResult?) -> Unit + ) + + // NEW: Navigate to external destination for result + abstract fun navigateForResult( + externalDestination: ExternalDestination, + fragmentSpec: FragmentSpec<*> = this.fragmentSpec, + composeSpec: ComposeSpec<*> = this.composeSpec, + callback: (TResult?) -> Unit + ) + + // NEW: Set result and go back + abstract fun setResultAndNavigateBack(result: TResult) + + // NEW: Cancel and go back (delivers null) + abstract fun cancelResultAndNavigateBack() +} +``` + +### Generated Code Interface + +```kotlin +// NEW: Interface for entries with results +interface ResultEntry : Entry { + val resultType: Class +} + +// NEW: Interface for destinations with results +interface DestinationWithResult { + val resultType: Class +} +``` + +### Usage Examples + +#### Example 1: Photo Picker (Internal Entry) + +```kotlin +// Define result type +@Parcelize +data class PhotoPickerResult(val photoUri: String) : Parcelable + +// Define screen with result +@UiEntry( + type = ImplementationType.Composable, + result = PhotoPickerResult::class +) +@Composable +fun PhotoPickerScreen(navigator: NavigationController) { + Button(onClick = { + navigator.setResultAndNavigateBack( + PhotoPickerResult("content://photo/123") + ) + }) { + Text("Select Photo") + } +} + +// Call from another screen +Button(onClick = { + navigator.navigateForResult( + entry = PhotoPickerScreenEntry.newInstance() + ) { result: PhotoPickerResult? -> + result?.let { updatePhoto(it.photoUri) } + } +}) { + Text("Change Photo") +} +``` + +#### Example 2: Confirmation Dialog (Fragment Type) + +```kotlin +@Parcelize +data class ConfirmationResult(val confirmed: Boolean) : Parcelable + +@UiEntry( + type = ImplementationType.Fragment, + args = ConfirmationArgs::class, + result = ConfirmationResult::class +) +@Composable +fun ConfirmationDialog( + args: ConfirmationArgs, + navigator: NavigationController +) { + AlertDialog( + onDismissRequest = { navigator.cancelResultAndNavigateBack() }, + confirmButton = { + Button(onClick = { + navigator.setResultAndNavigateBack( + ConfirmationResult(confirmed = true) + ) + }) { + Text("Confirm") + } + }, + dismissButton = { + Button(onClick = { + navigator.setResultAndNavigateBack( + ConfirmationResult(confirmed = false) + ) + }) { + Text("Cancel") + } + }, + text = { Text(args.message) } + ) +} +``` + +#### Example 3: Multi-Module Result + +```kotlin +// navigation module: Define destination +data class PhotoPickerDestination( + override val args: NoArgs +) : DestinationWithNoArgs, + DestinationWithResult + +// feature-photo module: Implement screen +@UiExternalEntry( + type = ImplementationType.Composable, + destination = PhotoPickerDestination::class, + result = PhotoPickerResult::class +) +@Composable +fun PhotoPickerScreen(navigator: NavigationController) { ... } + +// feature-profile module: Call with result +navigator.navigateForResult(PhotoPickerDestination) { result: PhotoPickerResult? -> + result?.let { updateProfilePhoto(it.photoUri) } +} +``` + +## Questions + +### Architecture & Design + +- [ ] Should we support result delivery after process death for Composable entries? + + - [ ] **Recommended**: Start without support, document limitation + - Most use cases don't require process death recovery + - Fragment entries already support this via Fragment Result API + - Can add SavedStateHandle support in future version if needed + - Reduces initial implementation complexity + - Risk: Some apps may need this for critical flows + - [ ] Alternative: Implement SavedStateHandle support from the start + - Provides consistent behavior across implementation types + - More complex implementation and testing + - Risk: Delayed feature delivery, increased maintenance burden + +- [ ] How should we handle nested navigate-for-result (Screen A → Screen B for result → Screen C for result)? + + - [ ] **Recommended**: Support naturally with callback stacking + - Each screen maintains its own result callback + - Natural behavior developers would expect + - No special handling required + - Risk: Potential confusion with multiple callbacks in flight + - [ ] Alternative: Disallow nested results with runtime error + - Simpler mental model + - Prevents potential issues + - Risk: Limits legitimate use cases + +- [ ] Should we add runtime type validation in addition to compile-time checks? + - [ ] **Recommended**: Compile-time only (current proposal) + - Type safety guaranteed by KSP code generation + - Better performance (no runtime overhead) + - Sufficient for correct usage + - Risk: No safety net if generated code has bugs + - [ ] Alternative: Add runtime type checking with exceptions + - Additional safety layer + - Helpful for debugging + - Risk: Performance overhead, false sense of security + +### Multi-Module Support + +- [ ] Should destinations be required to implement DestinationWithResult interface? + - [ ] **Recommended**: Yes, require interface implementation + - Compile-time validation of result types + - Clear declaration of destination capabilities + - Type-safe across module boundaries + - Risk: Slightly more boilerplate in destination definitions + - [ ] Alternative: Use runtime result type registration + - Less boilerplate + - No interface requirements + - Risk: Runtime errors, not type-safe, harder to discover + +### API Ergonomics + +- [ ] Should we provide a way to differentiate explicit cancellation from null results? + + - [ ] **Recommended**: Use nullable results (null = cancelled) + - Simple, consistent with Android Activity Result API + - One callback to implement + - Common pattern developers understand + - Risk: Ambiguous if result type can legitimately be "no data" + - [ ] Alternative: Use sealed class Result (Success/Cancelled) + - Explicit success/failure states + - No ambiguity + - Risk: Verbose, more boilerplate, unfamiliar pattern + +- [ ] Should navigateForResult be a separate method or overload of navigateTo? + - [ ] **Recommended**: Separate method (current proposal) + - Clear intent when reading code + - Easier to discover in IDE + - No confusion with optional callback parameter + - Risk: One more method to learn + - [ ] Alternative: Add optional callback parameter to navigateTo + - Single navigation method + - Fewer methods to learn + - Risk: Ambiguous when callback is omitted, overloading complexity + +### Testing & Validation + +- [ ] How should we handle configuration changes for Composable results? + - [ ] **Recommended**: Require ViewModel-scoped NavigationController + - Survives configuration changes + - Consistent with modern Android architecture + - Document this requirement clearly + - Risk: Developers might not follow best practice + - [ ] Alternative: Implement automatic callback restoration + - More robust + - Works regardless of NavigationController scope + - Risk: Complex implementation, potential memory leaks + +### Future Enhancements + +- [ ] Should we plan for Activity Result Contract integration? + + - [ ] **Recommended**: Add in future version based on demand + - Initial release focuses on core navigate-for-result + - Developers can call Activity Result API directly + - Evaluate real-world use cases before designing API + - Risk: Developers create their own wrappers + - [ ] Alternative: Include in initial release + - Complete solution from day one + - Consistent API for all result scenarios + - Risk: Delayed release, increased complexity + +- [ ] Should we support streaming results (Flow-based)? + - [ ] **Recommended**: Not in initial release + - Use SharedViewModel or event bus for complex scenarios + - Collect real-world use cases first + - Simpler initial implementation + - Risk: Developers may create workarounds + - [ ] Alternative: Add resultFlow variant + - Supports progress updates, multi-select + - More complete solution + - Risk: Significant complexity increase + +## Implementation Plan + +### Phase 1: Annotations & Runtime Foundation (3 days) + +- [ ] Add `result` parameter to `@UiEntry` annotation with default `NoResult::class` +- [ ] Add `result` parameter to `@UiExternalEntry` annotation with default `NoResult::class` +- [ ] Create `NoResult` marker class in `nibel-annotations` module +- [ ] Update annotation KDoc with result examples +- [ ] Create `ResultEntry` interface in `nibel-runtime` +- [ ] Create `DestinationWithResult` interface in `nibel-runtime` +- [ ] **Testing**: Verify annotations compile, default parameter works + +### Phase 2: NavigationController API (5 days) + +- [ ] Add `navigateForResult(entry, callback)` method to NavigationController +- [ ] Add `navigateForResult(externalDestination, callback)` method to NavigationController +- [ ] Add `setResultAndNavigateBack(result)` method to NavigationController +- [ ] Add `cancelResultAndNavigateBack()` method to NavigationController +- [ ] Implement callback storage mechanism in NibelNavigationController +- [ ] Implement result delivery for Fragment entries using Fragment Result API +- [ ] Implement result delivery for Composable entries using callback storage +- [ ] Add request key generation and management +- [ ] Implement callback cleanup after delivery/cancellation +- [ ] **Testing**: Unit tests for result delivery mechanism, mock navigation scenarios + +### Phase 3: Compiler Code Generation (7 days) + +- [ ] Update `EntryMetadata` to include `resultQualifiedName` field +- [ ] Modify `EntryGeneratingVisitor` to extract result type from annotations +- [ ] Add validation that result types are Parcelable +- [ ] Update `ComposableGenerator` to generate ResultEntry implementations +- [ ] Add `resultType` property to generated Composable entries with results +- [ ] Update companion object factory methods to return ResultEntry type +- [ ] Update `FragmentGenerator` to generate ResultEntry implementations for Fragments +- [ ] Add `resultType` property to generated Fragment entries with results +- [ ] Modify `EntryFactoryProviderGenerator` for external entries with results +- [ ] Generate destination extensions for result type access +- [ ] **Testing**: Compilation tests for all scenarios, verify generated code structure + +### Phase 4: Integration Testing & Sample App (7 days) + +- [ ] Create `ResultEntryCompileTest.kt` with all annotation combinations +- [ ] Verify generated code implements ResultEntry interface correctly +- [ ] Create `ResultNavigationTest.kt` for end-to-end testing +- [ ] Test result delivery for Composable entries +- [ ] Test result delivery for Fragment entries +- [ ] Test cancellation handling (null results) +- [ ] Test multi-module result navigation +- [ ] Test configuration change scenarios +- [ ] Add PhotoPickerScreen example to sample app (feature-A) +- [ ] Add ProfileScreen caller example to sample app (feature-B) +- [ ] Add ConfirmationDialog example to sample app +- [ ] Run full regression test suite (`./gradlew test`) +- [ ] Run full build verification (`./gradlew build`) +- [ ] Verify sample app builds and runs correctly +- [ ] **Testing**: All tests pass, sample app demonstrates all scenarios + +### Phase 5: Documentation & Release Preparation (3 days) + +- [ ] Update main README.md with navigate-for-result section +- [ ] Add comprehensive KDoc to all new public APIs +- [ ] Create usage examples document with common patterns +- [ ] Write migration guide for adding results to existing screens +- [ ] Document lifecycle considerations and limitations +- [ ] Document process death behavior differences (Fragment vs Composable) +- [ ] Update CHANGELOG with new feature +- [ ] Create release notes +- [ ] Final code review and polish +- [ ] Run pre-commit hooks and linting (`./gradlew lintFix`, `pre-commit run --all-files`) +- [ ] **Testing**: Documentation review, final QA pass + +### Estimated Timeline + +- **Total Duration**: 25 days (~5 weeks) +- **Phase 1**: Days 1-3 +- **Phase 2**: Days 3-8 +- **Phase 3**: Days 8-15 +- **Phase 4**: Days 15-22 +- **Phase 5**: Days 22-25 + +### Milestones + +- **M1** (Day 3): Annotations and interfaces complete +- **M2** (Day 8): NavigationController API implementation complete +- **M3** (Day 15): Code generation complete and tested +- **M4** (Day 22): Sample app and integration tests complete +- **M5** (Day 25): Documentation complete, ready for release + +## Testing Strategy + +### Compilation Tests + +**Location**: `tests/src/test/kotlin/nibel/tests/codegen/ResultEntryCompileTest.kt` + +Test scenarios: + +- [ ] Internal entry (Composable) with result +- [ ] Internal entry (Fragment) with result +- [ ] External entry (Composable) with result +- [ ] External entry (Fragment) with result +- [ ] Entry with result but no args +- [ ] Entry with both args and result +- [ ] Verify generated code implements ResultEntry interface +- [ ] Verify resultType property is correct +- [ ] Verify companion object signatures + +### Integration Tests + +**Location**: `tests/src/test/kotlin/nibel/tests/integration/ResultNavigationTest.kt` + +Test scenarios: + +- [ ] Navigate for result delivers result correctly +- [ ] Navigate for result handles cancellation (null) +- [ ] Multi-module result navigation works +- [ ] Fragment implementation type delivers results +- [ ] Composable implementation type delivers results +- [ ] Configuration changes preserve callbacks +- [ ] Nested navigate-for-result works +- [ ] Multiple results in sequence work correctly + +### Sample App Examples + +**Location**: `sample/` modules + +Examples to implement: + +- [ ] PhotoPickerScreen (feature-A) - demonstrates Composable result entry +- [ ] ProfileScreen (feature-B) - demonstrates calling with results +- [ ] ConfirmationDialog - demonstrates Fragment result entry +- [ ] Multi-module flow - demonstrates external entries with results + +### Test Coverage Goals + +- **Unit tests**: 90%+ coverage for new code +- **Compilation tests**: 100% coverage of annotation combinations +- **Integration tests**: All navigation flows with results covered +- **Regression tests**: All existing tests continue to pass + +## Release Plan + +### Version + +**Target**: Nibel 2.1.0 (minor version, new feature) + +### Pre-Release Checklist + +- [ ] All tests passing (`./gradlew test`) +- [ ] Full build successful (`./gradlew build`) +- [ ] Sample app builds and runs +- [ ] Documentation complete and reviewed +- [ ] CHANGELOG updated +- [ ] Migration guide written +- [ ] No breaking changes confirmed + +### Release Phases + +**Phase 1: Internal Alpha (Week 1)** + +- Release to internal team for testing +- Gather feedback on API ergonomics +- Test in real-world scenarios +- Fix critical issues + +**Phase 2: Beta Release (Week 2)** + +- Release to select early adopters +- Gather broader feedback +- Monitor for edge cases +- Refine documentation based on questions + +**Phase 3: General Availability (Week 3)** + +- Release to all users +- Announce in release notes +- Update website documentation +- Monitor adoption and issues + +### Rollback Plan + +- Feature is opt-in, no breaking changes +- If critical issues found, can: + 1. Document workarounds + 2. Fix in patch release (2.1.1) + 3. Deprecate in future major version if necessary +- Zero impact on users not adopting the feature + +### Communication Plan + +- **Announcement**: Blog post with examples and use cases +- **Documentation**: Update README and add migration guide +- **Sample Code**: Add examples to sample app +- **Support**: Monitor GitHub issues for questions + +### Success Criteria for Release + +- [ ] Zero breaking changes confirmed +- [ ] All tests passing +- [ ] Documentation complete +- [ ] Sample app demonstrates all scenarios +- [ ] Performance benchmarks meet targets +- [ ] No critical issues in beta testing + +--- + +## Appendix: Technical Details + +### Generated Code Example + +**Input annotation**: + +```kotlin +@UiEntry( + type = ImplementationType.Composable, + args = PhotoPickerArgs::class, + result = PhotoPickerResult::class +) +@Composable +fun PhotoPickerScreen(args: PhotoPickerArgs, navigator: NavigationController) +``` + +**Generated entry class**: + +```kotlin +@Parcelize +class PhotoPickerScreenEntry( + override val args: PhotoPickerArgs, + override val name: String, + internal var requestKey: String? = null, +) : ComposableEntry(args, name), + ResultEntry { + + override val resultType: Class + get() = PhotoPickerResult::class.java + + @Composable + override fun ComposableContent() { + PhotoPickerScreen( + args = LocalArgs.current as PhotoPickerArgs, + navigator = LocalNavigationController.current + ) + } + + companion object { + fun newInstance(args: PhotoPickerArgs): ResultEntry { + return PhotoPickerScreenEntry( + args = args, + name = buildRouteName(PhotoPickerScreenEntry::class.qualifiedName!!, args), + ) + } + } +} +``` + +### Module Changes Summary + +**nibel-annotations**: + +- Add `result` parameter to `@UiEntry` +- Add `result` parameter to `@UiExternalEntry` +- Add `NoResult` marker class + +**nibel-runtime**: + +- Add `ResultEntry` interface +- Add `DestinationWithResult` interface +- Add `navigateForResult()` methods to NavigationController +- Add `setResultAndNavigateBack()` method +- Add `cancelResultAndNavigateBack()` method +- Implement result delivery in NibelNavigationController + +**nibel-compiler**: + +- Update EntryMetadata with result type +- Update EntryGeneratingVisitor to extract results +- Update ComposableGenerator for ResultEntry +- Update FragmentGenerator for ResultEntry +- Update EntryFactoryProviderGenerator for external results + +**tests**: + +- Add ResultEntryCompileTest.kt +- Add ResultNavigationTest.kt +- Add regression tests + +**sample**: + +- Add PhotoPickerScreen example +- Add ProfileScreen with result handling +- Add ConfirmationDialog example + +### Backwards Compatibility Guarantee + +This feature guarantees 100% backwards compatibility: + +1. **No breaking changes**: All existing code compiles without modification +2. **Opt-in feature**: Default behavior is unchanged (result = NoResult::class) +3. **Generated code**: Entries without results generate identical code +4. **API additions only**: No existing APIs modified or deprecated +5. **Binary compatibility**: No changes to existing class hierarchies + +### Risk Mitigation + +**Risk**: Complexity of result delivery across Fragment/Composable boundaries +**Mitigation**: Use established Android APIs (Fragment Result API), comprehensive testing + +**Risk**: Memory leaks from stored callbacks +**Mitigation**: Automatic cleanup after delivery, lifecycle-aware storage, leak detection in tests + +**Risk**: Type safety edge cases +**Mitigation**: Compile-time validation via KSP, runtime type information in generated classes + +**Risk**: Process death handling inconsistency +**Mitigation**: Clear documentation, use Fragment Result API for critical flows + +**Risk**: Developer confusion about API usage +**Mitigation**: Comprehensive examples, clear documentation, IDE-friendly API design diff --git a/docs/rfcs/001-navigate-for-result/feedback.md b/docs/rfcs/001-navigate-for-result/feedback.md new file mode 100644 index 0000000..f635961 --- /dev/null +++ b/docs/rfcs/001-navigate-for-result/feedback.md @@ -0,0 +1,148 @@ +# RFC 001 Feedback: Navigate-For-Result Feature + +## Review Information + +| | | +| ----------------- | ------------------------------------------------------------------ | +| **RFC Number** | 001 | +| **RFC Title** | Navigate-For-Result: Type-Safe Result Handling in Nibel Navigation | +| **Review Period** | TBD | +| **Status** | Awaiting Review 📝 | + +## Instructions for Reviewers + +Please review the [RFC document](./rfc.md) and provide feedback in this file. Focus on: + +1. **Architecture & Design**: Is the proposed solution sound? Are there better alternatives? +2. **API Design**: Is the API intuitive and consistent with Nibel's existing patterns? +3. **Backwards Compatibility**: Are there any hidden breaking changes? +4. **Implementation Complexity**: Is the implementation plan realistic? +5. **Testing Strategy**: Is the testing approach comprehensive enough? +6. **Documentation**: Is the RFC clear and complete? +7. **Edge Cases**: Are there scenarios not covered by the RFC? + +## Feedback Template + +### [Reviewer Name] - [Date] + +**Overall Assessment**: [Approve / Request Changes / Reject] + +**Summary**: +[Brief overall feedback] + +**Detailed Comments**: + +#### Section: [Section Name] + +- **Comment**: [Your comment] +- **Severity**: [Critical / Major / Minor / Question] +- **Suggestion**: [Your suggestion, if applicable] + +--- + +## Feedback Received + +### Awaiting Reviews + +The following team members are invited to review: + +- [ ] Tech Lead / Architect +- [ ] Android Team Lead +- [ ] Backend Team (if multi-module concerns) +- [ ] Nibel Maintainers +- [ ] Developer Experience Team + +--- + +## Review Status + +### Review Checklist + +- [ ] **Architecture Review**: Approved by senior architect +- [ ] **API Design Review**: Approved by API design team +- [ ] **Security Review**: No security concerns identified +- [ ] **Performance Review**: No performance concerns identified +- [ ] **Testing Review**: Testing strategy is comprehensive +- [ ] **Documentation Review**: Documentation is complete and clear +- [ ] **Backwards Compatibility**: Verified no breaking changes + +### Open Issues + +Track any issues or questions that arise during review: + +#### Issue #1: [Title] + +**Status**: Open / Resolved / Deferred +**Raised By**: [Name] +**Description**: +[Description of the issue] + +**Resolution**: +[How this was resolved, if applicable] + +--- + +## Decision + +**Status**: Pending ⏳ + +Once all reviews are complete and issues resolved, update with one of: + +- ✅ **Approved**: RFC is approved for implementation +- ⚠️ **Approved with Changes**: RFC is approved with minor modifications +- 🔄 **Revision Required**: RFC needs significant changes before approval +- ❌ **Rejected**: RFC is not approved + +**Final Decision By**: [Name] +**Date**: [Date] +**Notes**: +[Any final notes or conditions] + +--- + +## Post-Review Actions + +After approval: + +1. [ ] Create implementation tracking issue +2. [ ] Update RFC status to "Approved" +3. [ ] Notify engineering team +4. [ ] Set up project board for implementation phases +5. [ ] Schedule kickoff meeting +6. [ ] Assign phase owners + +--- + +## Example Feedback (Remove This Section After First Review) + +### Jane Doe - 2025-10-28 + +**Overall Assessment**: Request Changes + +**Summary**: +The RFC is well-written and the approach is sound. I have concerns about process death handling for Composables and would like to see that addressed before approval. + +**Detailed Comments**: + +#### Section: Implementation Details + +- **Comment**: The callback storage approach for Composables won't survive process death. This could lead to lost results in production. +- **Severity**: Major +- **Suggestion**: Consider using SavedStateHandle even for initial release, or add clear warnings in documentation about this limitation. + +#### Section: API Design + +- **Comment**: The `navigateForResult` method name is clear and intuitive. +- **Severity**: Minor (Positive) +- **Suggestion**: None - this is good as-is. + +#### Section: Testing Strategy + +- **Comment**: Should we add specific tests for process death scenarios? +- **Severity**: Question +- **Suggestion**: Add a test phase for configuration changes and process death simulation. + +--- + +**Template Version**: 1.0 +**Last Updated**: 2025-10-28 diff --git a/docs/rfcs/001-navigate-for-result/log.md b/docs/rfcs/001-navigate-for-result/log.md new file mode 100644 index 0000000..e544f4d --- /dev/null +++ b/docs/rfcs/001-navigate-for-result/log.md @@ -0,0 +1,591 @@ +# Decision Log: Navigate-For-Result Feature + +This document tracks all major decisions made during the design of the navigate-for-result feature for Nibel. + +## Decision Format + +Each decision includes: + +- **Date**: When the decision was made +- **Decision**: What was decided +- **Context**: Why this decision was needed +- **Options Considered**: Alternative approaches +- **Rationale**: Why this option was chosen +- **Impact**: How this affects the implementation +- **Status**: Active, Superseded, or Deprecated + +--- + +## D001: Optional Parameter for Result Type + +**Date**: 2025-10-28 +**Status**: Active ✅ + +### Decision + +Add an optional `result` parameter to both `@UiEntry` and `@UiExternalEntry` annotations with default value `NoResult::class`. + +### Context + +Need to extend existing annotations to support result types while maintaining 100% backwards compatibility. The Turo repository already uses Nibel extensively. + +### Options Considered + +1. **Optional parameter on existing annotations** ⭐ SELECTED + + - Pros: Backwards compatible, consistent with `args` parameter, simple + - Cons: None significant + - Implementation: `val result: KClass = NoResult::class` + +2. **Separate annotations (@UiEntryWithResult, @UiExternalEntryWithResult)** + + - Pros: Cleaner separation, no confusion + - Cons: Not backwards compatible, annotation duplication, migration required + - Rejected: Breaks existing code + +3. **Builder pattern for annotations** + - Pros: Maximum flexibility + - Cons: Too different from existing pattern, verbose, not idiomatic for annotations + - Rejected: Too complex + +### Rationale + +- Follows the exact same pattern as the existing `args` parameter +- Zero breaking changes - default parameter makes it completely optional +- Developers familiar with `args` will immediately understand `result` +- Enables gradual adoption without coordination + +### Impact + +- **Backwards Compatibility**: ✅ 100% - existing code works unchanged +- **Learning Curve**: ✅ Minimal - follows existing pattern +- **Migration Effort**: ✅ Zero - opt-in feature +- **Code Complexity**: ✅ Low - one parameter addition + +--- + +## D002: NavigationController Method-Based API + +**Date**: 2025-10-28 +**Status**: Active ✅ + +### Decision + +Implement result handling via methods on `NavigationController`: + +- `navigateForResult(entry, callback)` +- `setResultAndNavigateBack(result)` +- `cancelResultAndNavigateBack()` + +### Context + +Need to define how screens set and receive results. The API should be discoverable, type-safe, and consistent with existing patterns. + +### Options Considered + +1. **NavigationController methods** ⭐ SELECTED + + - Pros: Centralized, consistent, easy to discover, clear intent + - Cons: None significant + - API: `navigator.setResultAndNavigateBack(result)` + +2. **ResultCallback parameter to composable** + + - Pros: Explicit in function signature + - Cons: Verbose, not idiomatic for Compose, hard to provide default + - Rejected: Poor API ergonomics + +3. **Composition local (LocalResultCallback.current)** + + - Pros: Consistent with LocalArgs pattern + - Cons: Less discoverable, awkward API, null handling complexity + - Rejected: Awkward to use + +4. **Shared ViewModel for results** + - Pros: Familiar pattern + - Cons: Not type-safe, couples modules, boilerplate + - Rejected: This is what we're trying to replace + +### Rationale + +- NavigationController is already the central navigation authority +- All navigation actions go through NavigationController +- Easy to find methods via IDE autocomplete +- Symmetric API: `navigateForResult` pairs with `setResultAndNavigateBack` +- Clear lifecycle: result is delivered and cleared automatically + +### Impact + +- **API Clarity**: ✅ Excellent - intent is explicit +- **Discoverability**: ✅ High - methods appear in IDE autocomplete +- **Type Safety**: ✅ Full - generics ensure type matching +- **Testing**: ✅ Easy - can mock NavigationController + +--- + +## D003: Dedicated navigateForResult Method + +**Date**: 2025-10-28 +**Status**: Active ✅ + +### Decision + +Create a new `navigateForResult()` method separate from `navigateTo()`. + +### Context + +Need to decide whether to extend the existing `navigateTo()` method or create a new method for result-based navigation. + +### Options Considered + +1. **New navigateForResult() method** ⭐ SELECTED + + - Pros: Explicit intent, clear API, type-safe callback, no overloading complexity + - Cons: One more method to learn + - API: `navigator.navigateForResult(entry) { result -> ... }` + +2. **Optional callback parameter on navigateTo()** + + - Pros: Single method, familiar name + - Cons: Overloading confusion, unclear when result is expected, optional param complexity + - Rejected: API becomes ambiguous + +3. **Result callback in Entry creation** + - Pros: Self-contained entry + - Cons: Complex entry creation, hard to test, lifecycle unclear + - Rejected: Poor separation of concerns + +### Rationale + +- **Explicit Intent**: Method name clearly indicates result is expected +- **Type Safety**: Callback parameter type can be inferred from entry's ResultEntry interface +- **No Ambiguity**: Clear when result is expected vs simple navigation +- **Better IDE Support**: Autocomplete suggests correct method based on use case +- **Follows Android Patterns**: Similar to Activity Result API's explicit registration + +### Impact + +- **API Clarity**: ✅ Excellent - intent is unambiguous +- **Learning Curve**: ✅ Low - clear method name +- **Code Clarity**: ✅ High - reading code makes intent obvious +- **Maintenance**: ✅ Easy - separate concerns + +--- + +## D004: Nullable Result for Cancellation + +**Date**: 2025-10-28 +**Status**: Active ✅ + +### Decision + +Use nullable result type `T?` where `null` indicates cancellation. + +### Context + +Need to handle the case where user cancels or backs out without providing a result. + +### Options Considered + +1. **Nullable result (T?)** ⭐ SELECTED + + - Pros: Simple, common Android pattern, easy to understand + - Cons: null can be ambiguous + - API: `callback: (PhotoResult?) -> Unit` + +2. **Sealed class Result** + + - Pros: Explicit success/failure, clear semantics + - Cons: Verbose, boilerplate, overkill for this use case + - API: `sealed class NavigationResult { data class Success(val value: T), object Cancelled }` + - Rejected: Too complex for this use case + +3. **Separate callbacks (onResult, onCancelled)** + + - Pros: Clear separation of concerns + - Cons: Two callbacks to manage, verbose, more state to track + - API: `navigateForResult(entry, onResult = {...}, onCancelled = {...})` + - Rejected: Too verbose + +4. **Optional Result wrapper** + - Pros: Explicit optional semantics + - Cons: Not idiomatic for Kotlin, unnecessary wrapper + - Rejected: Kotlin has nullable types + +### Rationale + +- **Android Consistency**: Activity Result API uses null for cancellation +- **Kotlin Idioms**: Nullable types are natural in Kotlin +- **Simplicity**: `result?.let { ... }` is concise and clear +- **No Boilerplate**: No wrapper classes needed +- **Common Pattern**: Developers are familiar with null = cancelled + +### Impact + +- **Code Simplicity**: ✅ High - minimal boilerplate +- **Readability**: ✅ Good - clear with safe call operator +- **Learning Curve**: ✅ Zero - standard Kotlin pattern +- **Edge Cases**: ⚠️ Need to document: null = cancelled, not error + +--- + +## D005: DestinationWithResult Interface + +**Date**: 2025-10-28 +**Status**: Active ✅ + +### Decision + +Create `DestinationWithResult` interface for multi-module navigation with results. + +### Context + +Multi-module navigation uses destinations defined in shared navigation module. Need to extend this pattern to support results. + +### Options Considered + +1. **DestinationWithResult interface** ⭐ SELECTED + + - Pros: Type-safe, compile-time checking, follows existing pattern + - Cons: Destination must implement interface + - API: `data class MyDestination(...) : DestinationWithArgs<...>, DestinationWithResult` + +2. **Runtime result type registration** + + - Pros: No interface needed, flexible + - Cons: Runtime errors, not type-safe, hard to validate + - Rejected: Loses compile-time safety + +3. **Result type in navigation module metadata** + + - Pros: Centralized + - Cons: Tight coupling, harder to maintain, not type-safe + - Rejected: Poor separation of concerns + +4. **Annotation on destination class** + - Pros: Declarative + - Cons: Hard to access at compile-time, runtime lookup needed + - Rejected: Less type-safe than interface + +### Rationale + +- **Consistency**: Follows the exact pattern of `DestinationWithArgs` +- **Type Safety**: Result type is part of destination's type signature +- **Compile-Time Validation**: Mismatches caught during compilation +- **Discoverability**: IDE shows which destinations return results +- **Code Generation**: Processor can easily extract result type from destination + +### Impact + +- **Type Safety**: ✅ Full - compile-time checking +- **Consistency**: ✅ High - matches existing DestinationWithArgs pattern +- **Discoverability**: ✅ Good - clear from destination interface +- **Migration**: ✅ Easy - add interface to destination + +--- + +## D006: Fragment Result API for Fragments + +**Date**: 2025-10-28 +**Status**: Active ✅ + +### Decision + +Use Android's Fragment Result API for result delivery when `ImplementationType.Fragment` is used. + +### Context + +Fragment implementation type wraps composables in fragments. Need to decide how to deliver results in this case. + +### Options Considered + +1. **Fragment Result API** ⭐ SELECTED + + - Pros: Native Android support, survives process death, well-tested, documented + - Cons: Fragment-specific (not an issue since we're using fragments) + - Implementation: Use SavedStateHandle for result passing + +2. **Custom callback storage** + + - Pros: Unified with Composable implementation + - Cons: Doesn't survive process death, reinventing the wheel + - Rejected: Fragment Result API is better + +3. **Activity Result API** + - Pros: Standard pattern + - Cons: Requires Activity, complex setup, overkill + - Rejected: Too complex for screen-to-screen results + +### Rationale + +- **Native Support**: Android provides this specifically for Fragment results +- **Process Death**: Survives configuration changes and process death +- **Battle-Tested**: Used in many production apps +- **Standard**: Follows Android best practices +- **Simple Integration**: Easy to integrate with NavigationController + +### Impact + +- **Reliability**: ✅ High - proven solution +- **Process Death**: ✅ Supported - results survive process death +- **Complexity**: ✅ Low - standard Android API +- **Documentation**: ✅ Excellent - Android documentation available + +--- + +## D007: Callback Storage for Composables + +**Date**: 2025-10-28 +**Status**: Active ✅ + +### Decision + +Use in-memory callback storage in NavigationController for Composable implementation type results. + +### Context + +Composable implementation type doesn't use fragments. Need a mechanism to deliver results for pure Composable navigation. + +### Options Considered + +1. **Callback storage in NavigationController** ⭐ SELECTED + + - Pros: Simple, works well with Compose, low overhead + - Cons: Doesn't survive process death (acceptable for most use cases) + - Implementation: Map Unit> in NavigationController + +2. **SavedStateHandle for Composable results** + + - Pros: Survives process death + - Cons: Complex, requires serialization, overkill for most use cases + - Considered for future enhancement + +3. **Navigation component SavedState** + + - Pros: Native Navigation component support + - Cons: Limited to Navigation component specifics, complex + - Rejected: Too specific to Navigation component + +4. **Shared ViewModel** + - Pros: Familiar pattern + - Cons: Not type-safe, boilerplate, this is what we're replacing + - Rejected: Defeats purpose of this feature + +### Rationale + +- **Simplicity**: Straightforward implementation +- **Performance**: Low overhead, no serialization +- **Compose Native**: Works naturally with Compose lifecycle +- **Acceptable Tradeoff**: Process death is rare, and can be handled if needed +- **Future Proof**: Can add SavedStateHandle support later if needed + +### Impact + +- **Implementation**: ✅ Simple - straightforward Map storage +- **Performance**: ✅ Excellent - no serialization overhead +- **Process Death**: ⚠️ Not supported - documented limitation +- **Future Enhancement**: ✅ Possible - can add SavedStateHandle later + +--- + +## D008: NoResult Marker Class + +**Date**: 2025-10-28 +**Status**: Active ✅ + +### Decision + +Create a `NoResult` object as the default value for the `result` parameter. + +### Context + +Need a way to indicate "no result" as the default behavior. Should follow the pattern of `NoArgs`. + +### Options Considered + +1. **NoResult object (Parcelable)** ⭐ SELECTED + + - Pros: Consistent with NoArgs, clear meaning, type-safe + - Cons: None + - Implementation: `object NoResult : Parcelable` + +2. **Null as default** + + - Pros: Simple + - Cons: Can't use nullable KClass, inconsistent with args pattern + - Rejected: Type system issues + +3. **Different annotation for results** + - Pros: No default needed + - Cons: Not backwards compatible + - Rejected: Already decided on optional parameter approach + +### Rationale + +- **Consistency**: Exactly matches the `NoArgs` pattern +- **Type Safety**: Parcelable type fits the type system +- **Clarity**: Explicit "NoResult" is clear in code +- **Code Generation**: Easy to check `if (resultType == NoResult::class)` + +### Impact + +- **Consistency**: ✅ Perfect - matches NoArgs exactly +- **Clarity**: ✅ High - intent is explicit +- **Implementation**: ✅ Trivial - single object declaration + +--- + +## D009: ResultEntry Interface + +**Date**: 2025-10-28 +**Status**: Active ✅ + +### Decision + +Create `ResultEntry` interface for entries that return results. + +### Context + +Need a way to distinguish entries that return results from those that don't, for type safety and runtime checks. + +### Options Considered + +1. **ResultEntry interface** ⭐ SELECTED + + - Pros: Type-safe, clear distinction, enables compile-time checks + - Cons: Additional interface (minimal cost) + - Implementation: `interface ResultEntry : Entry { val resultType: Class }` + +2. **Marker interface (no generics)** + + - Pros: Simpler + - Cons: No type safety, runtime casting needed + - Rejected: Loses type safety + +3. **Reflection-based approach** + - Pros: No interface needed + - Cons: Slow, error-prone, no compile-time checking + - Rejected: Performance and safety issues + +### Rationale + +- **Type Safety**: Generic parameters ensure result type is known at compile-time +- **Runtime Checks**: `resultType` property enables runtime validation if needed +- **API Clarity**: Clear from type signature that entry returns result +- **Code Generation**: Easy to generate implementing classes + +### Impact + +- **Type Safety**: ✅ Full - compile-time and runtime +- **API Clarity**: ✅ High - clear from interface +- **Code Generation**: ✅ Straightforward - add interface to generated class + +--- + +## D010: Compile-Time Validation Only + +**Date**: 2025-10-28 +**Status**: Active ✅ + +### Decision + +Rely on compile-time validation via KSP, minimal runtime checks. + +### Context + +Need to decide how much validation to do at runtime vs compile-time. + +### Options Considered + +1. **Compile-time only** ⭐ SELECTED + + - Pros: Errors caught early, no runtime overhead, type-safe + - Cons: Can't catch errors from dynamic code (rare in this context) + - Implementation: KSP validates all result types and parameters + +2. **Compile-time + runtime validation** + + - Pros: Defense in depth + - Cons: Runtime overhead, duplicates checks, throws at runtime + - Considered for future if issues arise + +3. **Runtime only** + - Pros: Flexible + - Cons: Late error detection, poor developer experience + - Rejected: Defeats purpose of type-safe library + +### Rationale + +- **Nibel Philosophy**: Compile-time safety is core to Nibel's design +- **Performance**: No runtime validation overhead +- **Developer Experience**: Errors caught immediately during development +- **Sufficient**: All result types are known at compile-time (annotations) + +### Impact + +- **Performance**: ✅ Excellent - zero runtime overhead +- **Safety**: ✅ High - compile-time catches all issues +- **Developer Experience**: ✅ Excellent - immediate feedback + +--- + +## Future Considerations + +These items are explicitly deferred for potential future enhancements: + +### FC001: Process Death Support for Composables + +**Status**: Deferred + +Consider adding SavedStateHandle support for Composable results to survive process death. + +**When to Revisit**: After v1 feedback, if users report process death issues + +### FC002: Activity Result Contracts Integration + +**Status**: Deferred + +Consider providing wrappers for common Activity Result Contracts (camera, gallery, etc.). + +**When to Revisit**: If users frequently request this feature + +### FC003: Streaming Results + +**Status**: Deferred + +Consider supporting Flow for streaming results (progress updates, multi-select, etc.). + +**When to Revisit**: If strong use cases emerge + +### FC004: Result Validation Framework + +**Status**: Deferred + +Consider adding a validation framework for result data. + +**When to Revisit**: If users report issues with invalid result data + +--- + +## Summary Statistics + +- **Total Decisions**: 10 active +- **Backwards Compatible**: 10/10 (100%) +- **Breaking Changes**: 0 +- **Deferred Decisions**: 4 +- **Rejected Alternatives**: 15 + +## Review Checklist + +- [x] All decisions have clear rationale +- [x] Impact assessment completed for each decision +- [x] Alternatives considered and documented +- [x] Backwards compatibility verified +- [x] Future considerations identified +- [x] Decisions align with Nibel philosophy + +--- + +**Document Status**: Complete ✅ +**Last Updated**: 2025-10-28 +**Reviewed By**: TBD diff --git a/docs/rfcs/001-navigate-for-result/rfc.md b/docs/rfcs/001-navigate-for-result/rfc.md new file mode 100644 index 0000000..a0b5e39 --- /dev/null +++ b/docs/rfcs/001-navigate-for-result/rfc.md @@ -0,0 +1,1805 @@ +# RFC 001: Navigate-For-Result Feature + +| | | +| ---------------- | ------------------------------------------------------------------ | +| **RFC Number** | 001 | +| **Title** | Navigate-For-Result: Type-Safe Result Handling in Nibel Navigation | +| **Status** | Draft | +| **Authors** | Nibel Team | +| **Created** | 2025-10-28 | +| **Last Updated** | 2025-10-28 | +| **Reviewers** | TBD | + +## Table of Contents + +- [Background](#background) +- [Motivation](#motivation) +- [Proposed Solution](#proposed-solution) +- [API Design](#api-design) +- [Implementation Details](#implementation-details) +- [Generated Code Examples](#generated-code-examples) +- [Backwards Compatibility](#backwards-compatibility) +- [Testing Strategy](#testing-strategy) +- [Implementation Plan](#implementation-plan) +- [Alternatives Considered](#alternatives-considered) +- [Open Questions](#open-questions) +- [Security Implications](#security-implications) +- [References](#references) + +--- + +## Background + +Nibel is a type-safe navigation library that enables seamless integration of Jetpack Compose in fragment-based Android apps. It currently supports three navigation scenarios: + +- **fragment → compose**: Navigate from existing fragments to Compose screens +- **compose → compose**: Navigate between Compose screens +- **compose → fragment**: Navigate from Compose screens back to fragments + +The library uses annotation-driven code generation (via KSP) to create type-safe navigation entry points with compile-time validation. + +### Current State + +Currently, Nibel supports: + +- Type-safe navigation with `NavigationController` +- Optional arguments via `args: KClass` parameter +- Multi-module navigation via destinations +- Both Fragment and Composable implementation types + +**What's Missing**: There is no built-in mechanism for screens to return results to their callers. This is a common pattern in Android (e.g., Activity Result API, Fragment Result API) and is essential for many user flows. + +--- + +## Motivation + +### Problem Statement + +Modern Android apps frequently need screens to return data to their callers: + +1. **User Selection Flows**: Photo picker, contact selector, location picker +2. **Form Confirmation**: User completes a form and returns the submitted data +3. **Decision Dialogs**: User makes a choice (approve/reject, select option) +4. **Data Creation**: Create a new item and return its ID +5. **Multi-step Workflows**: Gather information across multiple screens + +Without native result support, developers must resort to workarounds: + +- Shared ViewModels (couples modules, breaks encapsulation) +- Event buses (lacks type safety, hard to track) +- Navigation arguments in reverse direction (complex, error-prone) +- Direct callback passing (doesn't survive process death) + +### Goals + +1. **Type Safety**: Compile-time validation that result types match between caller and callee +2. **Backwards Compatible**: Existing code works without modifications +3. **Multi-Module Support**: Works across feature modules via destinations +4. **Android Integration**: Leverages Activity Result API pattern +5. **Consistent API**: Follows Nibel's existing design patterns +6. **Opt-in**: Feature is optional, not required for simple navigation + +### Success Criteria + +- Zero breaking changes to existing code +- Works for both `@UiEntry` and `@UiExternalEntry` +- Supports all implementation types (Fragment/Composable) +- Type-safe result delivery with compile-time checks +- Comprehensive test coverage +- Clear migration path and documentation + +--- + +## Proposed Solution + +### High-Level Design + +Extend the existing annotation system with an optional `result` parameter, similar to how `args` works: + +```kotlin +@UiEntry( + type = ImplementationType.Composable, + args = PhotoPickerArgs::class, + result = PhotoPickerResult::class // NEW +) +@Composable +fun PhotoPickerScreen( + args: PhotoPickerArgs, + navigator: NavigationController +) { + // Implementation + navigator.setResultAndNavigateBack(PhotoPickerResult(selectedPhoto)) +} +``` + +### Key Components + +1. **Annotation Extension**: Add optional `result` parameter to both `@UiEntry` and `@UiExternalEntry` +2. **NavigationController Methods**: Add result-aware navigation and result-setting methods +3. **Result Interfaces**: Generate result-aware entry interfaces for type safety +4. **Result Delivery Mechanism**: Handle result delivery across Fragment/Composable boundaries +5. **Multi-Module Support**: Extend destinations to carry result type information + +### Design Principles + +- **Opt-in**: Default is `NoResult::class`, no result handling +- **Type Safety**: Result type in annotation must match callback type +- **Nullable Results**: Callback receives `T?` where `null` indicates cancellation +- **Symmetric API**: Clear methods for setting and canceling results +- **Consistent**: Follows existing Nibel patterns (similar to args handling) + +--- + +## API Design + +### 1. Annotation Changes + +#### @UiEntry (Internal Navigation) + +```kotlin +@Target(FUNCTION) +@MustBeDocumented +annotation class UiEntry( + val type: ImplementationType, + val args: KClass = NoArgs::class, + val result: KClass = NoResult::class // NEW +) +``` + +#### @UiExternalEntry (Multi-Module Navigation) + +```kotlin +@Target(FUNCTION) +@MustBeDocumented +annotation class UiExternalEntry( + val type: ImplementationType, + val destination: KClass, + val result: KClass = NoResult::class // NEW +) +``` + +#### NoResult Marker Class + +```kotlin +package nibel.runtime + +/** + * Marker class indicating that a screen does not return a result. + * This is the default value for the `result` parameter in navigation annotations. + */ +@Parcelize +object NoResult : Parcelable +``` + +### 2. NavigationController Extensions + +```kotlin +abstract class NavigationController( + val fragmentSpec: FragmentSpec<*> = Nibel.fragmentSpec, + val composeSpec: ComposeSpec<*> = Nibel.composeSpec, +) { + + // Existing methods... + abstract fun navigateBack() + abstract fun navigateTo(entry: Entry, ...) + abstract fun navigateTo(externalDestination: ExternalDestination, ...) + + // NEW: Navigate for result + /** + * Navigate to a screen and receive a result when it completes. + * + * @param entry The entry to navigate to (must be a ResultEntry) + * @param callback Callback invoked with result (null if cancelled) + */ + abstract fun navigateForResult( + entry: Entry, + callback: (TResult?) -> Unit + ) + + // NEW: Set result and navigate back + /** + * Set a result and navigate back to the caller. + * The result will be delivered to the callback provided in navigateForResult. + * + * @param result The result to return to the caller + */ + abstract fun setResultAndNavigateBack(result: TResult) + + // NEW: Cancel and navigate back + /** + * Navigate back without setting a result (equivalent to user pressing back). + * The callback will receive null. + */ + abstract fun cancelResultAndNavigateBack() +} +``` + +### 3. Usage Examples + +#### Example 1: Internal Entry with Result (Composable) + +```kotlin +// Define result type +@Parcelize +data class PhotoPickerResult( + val photoUri: String +) : Parcelable + +// Define args type +@Parcelize +data class PhotoPickerArgs( + val allowMultiple: Boolean = false +) : Parcelable + +// Define screen with result +@UiEntry( + type = ImplementationType.Composable, + args = PhotoPickerArgs::class, + result = PhotoPickerResult::class +) +@Composable +fun PhotoPickerScreen( + args: PhotoPickerArgs, + navigator: NavigationController +) { + // UI implementation + Button(onClick = { + val result = PhotoPickerResult("content://photo/123") + navigator.setResultAndNavigateBack(result) + }) { + Text("Select Photo") + } + + Button(onClick = { + navigator.cancelResultAndNavigateBack() + }) { + Text("Cancel") + } +} + +// Caller +@UiEntry(type = ImplementationType.Composable) +@Composable +fun ProfileScreen(navigator: NavigationController) { + Button(onClick = { + val args = PhotoPickerArgs(allowMultiple = false) + navigator.navigateForResult( + entry = PhotoPickerScreenEntry.newInstance(args) + ) { result: PhotoPickerResult? -> + if (result != null) { + // User selected a photo + updateProfilePhoto(result.photoUri) + } else { + // User cancelled + } + } + }) { + Text("Change Photo") + } +} +``` + +#### Example 2: External Entry with Result (Multi-Module) + +```kotlin +// navigation module: Define destination with result +data class PhotoPickerDestination( + override val args: PhotoPickerArgs +) : DestinationWithArgs, + DestinationWithResult // NEW interface + +// feature-photo module: Implement screen +@UiExternalEntry( + type = ImplementationType.Fragment, + destination = PhotoPickerDestination::class, + result = PhotoPickerResult::class +) +@Composable +fun PhotoPickerScreen( + args: PhotoPickerArgs, + navigator: NavigationController +) { + // Implementation... + navigator.setResultAndNavigateBack(PhotoPickerResult(selectedUri)) +} + +// feature-profile module: Call with result +@UiExternalEntry( + type = ImplementationType.Composable, + destination = ProfileScreenDestination::class +) +@Composable +fun ProfileScreen(navigator: NavigationController) { + Button(onClick = { + navigator.navigateForResult( + externalDestination = PhotoPickerDestination( + args = PhotoPickerArgs(allowMultiple = false) + ) + ) { result: PhotoPickerResult? -> + result?.let { updateProfilePhoto(it.photoUri) } + } + }) { + Text("Change Photo") + } +} +``` + +#### Example 3: Fragment Implementation Type with Result + +```kotlin +@UiEntry( + type = ImplementationType.Fragment, + args = ConfirmationArgs::class, + result = ConfirmationResult::class +) +@Composable +fun ConfirmationDialog( + args: ConfirmationArgs, + navigator: NavigationController +) { + AlertDialog( + onDismissRequest = { navigator.cancelResultAndNavigateBack() }, + confirmButton = { + Button(onClick = { + navigator.setResultAndNavigateBack( + ConfirmationResult(confirmed = true) + ) + }) { + Text("Confirm") + } + }, + dismissButton = { + Button(onClick = { + navigator.setResultAndNavigateBack( + ConfirmationResult(confirmed = false) + ) + }) { + Text("Cancel") + } + }, + text = { Text(args.message) } + ) +} +``` + +#### Example 4: No Args, Only Result + +```kotlin +@Parcelize +data class UserSelectionResult( + val userId: String, + val userName: String +) : Parcelable + +@UiEntry( + type = ImplementationType.Composable, + result = UserSelectionResult::class +) +@Composable +fun UserSelectionScreen(navigator: NavigationController) { + LazyColumn { + items(users) { user -> + ListItem( + text = { Text(user.name) }, + modifier = Modifier.clickable { + navigator.setResultAndNavigateBack( + UserSelectionResult(user.id, user.name) + ) + } + ) + } + } +} + +// Caller +navigator.navigateForResult( + entry = UserSelectionScreenEntry.newInstance() +) { result: UserSelectionResult? -> + result?.let { selectedUser -> + updateSelectedUser(selectedUser.userId) + } +} +``` + +--- + +## Implementation Details + +### 1. Annotations Module (nibel-annotations) + +#### Changes to Existing Files + +**UiEntry.kt** - Add result parameter: + +```kotlin +annotation class UiEntry( + val type: ImplementationType, + val args: KClass = NoArgs::class, + val result: KClass = NoResult::class // ADD THIS +) +``` + +**UiExternalEntry.kt** - Add result parameter: + +```kotlin +annotation class UiExternalEntry( + val type: ImplementationType, + val destination: KClass, + val result: KClass = NoResult::class // ADD THIS +) +``` + +#### New Files + +**NoResult.kt**: + +```kotlin +package nibel.annotations + +import android.os.Parcelable +import kotlinx.parcelize.Parcelize + +/** + * Marker indicating a screen does not return a result. + * This is the default value for the `result` parameter. + * + * See [UiEntry], [UiExternalEntry]. + */ +@Parcelize +object NoResult : Parcelable +``` + +### 2. Runtime Module (nibel-runtime) + +#### New Interfaces + +**ResultEntry.kt**: + +```kotlin +package nibel.runtime + +/** + * Base interface for entries that return a result. + * Generated entry classes will implement this when result parameter is specified. + */ +interface ResultEntry : Entry { + /** + * Result type class for runtime type checking + */ + val resultType: Class +} +``` + +**DestinationWithResult.kt**: + +```kotlin +package nibel.runtime + +/** + * Interface for destinations that return a result. + * Used in multi-module navigation with results. + */ +interface DestinationWithResult { + /** + * Result type class for runtime resolution + */ + val resultType: Class + get() = TODO("Compiler will implement this in destination classes") +} +``` + +#### NavigationController Extensions + +**NavigationController.kt** - Add new methods: + +````kotlin +abstract class NavigationController( + val fragmentSpec: FragmentSpec<*> = Nibel.fragmentSpec, + val composeSpec: ComposeSpec<*> = Nibel.composeSpec, +) { + + // Existing methods remain unchanged... + + /** + * Navigate to a screen and receive a typed result when it completes. + * + * The callback will be invoked with: + * - Non-null result if callee calls setResultAndNavigateBack + * - Null if callee calls cancelResultAndNavigateBack or user presses back + * + * Example: + * ``` + * navigator.navigateForResult(PhotoPickerEntry.newInstance(args)) { result: PhotoResult? -> + * result?.let { handlePhoto(it.uri) } + * } + * ``` + */ + abstract fun navigateForResult( + entry: Entry, + fragmentSpec: FragmentSpec<*> = this.fragmentSpec, + composeSpec: ComposeSpec<*> = this.composeSpec, + callback: (TResult?) -> Unit + ) + + /** + * Navigate to an external destination and receive a typed result. + * Used for multi-module navigation with results. + */ + abstract fun navigateForResult( + externalDestination: ExternalDestination, + fragmentSpec: FragmentSpec<*> = this.fragmentSpec, + composeSpec: ComposeSpec<*> = this.composeSpec, + callback: (TResult?) -> Unit + ) + + /** + * Set a result and navigate back to the previous screen. + * The result will be delivered to the callback in navigateForResult. + * + * Can only be called from screens annotated with a result parameter. + */ + abstract fun setResultAndNavigateBack(result: TResult) + + /** + * Navigate back without setting a result (equivalent to back press). + * The navigateForResult callback will receive null. + * + * Can be called from any screen. + */ + abstract fun cancelResultAndNavigateBack() +} +```` + +#### Implementation in NibelNavigationController + +**NibelNavigationController.kt** - Implement result delivery: + +```kotlin +class NibelNavigationController( + private val navController: NavController, + fragmentSpec: FragmentSpec<*>, + composeSpec: ComposeSpec<*>, +) : NavigationController(fragmentSpec, composeSpec) { + + // Result callback storage + private val resultCallbacks = mutableMapOf Unit>() + + override fun navigateForResult( + entry: Entry, + fragmentSpec: FragmentSpec<*>, + composeSpec: ComposeSpec<*>, + callback: (TResult?) -> Unit + ) { + // Generate unique key for this navigation + val requestKey = UUID.randomUUID().toString() + + // Store callback + resultCallbacks[requestKey] = callback as (Any?) -> Unit + + // Navigate with request key + when (entry) { + is FragmentEntry -> { + // Use Fragment Result API + navController.currentBackStackEntry?.savedStateHandle?.let { handle -> + handle.getLiveData(requestKey).observeForever { result -> + callback(result) + handle.remove(requestKey) + resultCallbacks.remove(requestKey) + } + } + navigateTo(entry, fragmentSpec, composeSpec) + } + is ComposableEntry<*> -> { + // Store request key in entry metadata + (entry as? ComposableEntryWithResult)?.requestKey = requestKey + navigateTo(entry, fragmentSpec, composeSpec) + } + } + } + + override fun setResultAndNavigateBack(result: TResult) { + // Get current entry's request key + val requestKey = getCurrentRequestKey() + + // Deliver result via appropriate mechanism + when (getCurrentImplementationType()) { + ImplementationType.Fragment -> { + // Use Fragment Result API + navController.previousBackStackEntry?.savedStateHandle?.set(requestKey, result) + } + ImplementationType.Composable -> { + // Invoke stored callback + resultCallbacks[requestKey]?.invoke(result) + resultCallbacks.remove(requestKey) + } + } + + navigateBack() + } + + override fun cancelResultAndNavigateBack() { + val requestKey = getCurrentRequestKey() + + // Deliver null result + when (getCurrentImplementationType()) { + ImplementationType.Fragment -> { + navController.previousBackStackEntry?.savedStateHandle?.set(requestKey, null) + } + ImplementationType.Composable -> { + resultCallbacks[requestKey]?.invoke(null) + resultCallbacks.remove(requestKey) + } + } + + navigateBack() + } + + private fun getCurrentRequestKey(): String { + // Extract from current entry metadata + return navController.currentBackStackEntry + ?.arguments?.getString(REQUEST_KEY_ARG) + ?: error("No request key found for current entry") + } + + companion object { + private const val REQUEST_KEY_ARG = "nibel_request_key" + } +} +``` + +### 3. Compiler Module (nibel-compiler) + +#### Metadata Changes + +**EntryMetadata.kt**: + +```kotlin +sealed interface EntryMetadata { + val argsQualifiedName: String? + val resultQualifiedName: String? // NEW + val parameters: Map +} + +data class InternalEntryMetadata( + override val argsQualifiedName: String?, + override val resultQualifiedName: String?, // NEW + override val parameters: Map, +) : EntryMetadata + +data class ExternalEntryMetadata( + val destinationName: String, + val destinationPackageName: String, + val destinationQualifiedName: String, + override val argsQualifiedName: String?, + override val resultQualifiedName: String?, // NEW + override val parameters: Map, +) : EntryMetadata +``` + +#### Code Generation Changes + +**EntryGeneratingVisitor.kt** - Extract result type from annotation: + +```kotlin +private fun extractMetadata(function: KSFunctionDeclaration): EntryMetadata { + val annotation = function.annotations.first { /* UiEntry or UiExternalEntry */ } + + // Existing args extraction... + val argsType = annotation.arguments + .find { it.name?.asString() == "args" } + ?.value as? KSType + + // NEW: Extract result type + val resultType = annotation.arguments + .find { it.name?.asString() == "result" } + ?.value as? KSType + + val resultQualifiedName = resultType?.declaration?.qualifiedName?.asString() + ?.takeIf { it != "nibel.annotations.NoResult" } + + // Validate: if result is specified, it must be Parcelable + if (resultQualifiedName != null) { + validateParcelable(resultType, "result") + } + + return when (type) { + ExternalEntry -> ExternalEntryMetadata( + // ... existing fields + resultQualifiedName = resultQualifiedName, + ) + InternalEntry -> InternalEntryMetadata( + argsQualifiedName = argsQualifiedName, + resultQualifiedName = resultQualifiedName, + ) + } +} +``` + +**ComposableGenerator.kt** - Generate result-aware entry: + +```kotlin +fun generate(metadata: InternalEntryMetadata) { + val hasResult = metadata.resultQualifiedName != null + val resultTypeName = metadata.resultQualifiedName?.let { ClassName.bestGuess(it) } + + val entryClass = TypeSpec.classBuilder(entryClassName) + .addModifiers(KModifier.DATA) + .addAnnotation(Parcelize::class) + .apply { + if (hasResult) { + // Implement ResultEntry + addSuperinterface( + ClassName("nibel.runtime", "ResultEntry") + .parameterizedBy(argsTypeName, resultTypeName!!) + ) + + // Add resultType property + addProperty( + PropertySpec.builder("resultType", Class::class.asClassName().parameterizedBy(resultTypeName)) + .addModifiers(KModifier.OVERRIDE) + .initializer("%T::class.java", resultTypeName) + .build() + ) + } else { + // Normal ComposableEntry + addSuperinterface( + ClassName("nibel.runtime", "ComposableEntry") + .parameterizedBy(argsTypeName) + ) + } + } + // ... rest of generation + .build() +} +``` + +**FragmentGenerator.kt** - Similar changes for Fragment entries with results. + +--- + +## Generated Code Examples + +### Example 1: Internal Entry Without Result (Current Behavior - Unchanged) + +**Input**: + +```kotlin +@UiEntry(type = ImplementationType.Composable, args = MyArgs::class) +@Composable +fun MyScreen(args: MyArgs) { } +``` + +**Generated** (same as before): + +```kotlin +@Parcelize +class MyScreenEntry( + override val args: MyArgs, + override val name: String, +) : ComposableEntry(args, name) { + + @Composable + override fun ComposableContent() { + MyScreen(args = LocalArgs.current as MyArgs) + } + + companion object { + fun newInstance(args: MyArgs): ComposableEntry { + return MyScreenEntry( + args = args, + name = buildRouteName(MyScreenEntry::class.qualifiedName!!, args), + ) + } + } +} +``` + +### Example 2: Internal Entry With Result (New Feature) + +**Input**: + +```kotlin +@UiEntry( + type = ImplementationType.Composable, + args = PhotoPickerArgs::class, + result = PhotoPickerResult::class +) +@Composable +fun PhotoPickerScreen(args: PhotoPickerArgs, navigator: NavigationController) { } +``` + +**Generated**: + +```kotlin +@Parcelize +class PhotoPickerScreenEntry( + override val args: PhotoPickerArgs, + override val name: String, + internal var requestKey: String? = null, // For result delivery +) : ComposableEntry(args, name), + ResultEntry { // NEW interface + + override val resultType: Class + get() = PhotoPickerResult::class.java + + @Composable + override fun ComposableContent() { + PhotoPickerScreen( + args = LocalArgs.current as PhotoPickerArgs, + navigator = LocalNavigationController.current + ) + } + + companion object { + fun newInstance(args: PhotoPickerArgs): ResultEntry { + return PhotoPickerScreenEntry( + args = args, + name = buildRouteName(PhotoPickerScreenEntry::class.qualifiedName!!, args), + ) + } + } +} +``` + +### Example 3: External Entry With Result + +**Input**: + +```kotlin +// Destination +data class PhotoPickerDestination( + override val args: PhotoPickerArgs +) : DestinationWithArgs + +// Screen +@UiExternalEntry( + type = ImplementationType.Fragment, + destination = PhotoPickerDestination::class, + result = PhotoPickerResult::class +) +@Composable +fun PhotoPickerScreen(args: PhotoPickerArgs, navigator: NavigationController) { } +``` + +**Generated**: + +```kotlin +class PhotoPickerScreenEntry : ComposableFragment(), + ResultEntry { // NEW + + override val resultType: Class + get() = PhotoPickerResult::class.java + + @Composable + override fun ComposableContent() { + PhotoPickerScreen( + args = LocalArgs.current as PhotoPickerArgs, + navigator = LocalNavigationController.current + ) + } + + companion object : FragmentEntryFactory { + + override fun newInstance(destination: PhotoPickerDestination): FragmentEntry { + val fragment = PhotoPickerScreenEntry() + fragment.arguments = Bundle().apply { + putParcelable(ARGS_KEY, destination.args) + } + return FragmentEntry(fragment) + } + + fun newInstance(args: PhotoPickerArgs): FragmentEntry { + return newInstance(PhotoPickerDestination(args)) + } + } +} + +// Destination extension (generated in EntryFactoryProvider) +// This allows PhotoPickerDestination to declare it has a result +fun PhotoPickerDestination.resultType(): Class = + PhotoPickerResult::class.java +``` + +--- + +## Backwards Compatibility + +### Zero Breaking Changes Guarantee + +This feature is designed to be **100% backwards compatible**: + +1. **Optional Parameter**: The `result` parameter defaults to `NoResult::class` +2. **Existing Code Unchanged**: All current `@UiEntry` and `@UiExternalEntry` usages compile without modifications +3. **Generated Code**: Entries without result parameter generate the same code as before +4. **Runtime**: New methods are additions, not modifications +5. **API Surface**: No existing public APIs are changed or deprecated + +### Migration Path + +**Phase 1: Adoption (No Action Required)** + +- Library update with new feature +- Existing code continues to work +- No recompilation needed for apps not using the feature + +**Phase 2: Opt-In Usage** + +- Developers can add `result` parameter to new or existing screens +- Incrementally adopt the feature where needed +- No coordination required across modules + +**Phase 3: Full Adoption (Optional)** + +- Convert SharedViewModel patterns to navigate-for-result +- Remove workarounds for result passing +- Cleaner, more maintainable code + +### Compatibility Matrix + +| Scenario | Before | After | Compatible? | +| ------------------------- | -------- | -------- | ----------- | +| @UiEntry without args | ✅ Works | ✅ Works | ✅ Yes | +| @UiEntry with args | ✅ Works | ✅ Works | ✅ Yes | +| @UiExternalEntry | ✅ Works | ✅ Works | ✅ Yes | +| Multi-module navigation | ✅ Works | ✅ Works | ✅ Yes | +| Fragment implementation | ✅ Works | ✅ Works | ✅ Yes | +| Composable implementation | ✅ Works | ✅ Works | ✅ Yes | +| Existing generated code | ✅ Works | ✅ Works | ✅ Yes | + +### Testing Backwards Compatibility + +1. **Regression Test Suite**: Run all existing tests without modifications +2. **Sample App**: Existing sample app screens work without changes +3. **Binary Compatibility**: Check with binary compatibility validator +4. **Multi-Version Test**: Test with old and new versions coexisting + +--- + +## Testing Strategy + +### 1. Compilation Tests (tests/src/test/kotlin/nibel/tests/codegen/) + +**New File: `ResultEntryCompileTest.kt`** + +```kotlin +package nibel.tests.codegen + +import nibel.annotations.ImplementationType +import nibel.annotations.UiEntry +import nibel.annotations.UiExternalEntry + +// Test 1: Internal entry with result (Composable) +@UiEntry( + type = ImplementationType.Composable, + args = TestArgs::class, + result = TestResult::class +) +@Composable +fun ComposableEntryWithResult(args: TestArgs, navigator: NavigationController) = Unit + +// Test 2: Internal entry with result, no args +@UiEntry( + type = ImplementationType.Composable, + result = TestResult::class +) +@Composable +fun ComposableEntryWithResultNoArgs(navigator: NavigationController) = Unit + +// Test 3: Fragment entry with result +@UiEntry( + type = ImplementationType.Fragment, + args = TestArgs::class, + result = TestResult::class +) +@Composable +fun FragmentEntryWithResult(args: TestArgs) = Unit + +// Test 4: External entry with result +@UiExternalEntry( + type = ImplementationType.Composable, + destination = TestDestinationWithResult::class, + result = TestResult::class +) +@Composable +fun ExternalEntryWithResult(args: TestArgs, navigator: NavigationController) = Unit + +// Test result type +@Parcelize +data class TestResult(val value: String) : Parcelable + +// Verify generated code implements ResultEntry interface +// Verify resultType property is present +// Verify companion object newInstance returns correct type +``` + +**Validation Tests**: + +- Generated entry implements `ResultEntry` +- `resultType` property returns correct class +- Companion object `newInstance()` method signature is correct +- Type safety: result parameter type matches composable function usage + +### 2. Integration Tests + +**New File: `ResultNavigationTest.kt`** + +```kotlin +@Test +fun `navigateForResult delivers result correctly`() { + // Arrange + val testResult = TestResult("success") + var receivedResult: TestResult? = null + + // Act + navigator.navigateForResult(TestScreenEntry.newInstance(args)) { result -> + receivedResult = result + } + + // In test screen, call: + navigator.setResultAndNavigateBack(testResult) + + // Assert + assertEquals(testResult, receivedResult) +} + +@Test +fun `navigateForResult handles cancellation`() { + // Arrange + var receivedResult: TestResult? = TestResult("initial") + + // Act + navigator.navigateForResult(TestScreenEntry.newInstance(args)) { result -> + receivedResult = result + } + + navigator.cancelResultAndNavigateBack() + + // Assert + assertNull(receivedResult) +} + +@Test +fun `external entry with result works across modules`() { + // Test multi-module result delivery +} + +@Test +fun `fragment implementation type delivers results`() { + // Test Fragment Result API integration +} + +@Test +fun `composable implementation type delivers results`() { + // Test Compose result delivery mechanism +} +``` + +### 3. Sample App Integration + +**New Screens in Sample App**: + +**feature-A/PhotoPickerScreen.kt**: + +```kotlin +@UiExternalEntry( + type = ImplementationType.Composable, + destination = PhotoPickerDestination::class, + result = PhotoPickerResult::class +) +@Composable +fun PhotoPickerScreen(navigator: NavigationController, viewModel: PhotoPickerViewModel = hiltViewModel()) { + // Mock photo picker UI + LazyVerticalGrid(columns = GridCells.Fixed(3)) { + items(mockPhotos) { photo -> + Image( + painter = painterResource(photo.resId), + contentDescription = null, + modifier = Modifier + .aspectRatio(1f) + .clickable { + navigator.setResultAndNavigateBack( + PhotoPickerResult(photoUri = photo.uri) + ) + } + ) + } + } + + Button(onClick = { navigator.cancelResultAndNavigateBack() }) { + Text("Cancel") + } +} + +@Parcelize +data class PhotoPickerResult(val photoUri: String) : Parcelable +``` + +**feature-B/ProfileScreen.kt** (caller): + +```kotlin +@UiExternalEntry( + type = ImplementationType.Fragment, + destination = ProfileScreenDestination::class +) +@Composable +fun ProfileScreen(navigator: NavigationController) { + var selectedPhotoUri by remember { mutableStateOf(null) } + + Column { + selectedPhotoUri?.let { uri -> + AsyncImage(model = uri, contentDescription = "Profile photo") + } + + Button(onClick = { + navigator.navigateForResult( + PhotoPickerDestination + ) { result: PhotoPickerResult? -> + result?.let { selectedPhotoUri = it.photoUri } + } + }) { + Text("Select Photo") + } + } +} +``` + +### 4. Test Coverage Goals + +- **Unit Tests**: 90%+ coverage for new code +- **Compilation Tests**: All annotation combinations +- **Integration Tests**: All navigation flows with results +- **Sample App**: Real-world usage demonstration +- **Regression Tests**: Existing tests pass without modification + +### 5. Test Matrix + +| Implementation Type | Args | Result | Internal | External | Status | +| ------------------- | ---- | ------ | -------- | -------- | -------- | +| Composable | No | No | ✅ | ✅ | Existing | +| Composable | Yes | No | ✅ | ✅ | Existing | +| Composable | No | Yes | ✅ | ✅ | New | +| Composable | Yes | Yes | ✅ | ✅ | New | +| Fragment | No | No | ✅ | ✅ | Existing | +| Fragment | Yes | No | ✅ | ✅ | Existing | +| Fragment | No | Yes | ✅ | ✅ | New | +| Fragment | Yes | Yes | ✅ | ✅ | New | + +--- + +## Implementation Plan + +### Phase 1: Annotations & Foundation (Week 1) + +**Tasks**: + +1. Add `result` parameter to `@UiEntry` and `@UiExternalEntry` annotations +2. Create `NoResult` marker class +3. Update annotation KDoc with result examples +4. Create `ResultEntry` interface +5. Create `DestinationWithResult` interface + +**Deliverables**: + +- `nibel-annotations` module updated +- Basic `nibel-runtime` interfaces added +- Documentation updated + +**Testing**: + +- Annotations compile successfully +- Default parameter works (backwards compatible) + +### Phase 2: NavigationController API (Week 1-2) + +**Tasks**: + +1. Add `navigateForResult()` methods to NavigationController +2. Add `setResultAndNavigateBack()` method +3. Add `cancelResultAndNavigateBack()` method +4. Implement result delivery mechanism in `NibelNavigationController` +5. Handle Fragment Result API integration +6. Handle Composable result delivery + +**Deliverables**: + +- `NavigationController` API complete +- `NibelNavigationController` implementation +- Result callback storage and delivery + +**Testing**: + +- Unit tests for result delivery mechanism +- Mock navigation scenarios + +### Phase 3: Compiler Changes (Week 2-3) + +**Tasks**: + +1. Update `EntryMetadata` to include `resultQualifiedName` +2. Modify `EntryGeneratingVisitor` to extract result type +3. Update `ComposableGenerator` to generate result-aware entries +4. Update `FragmentGenerator` for Fragment result entries +5. Modify `EntryFactoryProviderGenerator` for external results +6. Add validation for result type (must be Parcelable) + +**Deliverables**: + +- `nibel-compiler` generates correct code for entries with results +- Generated code implements `ResultEntry` interface +- Proper type safety in generated code + +**Testing**: + +- Compilation tests for all scenarios +- Verify generated code structure +- Type checking validation + +### Phase 4: Testing & Sample App (Week 3-4) + +**Tasks**: + +1. Create `ResultEntryCompileTest.kt` +2. Create `ResultNavigationTest.kt` +3. Add PhotoPickerScreen to sample app (feature-A) +4. Add ProfileScreen caller to sample app (feature-B) +5. Add ConfirmationDialog example +6. Create comprehensive test matrix +7. Run full regression test suite + +**Deliverables**: + +- Complete test coverage +- Sample app demonstrations +- All tests passing + +**Testing**: + +- Run `./gradlew test` - all pass +- Run `./gradlew check` - all pass +- Sample app builds and runs + +### Phase 5: Documentation & Polish (Week 4) + +**Tasks**: + +1. Update main README with result feature +2. Write migration guide +3. Add KDoc to all public APIs +4. Create usage examples document +5. Update CHANGELOG +6. Review and polish all code + +**Deliverables**: + +- Complete documentation +- Migration guide +- Usage examples +- Release notes + +**Testing**: + +- Documentation review +- Code review +- Final QA pass + +### Timeline Summary + +| Phase | Duration | Start | End | +| ----------------------------- | ------------ | ------ | ------ | +| Phase 1: Annotations | 3 days | Day 1 | Day 3 | +| Phase 2: NavigationController | 5 days | Day 3 | Day 8 | +| Phase 3: Compiler | 7 days | Day 8 | Day 15 | +| Phase 4: Testing | 7 days | Day 15 | Day 22 | +| Phase 5: Documentation | 3 days | Day 22 | Day 25 | +| **Total** | **~5 weeks** | | | + +### Milestones + +- **M1** (End Week 1): Annotations and API design complete +- **M2** (End Week 2): NavigationController implementation complete +- **M3** (End Week 3): Code generation complete and tested +- **M4** (End Week 4): Sample app and integration tests complete +- **M5** (End Week 5): Documentation and release ready + +### Dependencies + +- No external dependencies required +- No breaking changes to existing code +- Can be developed incrementally +- Each phase can be reviewed independently + +--- + +## Alternatives Considered + +### 1. API Design: Annotation Approach + +| Option | Pros | Cons | Decision | +| ------------------------------- | -------------------------------------------------- | -------------------------------------------- | --------------- | +| **Optional parameter** (chosen) | Backwards compatible, consistent with args, simple | None significant | ✅ **Selected** | +| Separate @UiEntryWithResult | Cleaner separation, no risk of confusion | Not backwards compatible, duplication | ❌ Rejected | +| Builder pattern | Flexible | Too different from existing pattern, verbose | ❌ Rejected | + +### 2. Result Callback API + +| Option | Pros | Cons | Decision | +| ----------------------------------------- | ----------------------------------------- | ---------------------------------- | --------------- | +| **NavigationController methods** (chosen) | Centralized, consistent, easy to discover | None significant | ✅ **Selected** | +| ResultCallback parameter | Explicit in function signature | Verbose, not idiomatic for Compose | ❌ Rejected | +| Composition local | Consistent with LocalArgs | Less discoverable, awkward API | ❌ Rejected | + +### 3. Caller API + +| Option | Pros | Cons | Decision | +| -------------------------------- | -------------------------- | ------------------------------------------------ | --------------- | +| **navigateForResult()** (chosen) | Explicit intent, type-safe | New method to learn | ✅ **Selected** | +| Callback on navigateTo() | Single method | Overloading complexity, optional param confusion | ❌ Rejected | +| Entry provides callback | Self-contained | Entry creation becomes complex, hard to test | ❌ Rejected | + +### 4. Cancellation Handling + +| Option | Pros | Cons | Decision | +| ---------------------------- | ------------------------------ | -------------------------------- | --------------- | +| **Nullable result** (chosen) | Simple, common Android pattern | null can be ambiguous | ✅ **Selected** | +| Sealed class Result | Explicit success/failure | Verbose, boilerplate | ❌ Rejected | +| Separate callbacks | Clear separation | Two callbacks to manage, verbose | ❌ Rejected | + +### 5. Multi-Module Result Support + +| Option | Pros | Cons | Decision | +| ----------------------------------- | -------------------------------- | ------------------------------------ | --------------- | +| **DestinationWithResult interface** | Type-safe, compile-time checking | Destination must implement interface | ✅ **Selected** | +| Runtime result type registration | No interface needed | Runtime errors, not type-safe | ❌ Rejected | +| Result in navigation module | Centralized | Coupling, harder to maintain | ❌ Rejected | + +### 6. Implementation for Fragments + +| Option | Pros | Cons | Decision | +| ----------------------- | ---------------------------------------------- | ----------------------------- | --------------- | +| **Fragment Result API** | Native Android support, survives process death | Fragment-specific | ✅ **Selected** | +| Custom callback storage | Unified with Composable | Doesn't survive process death | ❌ Rejected | +| Activity Result API | Standard pattern | Requires Activity, complex | ❌ Rejected | + +### 7. Implementation for Composables + +| Option | Pros | Cons | Decision | +| -------------------------------------------- | ------------------------------- | ------------------------------- | --------------------- | +| **Callback storage in NavigationController** | Simple, works well with Compose | Doesn't survive process death | ✅ **Selected** | +| SavedStateHandle | Survives process death | Complex, requires serialization | ⚠️ Future enhancement | +| Navigation component SavedState | Native support | Limited to Navigation component | ❌ Rejected | + +--- + +## Open Questions + +### 1. Process Death Handling + +**Question**: Should we support result delivery after process death for Composable entries? + +**Current State**: + +- Fragment entries use Fragment Result API (survives process death) +- Composable entries use callback storage (does NOT survive process death) + +**Options**: + +- **Option A** (current): Document limitation, acceptable for most use cases +- **Option B**: Use SavedStateHandle for Composable results (complex, future enhancement) +- **Option C**: Only support results for Fragment implementation type + +**Recommendation**: Start with Option A, evaluate Option B in future versions based on user feedback. + +### 2. Activity Result Contracts Integration + +**Question**: Should we provide integration with Activity Result Contracts for system features? + +**Use Case**: Camera, gallery, permissions, etc. + +**Options**: + +- **Option A**: Not in initial release (users can call Activity Result API directly) +- **Option B**: Provide wrapper utilities for common contracts +- **Option C**: Full Activity Result Contract support via custom destination type + +**Recommendation**: Option A initially, Option B as follow-up feature. + +### 3. Multiple Results + +**Question**: Should we support streaming multiple results (e.g., progress updates)? + +**Use Case**: File upload with progress, multi-select picker + +**Options**: + +- **Option A**: Not supported, use SharedViewModel or event bus for complex scenarios +- **Option B**: Add `resultFlow: Flow` variant +- **Option C**: Support both single and streaming results + +**Recommendation**: Option A initially, collect real-world use cases before adding complexity. + +### 4. Result Validation + +**Question**: Should we validate result types at runtime? + +**Current State**: Compile-time checking via KSP + +**Options**: + +- **Option A**: Compile-time only (current) +- **Option B**: Add runtime type checking and throw if mismatch +- **Option C**: Add runtime checking with logging but don't throw + +**Recommendation**: Option A (compile-time is sufficient), Option B if we see type mismatches in production. + +### 5. Destination Result Type Declaration + +**Question**: How should destinations declare their result type? + +**Current Proposal**: `DestinationWithResult` interface + +**Options**: + +- **Option A** (current): Interface with resultType property +- **Option B**: Annotation on destination class +- **Option C**: Compile-time sealed hierarchy + +**Recommendation**: Option A (most flexible and type-safe). + +### 6. Nested Navigate-For-Result + +**Question**: What happens when screen A → screen B (for result) → screen C (for result)? + +**Scenarios**: + +- Screen B navigates for result before returning its own result +- Multiple levels of result-returning screens + +**Options**: + +- **Option A**: Support naturally (each screen tracks its own result callback) +- **Option B**: Disallow nested results (runtime error) +- **Option C**: Support with explicit parent result forwarding + +**Recommendation**: Option A - should work naturally with callback stacking. + +### 7. Configuration Changes + +**Question**: How do results survive configuration changes (rotation)? + +**Current State**: + +- Fragment entries: Handled by Fragment Result API +- Composable entries: Callbacks stored in ViewModel-scoped NavigationController + +**Recommendation**: Ensure NavigationController is ViewModel-scoped in sample app documentation. + +--- + +## Security Implications + +### Data Exposure Risks + +**Risk**: Results contain sensitive data (PII, credentials, etc.) + +**Mitigation**: + +- Results are Parcelable and stay within app process +- No network transmission +- Follow same security model as navigation arguments +- Document best practices for sensitive data + +**Recommendations**: + +1. Don't pass sensitive data in results when possible +2. Use encrypted storage for sensitive data, pass only references +3. Clear result data when screen is destroyed +4. Follow Android security best practices for Parcelables + +### Data Validation + +**Risk**: Malformed or unexpected result data + +**Mitigation**: + +- Compile-time type checking ensures type safety +- Result types must be Parcelable (validated at compile-time) +- Null handling for cancellation is explicit + +**Recommendations**: + +1. Validate result data in callback before use +2. Handle null results defensively +3. Use sealed classes for result types with multiple states + +### Memory Leaks + +**Risk**: Stored callbacks could cause memory leaks + +**Mitigation**: + +- Callbacks cleared after delivery +- Callbacks cleared on cancellation +- NavigationController lifecycle-aware + +**Recommendations**: + +1. Ensure NavigationController is ViewModel-scoped +2. Document lifecycle considerations +3. Add leak detection in debug builds + +### Denial of Service + +**Risk**: Result callback queue grows unbounded + +**Mitigation**: + +- Each navigation creates single callback entry +- Callbacks removed after use +- Callback storage is bounded by navigation stack depth + +**Recommendations**: + +1. Add maximum callback storage limit +2. Add monitoring for callback queue size +3. Clear callbacks on app background + +--- + +## References + +### Android APIs + +1. **Activity Result API** + + - [Activity Result API Guide](https://developer.android.com/training/basics/intents/result) + - Pattern: Register contract, launch for result, handle callback + - Inspiration for our `navigateForResult` API + +2. **Fragment Result API** + + - [Fragment Result API Guide](https://developer.android.com/guide/fragments/communicate#fragment-result) + - Used for Fragment implementation type result delivery + - Survives process death + +3. **Navigation Component** + - [Navigation Result Handling](https://developer.android.com/guide/navigation/navigation-programmatic#returning_a_result) + - SavedStateHandle-based result passing + - Alternative approach we considered + +### Kotlin / Compose + +4. **Compose Navigation** + + - [Compose Navigation with Results](https://developer.android.com/jetpack/compose/navigation#returning_a_result) + - Use SavedStateHandle for results + - Our approach is more type-safe + +5. **Kotlin Parcelize** + - [Parcelable implementation generator](https://kotlinlang.org/docs/parcelize.html) + - All args and results must be Parcelable + +### Internal Documentation + +6. **Nibel Architecture Patterns** (memory) +7. **Nibel Project Structure** (memory) +8. **Current @UiEntry documentation** (nibel-annotations) +9. **Current @UiExternalEntry documentation** (nibel-annotations) + +--- + +## Glossary + +| Term | Definition | +| ------------------------- | --------------------------------------------------------------- | +| **Entry** | Generated navigation class for a screen (e.g., `MyScreenEntry`) | +| **ResultEntry** | Entry interface for screens that return results | +| **NavigationController** | Core navigation component injected into composables | +| **Result** | Data returned from a called screen to its caller | +| **Callback** | Function invoked when result is delivered | +| **Destination** | Lightweight navigation intent for multi-module navigation | +| **DestinationWithResult** | Destination interface for screens returning results | +| **NoResult** | Marker class for screens without results (default) | +| **Internal Entry** | Entry defined with @UiEntry (single module) | +| **External Entry** | Entry defined with @UiExternalEntry (multi-module) | +| **Implementation Type** | Fragment or Composable (how entry is rendered) | +| **Args** | Input parameters for a screen (Parcelable) | +| **KSP** | Kotlin Symbol Processing (annotation processing) | +| **Parcelable** | Android serialization interface for inter-component data | +| **Navigate-for-result** | Navigation pattern where caller receives data back | +| **Cancellation** | User exits without providing result (callback receives null) | + +--- + +## Appendix: Complete Usage Example + +### Scenario: Photo Selection Flow + +**Use Case**: User needs to select a photo from gallery for their profile. + +#### Step 1: Define Result Type + +```kotlin +// shared/models/PhotoResult.kt +@Parcelize +data class PhotoResult( + val photoUri: String, + val fileName: String, + val mimeType: String +) : Parcelable +``` + +#### Step 2: Define Destination (Multi-Module) + +```kotlin +// navigation module +object PhotoPickerDestination : DestinationWithNoArgs, DestinationWithResult +``` + +#### Step 3: Implement Photo Picker Screen + +```kotlin +// feature-photo/PhotoPickerScreen.kt +@UiExternalEntry( + type = ImplementationType.Composable, + destination = PhotoPickerDestination::class, + result = PhotoResult::class +) +@Composable +fun PhotoPickerScreen( + navigator: NavigationController, + viewModel: PhotoPickerViewModel = hiltViewModel() +) { + val photos by viewModel.photos.collectAsStateWithLifecycle() + + Scaffold( + topBar = { + TopAppBar( + title = { Text("Select Photo") }, + navigationIcon = { + IconButton(onClick = { navigator.cancelResultAndNavigateBack() }) { + Icon(Icons.Default.Close, "Cancel") + } + } + ) + } + ) { padding -> + LazyVerticalGrid( + columns = GridCells.Fixed(3), + modifier = Modifier.padding(padding) + ) { + items(photos) { photo -> + AsyncImage( + model = photo.uri, + contentDescription = null, + modifier = Modifier + .aspectRatio(1f) + .clickable { + val result = PhotoResult( + photoUri = photo.uri, + fileName = photo.fileName, + mimeType = photo.mimeType + ) + navigator.setResultAndNavigateBack(result) + } + ) + } + } + } +} +``` + +#### Step 4: Call from Profile Screen + +```kotlin +// feature-profile/ProfileScreen.kt +@UiExternalEntry( + type = ImplementationType.Fragment, + destination = ProfileScreenDestination::class +) +@Composable +fun ProfileScreen( + navigator: NavigationController, + viewModel: ProfileViewModel = hiltViewModel() +) { + val profile by viewModel.profile.collectAsStateWithLifecycle() + + Column { + AsyncImage( + model = profile.photoUri ?: R.drawable.default_avatar, + contentDescription = "Profile photo", + modifier = Modifier + .size(120.dp) + .clip(CircleShape) + ) + + Button( + onClick = { + navigator.navigateForResult(PhotoPickerDestination) { result: PhotoResult? -> + result?.let { photo -> + viewModel.updateProfilePhoto(photo.photoUri) + // Show success message + viewModel.showMessage("Photo updated successfully") + } + } + } + ) { + Text("Change Photo") + } + } +} +``` + +#### Step 5: ViewModel (Optional - for state management) + +```kotlin +// feature-profile/ProfileViewModel.kt +@HiltViewModel +class ProfileViewModel @Inject constructor( + private val profileRepository: ProfileRepository +) : ViewModel() { + + private val _profile = MutableStateFlow(Profile()) + val profile: StateFlow = _profile.asStateFlow() + + fun updateProfilePhoto(photoUri: String) { + viewModelScope.launch { + _profile.update { it.copy(photoUri = photoUri) } + profileRepository.updatePhoto(photoUri) + } + } + + fun showMessage(message: String) { + // Show snackbar or toast + } +} +``` + +### Generated Code (Reference) + +The KSP processor will generate: + +```kotlin +// Generated: PhotoPickerScreenEntry.kt +@Parcelize +class PhotoPickerScreenEntry( + override val args: NoArgs, + override val name: String, + internal var requestKey: String? = null, +) : ComposableEntry(args, name), + ResultEntry { + + override val resultType: Class + get() = PhotoResult::class.java + + @Composable + override fun ComposableContent() { + PhotoPickerScreen( + navigator = LocalNavigationController.current + ) + } + + companion object : ComposableEntryFactory { + override fun newInstance(destination: PhotoPickerDestination): ComposableEntry { + return PhotoPickerScreenEntry( + args = NoArgs, + name = buildRouteName(PhotoPickerScreenEntry::class.qualifiedName!!, NoArgs), + ) + } + } +} +``` + +### Complete Flow + +1. User taps "Change Photo" button in ProfileScreen +2. `navigator.navigateForResult(PhotoPickerDestination)` is called +3. NavigationController stores callback and navigates to PhotoPickerScreen +4. User selects a photo in PhotoPickerScreen +5. `navigator.setResultAndNavigateBack(PhotoResult(...))` is called +6. NavigationController delivers result to callback +7. ProfileViewModel updates profile with new photo +8. UI updates to show new photo + +--- + +**End of RFC** diff --git a/docs/rfcs/001-navigate-for-result/thoughts.md b/docs/rfcs/001-navigate-for-result/thoughts.md new file mode 100644 index 0000000..d3779c3 --- /dev/null +++ b/docs/rfcs/001-navigate-for-result/thoughts.md @@ -0,0 +1,379 @@ +# Thoughts & Research: Navigate-For-Result Feature + +## Initial Context + +The Nibel library currently supports type-safe navigation between screens with optional arguments, but lacks a mechanism for screens to return results to their callers. This is a common pattern in Android applications (Activity Result API, Fragment Result API) and is essential for many user flows. + +## Research Summary + +### Current Nibel Architecture + +#### 1. Annotations + +The library uses two main annotations for marking screens: + +**@UiEntry** - For internal (single-module) navigation: + +```kotlin +annotation class UiEntry( + val type: ImplementationType, + val args: KClass = NoArgs::class +) +``` + +**@UiExternalEntry** - For multi-module navigation: + +```kotlin +annotation class UiExternalEntry( + val type: ImplementationType, + val destination: KClass +) +``` + +Both support two implementation types: + +- `ImplementationType.Fragment` - Generates a Fragment wrapper +- `ImplementationType.Composable` - Generates a lightweight Composable wrapper + +#### 2. Code Generation (KSP) + +The code generation flow: + +1. `UiEntryProcessor` - Main KSP processor that handles both annotations +2. `EntryGeneratingVisitor` - Visits annotated functions and collects metadata +3. Generator classes: + - `ComposableGenerator` - Generates Composable entry classes + - `FragmentGenerator` - Generates Fragment entry classes + - `EntryFactoryProviderGenerator` - Generates factory registration for external entries + +Key metadata structure: + +```kotlin +sealed interface EntryMetadata { + val argsQualifiedName: String? + val parameters: Map +} + +data class InternalEntryMetadata(...) +data class ExternalEntryMetadata( + val destinationName: String, + val destinationPackageName: String, + val destinationQualifiedName: String, + ... +) +``` + +#### 3. Navigation Flow + +Current navigation methods in `NavigationController`: + +```kotlin +abstract fun navigateBack() +abstract fun navigateTo(entry: Entry, ...) +abstract fun navigateTo(externalDestination: ExternalDestination, ...) +``` + +Entry classes are generated with companion objects containing `newInstance()` factory methods: + +```kotlin +companion object { + fun newInstance(args: MyArgs): ComposableEntry { + return MyScreenEntry(args, name) + } +} +``` + +#### 4. Multi-Module Support + +Destinations are defined in a shared navigation module: + +- `DestinationWithNoArgs` - Simple object destinations +- `DestinationWithArgs` - Destinations with arguments + +Feature modules implement screens with `@UiExternalEntry`, referencing destinations. +The main app module depends on all features and can navigate between them. + +### Testing Infrastructure + +**Compilation Tests** (`tests/src/test/kotlin/nibel/tests/codegen/`): + +- `InternalEntryCompileTest.kt` - Tests @UiEntry code generation +- `ExternalEntryCompileTest.kt` - Tests @UiExternalEntry code generation +- `InternalEntryParamCompileTest.kt` - Tests parameter handling +- `ExternalEntryParamCompileTest.kt` - Tests external parameter handling + +Tests verify that: + +- Generated entry classes implement correct interfaces +- Companion object factory methods have correct signatures +- Argument types match between annotation and function parameters + +**Sample App** (`sample/`): + +- Multi-module structure demonstrating real-world usage +- feature-A, feature-B, feature-C independent feature modules +- Shared navigation module with destinations +- Uses Hilt for dependency injection +- State management with StateFlow and side effects via Channel + +### Key Findings + +1. **Arguments Pattern**: The existing `args` parameter is optional with default `NoArgs::class`. This provides a clear pattern for adding `result` parameter. + +2. **Type Safety**: Compile-time validation is critical. The processor validates that argument types in annotations match function parameters. + +3. **Generated Code**: Entry classes extend base interfaces (`ComposableEntry`, `FragmentEntry`). We can extend this with `ResultEntry`. + +4. **Multi-Module**: Destinations need to be extended to support result types for external entries. + +5. **No Result Mechanism**: Currently there is NO built-in result mechanism. Search for "navigateForResult", "setResult", "ResultEntry" returned no matches in nibel-runtime. + +## Requirements & Constraints + +### Must Have + +1. **Backwards Compatibility**: Existing code MUST work without changes. The Turo repository already uses Nibel. +2. **Type Safety**: Result types must be validated at compile-time. +3. **Works for both annotations**: @UiEntry and @UiExternalEntry +4. **Supports both implementation types**: Fragment and Composable +5. **Multi-module support**: Results must work across feature modules + +### Should Have + +1. **Clear API**: Easy to understand and discover +2. **Consistent**: Follows existing Nibel patterns +3. **Testable**: Easy to write tests for result handling +4. **Documentation**: Clear examples and migration guide + +### Nice to Have + +1. **Process death handling**: Results survive process death (for Fragment implementation) +2. **Lifecycle awareness**: Proper cleanup of result callbacks +3. **Debugging support**: Clear error messages for misuse + +## Design Decisions + +### 1. API Design: Optional Parameter ✅ + +**Decision**: Add optional `result` parameter to existing annotations + +**Rationale**: + +- 100% backwards compatible +- Consistent with how `args` works +- Simple and easy to understand +- No annotation duplication + +**Alternative Considered**: Separate `@UiEntryWithResult` annotation + +- Rejected: Not backwards compatible, creates duplication + +### 2. Callback API: NavigationController Methods ✅ + +**Decision**: Add methods to NavigationController: + +- `navigateForResult(entry, callback)` +- `setResultAndNavigateBack(result)` +- `cancelResultAndNavigateBack()` + +**Rationale**: + +- Centralized in one place (NavigationController) +- Easy to discover +- Consistent with existing navigation methods +- Clear intent + +**Alternative Considered**: ResultCallback as composable parameter + +- Rejected: Not idiomatic for Compose, verbose + +**Alternative Considered**: Composition local for result callback + +- Rejected: Less discoverable, awkward API + +### 3. Caller API: New navigateForResult Method ✅ + +**Decision**: Add new `navigateForResult()` method separate from `navigateTo()` + +**Rationale**: + +- Makes intent explicit (expecting a result) +- Type-safe callback parameter +- Doesn't clutter existing `navigateTo()` API +- Clear separation of concerns + +**Alternative Considered**: Optional callback parameter on `navigateTo()` + +- Rejected: Overloading complexity, unclear when result is expected + +### 4. Cancellation: Nullable Result ✅ + +**Decision**: Callback receives `T?` where `null` indicates cancellation + +**Rationale**: + +- Simple and common Android pattern (Activity Result API uses this) +- No additional types needed +- Easy to understand: `result?.let { ... }` + +**Alternative Considered**: Sealed class `Result` + +- Rejected: More verbose, adds boilerplate + +**Alternative Considered**: Separate onResult and onCancelled callbacks + +- Rejected: Two callbacks to manage, verbose + +### 5. Multi-Module: DestinationWithResult Interface ✅ + +**Decision**: Create `DestinationWithResult` interface + +**Rationale**: + +- Type-safe at compile-time +- Follows existing pattern (DestinationWithArgs) +- Clear declaration of result type +- Easy to validate in code generation + +### 6. Implementation: Fragment Result API for Fragments ✅ + +**Decision**: Use Android's Fragment Result API for Fragment implementation type + +**Rationale**: + +- Native Android support +- Survives process death +- Well-tested and documented +- Standard approach + +**Implementation Note**: For Composable implementation type, use callback storage in NavigationController (doesn't survive process death but acceptable for most use cases). + +## Implementation Strategy + +### Phase 1: Foundation + +- Add `result` parameter to annotations (default `NoResult::class`) +- Create `NoResult` marker class +- Create `ResultEntry` interface +- Create `DestinationWithResult` interface + +### Phase 2: NavigationController API + +- Add `navigateForResult()` methods +- Add `setResultAndNavigateBack()` method +- Add `cancelResultAndNavigateBack()` method +- Implement result delivery mechanism in `NibelNavigationController` + +### Phase 3: Code Generation + +- Update `EntryMetadata` to include `resultQualifiedName` +- Modify `EntryGeneratingVisitor` to extract result parameter +- Update generators to produce result-aware entry classes +- Implement result type validation + +### Phase 4: Testing + +- Create `ResultEntryCompileTest.kt` +- Add integration tests for result delivery +- Add sample app demonstrations +- Test all combinations (internal/external, fragment/composable, args/no-args, result/no-result) + +### Phase 5: Documentation + +- Update KDoc for all public APIs +- Write migration guide +- Update README with examples +- Create comprehensive usage guide + +## Open Questions + +### 1. Process Death for Composables + +Should Composable entries support result delivery after process death? + +**Current Approach**: No (use callback storage, doesn't survive process death) +**Future Enhancement**: Use SavedStateHandle for Composable results (complex) + +**Decision**: Start without process death support for Composables, evaluate based on user feedback. + +### 2. Activity Result Contracts + +Should we integrate with Activity Result Contracts for system features (camera, gallery, permissions)? + +**Decision**: Not in initial release. Users can call Activity Result API directly. Consider as future enhancement. + +### 3. Multiple Results / Streaming + +Should we support streaming multiple results (e.g., progress updates)? + +**Decision**: No. For complex scenarios, use SharedViewModel or event bus. Keep initial feature simple. + +### 4. Nested Navigate-For-Result + +What happens when screen A → B (for result) → C (for result)? + +**Expected Behavior**: Should work naturally with callback stacking. Each screen tracks its own result callback. + +**Decision**: Support naturally, test in integration tests. + +## Success Metrics + +1. **Zero Breaking Changes**: All existing tests pass without modification +2. **Comprehensive Testing**: New feature has >90% test coverage +3. **Clear Documentation**: Usage examples for all scenarios +4. **Performance**: No significant performance impact on navigation +5. **Adoption**: Feature is easy to adopt incrementally + +## References + +- Activity Result API: https://developer.android.com/training/basics/intents/result +- Fragment Result API: https://developer.android.com/guide/fragments/communicate#fragment-result +- Navigation Result Handling: https://developer.android.com/guide/navigation/navigation-programmatic#returning_a_result +- Nibel Architecture Patterns (memory) +- Nibel Project Structure (memory) + +## Timeline Estimate + +- Phase 1 (Foundation): 3 days +- Phase 2 (NavigationController): 5 days +- Phase 3 (Code Generation): 7 days +- Phase 4 (Testing): 7 days +- Phase 5 (Documentation): 3 days + +**Total: ~5 weeks** + +## Risks & Mitigation + +### Risk: Breaking Existing Code + +**Mitigation**: Optional parameter with default value, extensive regression testing + +### Risk: Type Safety Gaps + +**Mitigation**: Compile-time validation via KSP, comprehensive type checking tests + +### Risk: Process Death Edge Cases + +**Mitigation**: Document limitations for Composable implementation, use Fragment Result API for Fragment implementation + +### Risk: Complex Multi-Module Setup + +**Mitigation**: Clear examples in sample app, comprehensive documentation + +### Risk: Memory Leaks from Callbacks + +**Mitigation**: Clear callbacks after use, ensure NavigationController is ViewModel-scoped, add leak detection + +## Next Steps + +1. Review and approve RFC with team +2. Create tracking issue for implementation +3. Set up project board with phases +4. Begin Phase 1 implementation +5. Regular check-ins on progress + +--- + +**Document Status**: Research Complete ✅ +**Last Updated**: 2025-10-28 +**Ready for**: RFC Review diff --git a/nibel-annotations/src/main/kotlin/nibel/annotations/Destination.kt b/nibel-annotations/src/main/kotlin/nibel/annotations/Destination.kt index a9d76d6..17a5c02 100644 --- a/nibel-annotations/src/main/kotlin/nibel/annotations/Destination.kt +++ b/nibel-annotations/src/main/kotlin/nibel/annotations/Destination.kt @@ -31,6 +31,68 @@ interface DestinationWithArgs : ExternalDestination { val args: A } +/** + * An external destination type for screens that return a result. + * + * This interface should be implemented by destinations whose associated screens return + * a result to their caller. When a destination implements this interface, callers can + * use `NavigationController.navigateForResult()` to receive type-safe results. + * + * The result type [TResult] must be [Parcelable] and match the `result` parameter + * specified in the associated [UiExternalEntry] annotation. + * + * Example: + * ``` + * @Parcelize + * data class PhotoResult(val uri: String) : Parcelable + * + * // Destination with no args but with result + * object PhotoPickerDestination : DestinationWithNoArgs, DestinationWithResult + * + * // Destination with both args and result + * data class EditPhotoDestination( + * override val args: EditPhotoArgs + * ) : DestinationWithArgs, DestinationWithResult + * + * // Associated screen + * @UiExternalEntry( + * type = ImplementationType.Composable, + * destination = PhotoPickerDestination::class, + * result = PhotoResult::class + * ) + * @Composable + * fun PhotoPickerScreen(navigator: NavigationController) { + * // ... photo selection UI + * navigator.setResultAndNavigateBack(PhotoResult(selectedUri)) + * } + * + * // Caller from another feature module + * navigator.navigateForResult(PhotoPickerDestination) { result: PhotoResult? -> + * result?.let { /* handle result */ } + * } + * ``` + * + * @param TResult The type of result returned by the screen (must be [Parcelable]) + * + * @see UiExternalEntry + * @see DestinationWithNoArgs + * @see DestinationWithArgs + */ +interface DestinationWithResult : ExternalDestination { + /** + * The runtime [Class] of the result type. + * Used for type checking and result delivery at runtime. + * + * This property is typically implemented by the code generator or can be + * provided as a default implementation. + */ + val resultType: Class + get() = throw NotImplementedError( + "resultType must be implemented. " + + "This is typically generated by the compiler for destinations with results.", + ) +} + /** * A base type for destinations for internal single-module navigation. */ diff --git a/nibel-annotations/src/main/kotlin/nibel/annotations/UiEntry.kt b/nibel-annotations/src/main/kotlin/nibel/annotations/UiEntry.kt index 3a74d76..1fab135 100644 --- a/nibel-annotations/src/main/kotlin/nibel/annotations/UiEntry.kt +++ b/nibel-annotations/src/main/kotlin/nibel/annotations/UiEntry.kt @@ -4,6 +4,7 @@ import android.os.Parcelable import nibel.annotations.ImplementationType.Composable import nibel.annotations.ImplementationType.Fragment import nibel.runtime.NoArgs +import nibel.runtime.NoResult import kotlin.annotation.AnnotationTarget.FUNCTION import kotlin.reflect.KClass @@ -136,4 +137,38 @@ annotation class UiEntry( * See [UiEntry]. */ val args: KClass = NoArgs::class, + /** + * Optional `Parcelable` result that the screen returns to its caller. + * If not specified, the screen is considered to not return a result. + * + * When specified, the generated entry class will implement `ResultEntry` interface, + * and callers can use `NavigationController.navigateForResult()` to receive the result. + * + * The screen can return a result using `NavigationController.setResultAndNavigateBack(result)` + * or cancel without result using `NavigationController.cancelResultAndNavigateBack()`. + * + * Example: + * ``` + * @Parcelize + * data class PhotoResult(val uri: String) : Parcelable + * + * @UiEntry( + * type = ImplementationType.Composable, + * result = PhotoResult::class + * ) + * @Composable + * fun PhotoPickerScreen(navigator: NavigationController) { + * // ... photo selection UI + * navigator.setResultAndNavigateBack(PhotoResult(selectedUri)) + * } + * + * // Caller + * navigator.navigateForResult(PhotoPickerScreenEntry.newInstance()) { result: PhotoResult? -> + * result?.let { /* handle photo selection */ } + * } + * ``` + * + * See [UiEntry], NavigationController. + */ + val result: KClass = NoResult::class, ) diff --git a/nibel-annotations/src/main/kotlin/nibel/annotations/UiExternalEntry.kt b/nibel-annotations/src/main/kotlin/nibel/annotations/UiExternalEntry.kt index 19c50d3..cf44d75 100644 --- a/nibel-annotations/src/main/kotlin/nibel/annotations/UiExternalEntry.kt +++ b/nibel-annotations/src/main/kotlin/nibel/annotations/UiExternalEntry.kt @@ -1,7 +1,9 @@ package nibel.annotations +import android.os.Parcelable import nibel.annotations.ImplementationType.Composable import nibel.annotations.ImplementationType.Fragment +import nibel.runtime.NoResult import kotlin.annotation.AnnotationTarget.FUNCTION import kotlin.reflect.KClass @@ -178,4 +180,46 @@ annotation class UiExternalEntry( * See [UiExternalEntry], [ExternalDestination]. */ val destination: KClass, + /** + * Optional `Parcelable` result that the screen returns to its caller. + * If not specified, the screen is considered to not return a result. + * + * When specified, the generated entry class will implement `ResultEntry` interface, + * and callers can use `NavigationController.navigateForResult()` with the destination to receive the result. + * + * The destination should also implement `DestinationWithResult` to enable type-safe + * multi-module result navigation. + * + * The screen can return a result using `NavigationController.setResultAndNavigateBack(result)` + * or cancel without result using `NavigationController.cancelResultAndNavigateBack()`. + * + * Example: + * ``` + * // navigation module + * @Parcelize + * data class PhotoResult(val uri: String) : Parcelable + * + * object PhotoPickerDestination : DestinationWithNoArgs, DestinationWithResult + * + * // feature module + * @UiExternalEntry( + * type = ImplementationType.Composable, + * destination = PhotoPickerDestination::class, + * result = PhotoResult::class + * ) + * @Composable + * fun PhotoPickerScreen(navigator: NavigationController) { + * // ... photo selection UI + * navigator.setResultAndNavigateBack(PhotoResult(selectedUri)) + * } + * + * // Caller (from another feature module) + * navigator.navigateForResult(PhotoPickerDestination) { result: PhotoResult? -> + * result?.let { /* handle photo selection */ } + * } + * ``` + * + * See [UiExternalEntry], [DestinationWithResult], NavigationController. + */ + val result: KClass = NoResult::class, ) diff --git a/nibel-compiler/src/main/kotlin/nibel/compiler/generator/AbstractEntryGeneratingVisitor.kt b/nibel-compiler/src/main/kotlin/nibel/compiler/generator/AbstractEntryGeneratingVisitor.kt index 29586d7..51583e1 100644 --- a/nibel-compiler/src/main/kotlin/nibel/compiler/generator/AbstractEntryGeneratingVisitor.kt +++ b/nibel-compiler/src/main/kotlin/nibel/compiler/generator/AbstractEntryGeneratingVisitor.kt @@ -12,6 +12,7 @@ import com.google.devtools.ksp.symbol.Modifier import nibel.annotations.DestinationWithArgs import nibel.annotations.DestinationWithNoArgs import nibel.runtime.NoArgs +import nibel.runtime.NoResult abstract class AbstractEntryGeneratingVisitor( private val resolver: Resolver, @@ -31,6 +32,8 @@ abstract class AbstractEntryGeneratingVisitor( val reference = declaration.findDestinationSuperType(symbol) ?: return null + val resultQualifiedName = parseResultQualifiedName(symbol) + val ref = reference.toString() return when { ref.startsWith(DestinationWithArgs::class.simpleName!!) -> { @@ -52,6 +55,7 @@ abstract class AbstractEntryGeneratingVisitor( destinationPackageName = destinationPackageName.asString(), destinationQualifiedName = destinationClassName.asString(), argsQualifiedName = argsClassName, + resultQualifiedName = resultQualifiedName, parameters = emptyMap(), ) } @@ -62,6 +66,7 @@ abstract class AbstractEntryGeneratingVisitor( destinationPackageName = destinationPackageName.asString(), destinationQualifiedName = destinationClassName.asString(), argsQualifiedName = null, + resultQualifiedName = resultQualifiedName, parameters = emptyMap(), ) @@ -76,15 +81,19 @@ abstract class AbstractEntryGeneratingVisitor( return null } + val resultQualifiedName = parseResultQualifiedName(symbol) + val argsClassName = arg.declaration.qualifiedName!!.asString() return if (argsClassName == NoArgs::class.qualifiedName) { InternalEntryMetadata( argsQualifiedName = null, + resultQualifiedName = resultQualifiedName, parameters = emptyMap(), ) } else { InternalEntryMetadata( argsQualifiedName = argsClassName, + resultQualifiedName = resultQualifiedName, parameters = emptyMap(), ) } @@ -132,6 +141,24 @@ abstract class AbstractEntryGeneratingVisitor( return true } + private fun KSClassDeclaration.isCorrectResultDeclaration(symbol: KSNode): Boolean { + if (Modifier.DATA !in modifiers && classKind != ClassKind.OBJECT) { + logger.error( + message = "Result are allowed to be only 'data class' or 'object'.", + symbol = symbol, + ) + return false + } + if (typeParameters.isNotEmpty()) { + logger.error( + message = "Result declarations are not allowed to have generic type parameters.", + symbol = symbol, + ) + return false + } + return true + } + private fun KSClassDeclaration.findDestinationSuperType(symbol: KSNode): KSTypeReference? { val destinationSuperType = superTypes.toList() .find { it.toString().startsWith("DestinationWith") } @@ -146,4 +173,18 @@ abstract class AbstractEntryGeneratingVisitor( } return destinationSuperType } + + private fun Arguments.parseResultQualifiedName(symbol: KSNode): String? { + val resultArg = this["result"] as? KSType ?: return null + val resultDeclaration = resultArg.declaration as? KSClassDeclaration ?: return null + if (!resultDeclaration.isCorrectResultDeclaration(symbol)) { + return null + } + val resultClassName = resultArg.declaration.qualifiedName!!.asString() + return if (resultClassName == NoResult::class.qualifiedName) { + null + } else { + resultClassName + } + } } diff --git a/nibel-compiler/src/main/kotlin/nibel/compiler/generator/ComposableGenerator.kt b/nibel-compiler/src/main/kotlin/nibel/compiler/generator/ComposableGenerator.kt index 10e94a0..327d40d 100644 --- a/nibel-compiler/src/main/kotlin/nibel/compiler/generator/ComposableGenerator.kt +++ b/nibel-compiler/src/main/kotlin/nibel/compiler/generator/ComposableGenerator.kt @@ -70,6 +70,7 @@ class ComposableGenerator( packageName = packageName, composableHolderName = composableHolderName, argsQualifiedName = metadata.argsQualifiedName, + resultQualifiedName = metadata.resultQualifiedName, composableContent = composableContent, composableEntryFactory = composableEntryFactory, ) @@ -79,6 +80,7 @@ class ComposableGenerator( packageName: String, composableHolderName: String, argsQualifiedName: String?, + resultQualifiedName: String?, composableContent: String, composableEntryFactory: String, ) { @@ -98,6 +100,7 @@ class ComposableGenerator( composableHolderName = composableHolderName, composableContent = composableContent, argsQualifiedName = argsQualifiedName, + resultQualifiedName = resultQualifiedName, composableEntryFactory = composableEntryFactory, ) } else { @@ -105,6 +108,7 @@ class ComposableGenerator( packageName = packageName, composableHolderName = composableHolderName, composableContent = composableContent, + resultQualifiedName = resultQualifiedName, composableEntryFactory = composableEntryFactory, ) } diff --git a/nibel-compiler/src/main/kotlin/nibel/compiler/generator/EntryMetadata.kt b/nibel-compiler/src/main/kotlin/nibel/compiler/generator/EntryMetadata.kt index c3926fb..74bcb55 100644 --- a/nibel-compiler/src/main/kotlin/nibel/compiler/generator/EntryMetadata.kt +++ b/nibel-compiler/src/main/kotlin/nibel/compiler/generator/EntryMetadata.kt @@ -2,6 +2,7 @@ package nibel.compiler.generator sealed interface EntryMetadata { val argsQualifiedName: String? + val resultQualifiedName: String? val parameters: Map } @@ -10,10 +11,12 @@ data class ExternalEntryMetadata( val destinationPackageName: String, val destinationQualifiedName: String, override val argsQualifiedName: String?, + override val resultQualifiedName: String?, override val parameters: Map, ) : EntryMetadata data class InternalEntryMetadata( override val argsQualifiedName: String?, + override val resultQualifiedName: String?, override val parameters: Map, ) : EntryMetadata diff --git a/nibel-compiler/src/main/kotlin/nibel/compiler/generator/FragmentGenerator.kt b/nibel-compiler/src/main/kotlin/nibel/compiler/generator/FragmentGenerator.kt index 26ab605..fdf609b 100644 --- a/nibel-compiler/src/main/kotlin/nibel/compiler/generator/FragmentGenerator.kt +++ b/nibel-compiler/src/main/kotlin/nibel/compiler/generator/FragmentGenerator.kt @@ -71,6 +71,8 @@ class FragmentGenerator( generateFragmentFile( packageName = packageName, fragmentName = fragmentName, + argsQualifiedName = metadata.argsQualifiedName, + resultQualifiedName = metadata.resultQualifiedName, composableContent = composableContent, fragmentEntryFactory = fragmentEntryFactory, ) @@ -79,6 +81,8 @@ class FragmentGenerator( private fun generateFragmentFile( packageName: String, fragmentName: String, + argsQualifiedName: String?, + resultQualifiedName: String?, composableContent: String, fragmentEntryFactory: String, ) { @@ -95,6 +99,8 @@ class FragmentGenerator( val content = fragmentEntryTemplate( packageName = packageName, fragmentName = fragmentName, + argsQualifiedName = argsQualifiedName, + resultQualifiedName = resultQualifiedName, composableContent = composableContent, fragmentEntryFactory = fragmentEntryFactory, ) diff --git a/nibel-compiler/src/main/kotlin/nibel/compiler/template/ComposableEntryTemplate.kt b/nibel-compiler/src/main/kotlin/nibel/compiler/template/ComposableEntryTemplate.kt index 8a6a982..f4f38b9 100644 --- a/nibel-compiler/src/main/kotlin/nibel/compiler/template/ComposableEntryTemplate.kt +++ b/nibel-compiler/src/main/kotlin/nibel/compiler/template/ComposableEntryTemplate.kt @@ -4,6 +4,7 @@ fun composableEntryWithArgsTemplate( packageName: String, composableHolderName: String, argsQualifiedName: String, + resultQualifiedName: String?, composableContent: String, composableEntryFactory: String, ) = """ @@ -14,19 +15,20 @@ import androidx.compose.runtime.Composable import nibel.runtime.ComposableEntry import nibel.runtime.ComposableEntryFactory import kotlinx.parcelize.Parcelize +${if (resultQualifiedName != null) "import nibel.runtime.ResultEntry" else ""} @Parcelize class $composableHolderName( override val args: $argsQualifiedName, override val name: String, -) : ComposableEntry<$argsQualifiedName>(args, name) { +) : ComposableEntry<$argsQualifiedName>(args, name)${if (resultQualifiedName != null) ", ResultEntry<$argsQualifiedName, $resultQualifiedName>" else ""} { @Composable override fun ComposableContent() { $composableContent } -$composableEntryFactory +${if (resultQualifiedName != null) " override val resultType: Class<$resultQualifiedName> = $resultQualifiedName::class.java\n" else ""}$composableEntryFactory } """.trimIndent() @@ -34,6 +36,7 @@ $composableEntryFactory fun composableEntryWithNoArgsTemplate( packageName: String, composableHolderName: String, + resultQualifiedName: String?, composableContent: String, composableEntryFactory: String, ) = """ @@ -44,18 +47,19 @@ import androidx.compose.runtime.Composable import nibel.runtime.ComposableEntry import nibel.runtime.ComposableEntryFactory import kotlinx.parcelize.Parcelize +${if (resultQualifiedName != null) "import nibel.runtime.ResultEntry" else ""} @Parcelize class $composableHolderName( override val name: String, -) : ComposableEntry(null, name) { +) : ComposableEntry(null, name)${if (resultQualifiedName != null) ", ResultEntry" else ""} { @Composable override fun ComposableContent() { $composableContent } -$composableEntryFactory +${if (resultQualifiedName != null) " override val resultType: Class<$resultQualifiedName> = $resultQualifiedName::class.java\n" else ""}$composableEntryFactory } """.trimIndent() diff --git a/nibel-compiler/src/main/kotlin/nibel/compiler/template/FragmentEntryTemplates.kt b/nibel-compiler/src/main/kotlin/nibel/compiler/template/FragmentEntryTemplates.kt index 2379a59..1d57bb3 100644 --- a/nibel-compiler/src/main/kotlin/nibel/compiler/template/FragmentEntryTemplates.kt +++ b/nibel-compiler/src/main/kotlin/nibel/compiler/template/FragmentEntryTemplates.kt @@ -3,25 +3,34 @@ package nibel.compiler.template fun fragmentEntryTemplate( packageName: String, fragmentName: String, + argsQualifiedName: String?, + resultQualifiedName: String?, composableContent: String, fragmentEntryFactory: String, ) = """ package $packageName +import android.os.Parcelable import androidx.compose.runtime.Composable import nibel.runtime.asNibelArgs import nibel.runtime.ComposableFragment import nibel.runtime.FragmentEntryFactory import nibel.runtime.FragmentEntry +${if (resultQualifiedName != null) "import nibel.runtime.ResultEntry" else ""} -class $fragmentName : ComposableFragment() { +class $fragmentName : ComposableFragment()${if (resultQualifiedName != null) { + val argsType = argsQualifiedName ?: "Parcelable" + ", ResultEntry<$argsType, $resultQualifiedName>" +} else { + "" +}} { @Composable override fun ComposableContent() { $composableContent } -$fragmentEntryFactory +${if (resultQualifiedName != null) " override val resultType: Class<$resultQualifiedName> = $resultQualifiedName::class.java\n" else ""}$fragmentEntryFactory } """.trimIndent() diff --git a/nibel-runtime/src/main/kotlin/nibel/runtime/ComposableEntry.kt b/nibel-runtime/src/main/kotlin/nibel/runtime/ComposableEntry.kt index 922db05..99d92a9 100644 --- a/nibel-runtime/src/main/kotlin/nibel/runtime/ComposableEntry.kt +++ b/nibel-runtime/src/main/kotlin/nibel/runtime/ComposableEntry.kt @@ -35,6 +35,3 @@ abstract class ComposableEntry( } } } - -@Suppress("UnusedParameter") -fun buildRouteName(base: String, args: Parcelable? = null): String = base diff --git a/nibel-runtime/src/main/kotlin/nibel/runtime/ComposableFragment.kt b/nibel-runtime/src/main/kotlin/nibel/runtime/ComposableFragment.kt index 54ba59d..4092b8a 100644 --- a/nibel-runtime/src/main/kotlin/nibel/runtime/ComposableFragment.kt +++ b/nibel-runtime/src/main/kotlin/nibel/runtime/ComposableFragment.kt @@ -34,11 +34,13 @@ abstract class ComposableFragment : Fragment() { setViewCompositionStrategy(ViewCompositionStrategy.DisposeOnViewTreeLifecycleDestroyed) @Suppress("DEPRECATION") val args = arguments?.get(Nibel.argsKey) as? Parcelable? + val requestKey = arguments?.getRequestKey() setContent { Nibel.RootDelegate.Content { CompositionLocalProvider( LocalImplementationType provides ImplementationType.Fragment, + LocalRequestKey provides requestKey, ) { Nibel.NavigationDelegate.Content(rootArgs = args) { ComposableContent() diff --git a/nibel-runtime/src/main/kotlin/nibel/runtime/ComposeNavigationDelegate.kt b/nibel-runtime/src/main/kotlin/nibel/runtime/ComposeNavigationDelegate.kt index 74b4fa9..35122aa 100644 --- a/nibel-runtime/src/main/kotlin/nibel/runtime/ComposeNavigationDelegate.kt +++ b/nibel-runtime/src/main/kotlin/nibel/runtime/ComposeNavigationDelegate.kt @@ -56,6 +56,12 @@ open class ComposeNavigationDelegate : NavigationDelegate { val LocalArgs = compositionLocalOf { null } + +/** + * Provides request key for result-based navigation or `null` if not navigated via navigateForResult. + */ +val LocalRequestKey = compositionLocalOf { + null +} diff --git a/nibel-runtime/src/main/kotlin/nibel/runtime/NavigationController.kt b/nibel-runtime/src/main/kotlin/nibel/runtime/NavigationController.kt index 4393506..21bebf4 100644 --- a/nibel-runtime/src/main/kotlin/nibel/runtime/NavigationController.kt +++ b/nibel-runtime/src/main/kotlin/nibel/runtime/NavigationController.kt @@ -1,5 +1,6 @@ package nibel.runtime +import android.os.Parcelable import nibel.annotations.ExternalDestination import nibel.annotations.UiEntry import nibel.annotations.UiExternalEntry @@ -46,4 +47,125 @@ abstract class NavigationController( fragmentSpec: FragmentSpec<*> = this.fragmentSpec, composeSpec: ComposeSpec<*> = this.composeSpec, ) + + /** + * Navigate to a screen and receive a typed result when it completes. + * + * The screen being navigated to must be annotated with a `result` parameter in [UiEntry] + * or [UiExternalEntry], and the entry must implement [ResultEntry]. + * + * The callback will be invoked with: + * - Non-null result if the screen calls [setResultAndNavigateBack] + * - Null if the screen calls [cancelResultAndNavigateBack] or user presses back + * + * Example: + * ``` + * navigator.navigateForResult(PhotoPickerEntry.newInstance(args)) { result: PhotoResult? -> + * result?.let { handlePhoto(it.uri) } + * } + * ``` + * + * @param entry The entry to navigate to (must implement [ResultEntry]) + * @param fragmentSpec Fragment specification for Fragment implementation types + * @param composeSpec Compose specification for Composable implementation types + * @param callback Callback invoked with result (null if cancelled) + * + * @see setResultAndNavigateBack + * @see cancelResultAndNavigateBack + * @see UiEntry + * @see ResultEntry + */ + abstract fun navigateForResult( + entry: Entry, + fragmentSpec: FragmentSpec<*> = this.fragmentSpec, + composeSpec: ComposeSpec<*> = this.composeSpec, + callback: (TResult?) -> Unit, + ) + + /** + * Navigate to an external destination and receive a typed result when it completes. + * + * The destination's associated screen must be annotated with a `result` parameter in + * [UiExternalEntry]. The destination should implement [nibel.annotations.DestinationWithResult] + * for type safety. + * + * The callback will be invoked with: + * - Non-null result if the screen calls [setResultAndNavigateBack] + * - Null if the screen calls [cancelResultAndNavigateBack] or user presses back + * + * Example: + * ``` + * navigator.navigateForResult(PhotoPickerDestination) { result: PhotoResult? -> + * result?.let { handlePhoto(it.uri) } + * } + * ``` + * + * @param externalDestination The external destination to navigate to + * @param fragmentSpec Fragment specification for Fragment implementation types + * @param composeSpec Compose specification for Composable implementation types + * @param callback Callback invoked with result (null if cancelled) + * + * @see setResultAndNavigateBack + * @see cancelResultAndNavigateBack + * @see UiExternalEntry + * @see nibel.annotations.DestinationWithResult + */ + abstract fun navigateForResult( + externalDestination: ExternalDestination, + fragmentSpec: FragmentSpec<*> = this.fragmentSpec, + composeSpec: ComposeSpec<*> = this.composeSpec, + callback: (TResult?) -> Unit, + ) + + /** + * Set a result and navigate back to the previous screen. + * + * The result will be delivered to the callback provided in [navigateForResult]. + * This method can only be called from screens annotated with a `result` parameter + * in [UiEntry] or [UiExternalEntry]. + * + * Example: + * ``` + * @UiEntry( + * type = ImplementationType.Composable, + * result = PhotoResult::class + * ) + * @Composable + * fun PhotoPickerScreen(navigator: NavigationController) { + * // ... photo selection UI + * Button(onClick = { + * navigator.setResultAndNavigateBack(PhotoResult(selectedUri)) + * }) { + * Text("Select") + * } + * } + * ``` + * + * @param result The result to return to the caller + * + * @see navigateForResult + * @see cancelResultAndNavigateBack + */ + abstract fun setResultAndNavigateBack(result: TResult) + + /** + * Navigate back without setting a result (equivalent to user pressing back button). + * + * The [navigateForResult] callback will receive null, indicating cancellation. + * This method can be called from any screen. + * + * Example: + * ``` + * @Composable + * fun PhotoPickerScreen(navigator: NavigationController) { + * Button(onClick = { navigator.cancelResultAndNavigateBack() }) { + * Text("Cancel") + * } + * } + * ``` + * + * @see navigateForResult + * @see setResultAndNavigateBack + */ + abstract fun cancelResultAndNavigateBack() } diff --git a/nibel-runtime/src/main/kotlin/nibel/runtime/NibelArgs.kt b/nibel-runtime/src/main/kotlin/nibel/runtime/NibelArgs.kt index d2e0b3f..da56e02 100644 --- a/nibel-runtime/src/main/kotlin/nibel/runtime/NibelArgs.kt +++ b/nibel-runtime/src/main/kotlin/nibel/runtime/NibelArgs.kt @@ -11,16 +11,33 @@ import androidx.lifecycle.SavedStateHandle */ const val NIBEL_ARGS = "nibel_args" +/** + * Key for request ID used in result-based navigation. + */ +const val NIBEL_REQUEST_KEY = "nibel_request_key" + /** * Convert [Parcelable] args to a [Bundle]. */ fun Parcelable.asNibelArgs() = Bundle().apply { putParcelable(Nibel.argsKey, this@asNibelArgs) } +/** + * Add request key to a [Bundle] for result-based navigation. + */ +fun Bundle.putRequestKey(key: String?) { + key?.let { putString(NIBEL_REQUEST_KEY, it) } +} + /** * Retrieve args from a [SavedStateHandle]. */ fun SavedStateHandle.getNibelArgs(): A? = get(Nibel.argsKey) +/** + * Retrieve request key from a [SavedStateHandle]. + */ +fun SavedStateHandle.getRequestKey(): String? = get(NIBEL_REQUEST_KEY) + /** * Retrieve args from a [Bundle]. */ @@ -31,3 +48,8 @@ inline fun Bundle.getNibelArgs(): A? = @Suppress("DEPRECATION") getParcelable(Nibel.argsKey) } + +/** + * Retrieve request key from a [Bundle]. + */ +fun Bundle.getRequestKey(): String? = getString(NIBEL_REQUEST_KEY) diff --git a/nibel-runtime/src/main/kotlin/nibel/runtime/NibelNavigationController.kt b/nibel-runtime/src/main/kotlin/nibel/runtime/NibelNavigationController.kt index d671a29..26223ff 100644 --- a/nibel-runtime/src/main/kotlin/nibel/runtime/NibelNavigationController.kt +++ b/nibel-runtime/src/main/kotlin/nibel/runtime/NibelNavigationController.kt @@ -1,9 +1,11 @@ package nibel.runtime +import android.os.Parcelable import androidx.activity.OnBackPressedDispatcher import androidx.fragment.app.FragmentManager import androidx.navigation.NavController import nibel.annotations.ExternalDestination +import java.util.UUID /** * Default implementation of [NavigationController]. Depending on circumstances, relies on 2 tools @@ -26,6 +28,23 @@ open class NibelNavigationController( protected val composeNavigationContext = ComposeNavigationContext(internalNavController, exploredEntries) + // Note: Result callbacks are now stored in ResultCallbackRegistry singleton + // to persist across Fragment/NavigationController instance recreation + + /** + * Storage for the current screen's request key (if navigated via navigateForResult). + * Used to deliver results back to the caller. + */ + private var currentRequestKey: String? = null + + /** + * Sets the request key for this navigation controller instance. + * Used when a Fragment is navigated to via navigateForResult. + */ + fun setRequestKeyFromFragment(key: String?) { + currentRequestKey = key + } + override fun navigateBack() { onBackPressedDispatcher.onBackPressed() } @@ -71,4 +90,101 @@ open class NibelNavigationController( else -> error("Unknown compose navigation spec '${composeSpec.javaClass}'") } } + + override fun navigateForResult( + entry: Entry, + fragmentSpec: FragmentSpec<*>, + composeSpec: ComposeSpec<*>, + callback: (TResult?) -> Unit, + ) { + // Generate unique request key for this navigation + val requestKey = UUID.randomUUID().toString() + + // Store the callback with type erasure (we trust compile-time type safety) + @Suppress("UNCHECKED_CAST") + ResultCallbackRegistry.storeCallback(requestKey, callback as (Any?) -> Unit) + + // Store request key so the callee can use it to return results + // For Fragment entries, pass it through Bundle arguments + // For Composable entries, store it in currentRequestKey + val previousRequestKey = currentRequestKey + currentRequestKey = requestKey + + // Inject request key into Fragment arguments for result-based navigation + if (entry is FragmentEntry) { + val args = entry.fragment.arguments ?: android.os.Bundle() + args.putRequestKey(requestKey) + entry.fragment.arguments = args + } + + try { + // Perform the navigation + navigateTo(entry, fragmentSpec, composeSpec) + } catch (e: IllegalStateException) { + // If navigation fails, clean up and restore state + ResultCallbackRegistry.removeCallback(requestKey) + currentRequestKey = previousRequestKey + throw e + } catch (e: IllegalArgumentException) { + // If navigation fails, clean up and restore state + ResultCallbackRegistry.removeCallback(requestKey) + currentRequestKey = previousRequestKey + throw e + } + } + + override fun navigateForResult( + externalDestination: ExternalDestination, + fragmentSpec: FragmentSpec<*>, + composeSpec: ComposeSpec<*>, + callback: (TResult?) -> Unit, + ) { + // Find the entry for this destination + val destinationEntry = Nibel.findEntryFactory(externalDestination) + ?.newInstance(externalDestination) + ?: error("Unable to find destination '${externalDestination.javaClass}'") + + // Delegate to the entry-based navigateForResult + navigateForResult(destinationEntry, fragmentSpec, composeSpec, callback) + } + + override fun setResultAndNavigateBack(result: TResult) { + val requestKey = currentRequestKey + ?: error( + "setResultAndNavigateBack() called but no request key found. " + + "This screen was not navigated to via navigateForResult().", + ) + + // Deliver the result through the callback + deliverResult(requestKey, result) + + // Navigate back + navigateBack() + } + + override fun cancelResultAndNavigateBack() { + val requestKey = currentRequestKey + + if (requestKey != null) { + // Deliver null result to indicate cancellation + deliverResult(requestKey, null) + } + + // Navigate back (this works even if no request key, like regular back) + navigateBack() + } + + /** + * Delivers a result to the callback associated with the given request key. + * Cleans up the callback after delivery. + */ + private fun deliverResult(requestKey: String, result: Any?) { + val callback = ResultCallbackRegistry.removeCallback(requestKey) + callback?.invoke(result) + + // Clear current request key if it matches + if (currentRequestKey == requestKey) { + currentRequestKey = null + } + } } diff --git a/nibel-runtime/src/main/kotlin/nibel/runtime/NoResult.kt b/nibel-runtime/src/main/kotlin/nibel/runtime/NoResult.kt new file mode 100644 index 0000000..a7dc214 --- /dev/null +++ b/nibel-runtime/src/main/kotlin/nibel/runtime/NoResult.kt @@ -0,0 +1,17 @@ +package nibel.runtime + +import android.os.Parcelable +import kotlinx.parcelize.Parcelize + +/** + * A special type of result that is treated specially by Nibel's annotation processor and marks a + * screen that does not return a result. + * + * This is the default value for the `result` parameter in [nibel.annotations.UiEntry] and + * [nibel.annotations.UiExternalEntry] annotations. + * + * @see nibel.annotations.UiEntry + * @see nibel.annotations.UiExternalEntry + */ +@Parcelize +object NoResult : Parcelable diff --git a/nibel-runtime/src/main/kotlin/nibel/runtime/ResultCallbackRegistry.kt b/nibel-runtime/src/main/kotlin/nibel/runtime/ResultCallbackRegistry.kt new file mode 100644 index 0000000..89166c5 --- /dev/null +++ b/nibel-runtime/src/main/kotlin/nibel/runtime/ResultCallbackRegistry.kt @@ -0,0 +1,80 @@ +package nibel.runtime + +import java.util.concurrent.ConcurrentHashMap + +/** + * Global registry for result callbacks that persists across Fragment/Activity recreation. + * This ensures callbacks survive navigation and configuration changes. + * + * Production-hardened with: + * - TTL-based cleanup (5-minute expiry) to prevent memory leaks + * - Max callback limit (50) to prevent DoS attacks + * - Thread-safe implementation using ConcurrentHashMap + */ +internal object ResultCallbackRegistry { + private const val MAX_CALLBACKS = 50 + private const val TTL_MILLIS = 5 * 60 * 1000L // 5 minutes + + private val callbacks = ConcurrentHashMap() + + /** + * Stores a callback with timestamp for TTL-based cleanup. + */ + fun storeCallback(requestKey: String, callback: (Any?) -> Unit) { + // Cleanup stale callbacks if we're at the limit + if (callbacks.size >= MAX_CALLBACKS) { + cleanupStaleCallbacks() + + // If still at limit after cleanup, drop the oldest callback + if (callbacks.size >= MAX_CALLBACKS) { + val oldestKey = callbacks.entries + .minByOrNull { it.value.timestamp } + ?.key + oldestKey?.let { callbacks.remove(it) } + } + } + + callbacks[requestKey] = CallbackEntry(callback, System.currentTimeMillis()) + } + + /** + * Removes and returns a callback, or null if not found. + */ + fun removeCallback(requestKey: String): ((Any?) -> Unit)? { + return callbacks.remove(requestKey)?.callback + } + + /** + * Checks if a callback exists for the given request key. + */ + fun hasCallback(requestKey: String): Boolean { + return callbacks.containsKey(requestKey) + } + + /** + * Clears all callbacks. Should only be used in tests or emergency cleanup. + */ + fun clear() { + callbacks.clear() + } + + /** + * Removes callbacks older than TTL_MILLIS to prevent memory leaks + * from abandoned navigation flows. + */ + private fun cleanupStaleCallbacks() { + val now = System.currentTimeMillis() + val staleKeys = callbacks.filterValues { entry -> + now - entry.timestamp > TTL_MILLIS + }.keys + staleKeys.forEach { callbacks.remove(it) } + } + + /** + * Internal data class to track callback with creation timestamp. + */ + private data class CallbackEntry( + val callback: (Any?) -> Unit, + val timestamp: Long, + ) +} diff --git a/nibel-runtime/src/main/kotlin/nibel/runtime/ResultEntry.kt b/nibel-runtime/src/main/kotlin/nibel/runtime/ResultEntry.kt new file mode 100644 index 0000000..6bda8e6 --- /dev/null +++ b/nibel-runtime/src/main/kotlin/nibel/runtime/ResultEntry.kt @@ -0,0 +1,71 @@ +package nibel.runtime + +import android.os.Parcelable + +/** + * Marker interface for entry classes that return a result to their caller. + * + * Generated entry classes implement this interface when the `result` parameter is specified + * in [nibel.annotations.UiEntry] or [nibel.annotations.UiExternalEntry] annotations. + * + * This interface is always implemented alongside either [ComposableEntry] or [FragmentEntry], + * never on its own. It adds result type information to enable type-safe result navigation where: + * - The caller uses `NavigationController.navigateForResult()` to navigate and receive a callback + * - The called screen uses `NavigationController.setResultAndNavigateBack()` to return a result + * - The called screen can use `NavigationController.cancelResultAndNavigateBack()` to cancel without result + * + * The result type [TResult] must be [Parcelable] and is validated at compile-time by the + * annotation processor. + * + * Example usage: + * ``` + * // Define result type + * @Parcelize + * data class PhotoResult(val uri: String) : Parcelable + * + * // Annotate screen with result + * @UiEntry( + * type = ImplementationType.Composable, + * result = PhotoResult::class + * ) + * @Composable + * fun PhotoPickerScreen(navigator: NavigationController) { + * // ... selection UI + * navigator.setResultAndNavigateBack(PhotoResult(selectedUri)) + * } + * + * // Navigate and receive result + * navigator.navigateForResult(PhotoPickerScreenEntry.newInstance()) { result: PhotoResult? -> + * if (result != null) { + * // User selected a photo + * } else { + * // User cancelled + * } + * } + * ``` + * + * The generated entry class will look like: + * ``` + * class PhotoPickerScreenEntry(...) + * : ComposableEntry(...), ResultEntry { + * override val resultType: Class = PhotoResult::class.java + * // ... + * } + * ``` + * + * @param TArgs The type of arguments passed to the entry (must be [Parcelable]) + * @param TResult The type of result returned by the entry (must be [Parcelable]) + * + * @see nibel.annotations.UiEntry + * @see nibel.annotations.UiExternalEntry + * @see NavigationController.navigateForResult + * @see NavigationController.setResultAndNavigateBack + * @see NavigationController.cancelResultAndNavigateBack + */ +interface ResultEntry { + /** + * The runtime [Class] of the result type. + * Used for type checking and result delivery at runtime. + */ + val resultType: Class +} diff --git a/nibel-runtime/src/main/kotlin/nibel/runtime/RouteBuilder.kt b/nibel-runtime/src/main/kotlin/nibel/runtime/RouteBuilder.kt new file mode 100644 index 0000000..1c47daf --- /dev/null +++ b/nibel-runtime/src/main/kotlin/nibel/runtime/RouteBuilder.kt @@ -0,0 +1,22 @@ +package nibel.runtime + +import android.os.Parcelable + +/** + * Builds a unique route name for navigation based on the entry class name and arguments. + * + * The route name is used by the Compose Navigation library to identify destinations. + * For entries with arguments, the route includes a hash of the arguments to ensure uniqueness. + * For entries without arguments, only the qualified name is used. + * + * @param qualifiedName The fully qualified class name of the entry + * @param args The arguments for the entry, or null if no arguments + * @return A unique route string for navigation + */ +fun buildRouteName(qualifiedName: String, args: Parcelable?): String { + return if (args != null) { + "$qualifiedName@${args.hashCode()}" + } else { + qualifiedName + } +} diff --git a/nibel-stub/src/main/kotlin/nibel/runtime/NoResult.kt b/nibel-stub/src/main/kotlin/nibel/runtime/NoResult.kt new file mode 100644 index 0000000..67c179d --- /dev/null +++ b/nibel-stub/src/main/kotlin/nibel/runtime/NoResult.kt @@ -0,0 +1,6 @@ +package nibel.runtime + +import android.os.Parcelable + +@Suppress("ParcelCreator") +object NoResult : Parcelable diff --git a/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondScreen.kt b/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondScreen.kt index 7b26b73..936b907 100644 --- a/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondScreen.kt +++ b/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondScreen.kt @@ -43,7 +43,10 @@ private fun SideEffectHandler(sideEffects: Flow) { is SecondSideEffect.NavigateBack -> navigateBack() is SecondSideEffect.NavigateToThirdScreen -> { val args = ThirdArgs(it.inputText) - navigateTo(ThirdScreenDestination(args)) + navigateForResult( + externalDestination = ThirdScreenDestination(args), + callback = it.callback, + ) } } } diff --git a/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondSideEffect.kt b/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondSideEffect.kt index f602c81..5238957 100644 --- a/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondSideEffect.kt +++ b/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondSideEffect.kt @@ -1,8 +1,13 @@ package com.turo.nibel.sample.featureA.secondscreen +import com.turo.nibel.sample.navigation.ThirdScreenResult + sealed interface SecondSideEffect { object NavigateBack : SecondSideEffect - data class NavigateToThirdScreen(val inputText: String) : SecondSideEffect + data class NavigateToThirdScreen( + val inputText: String, + val callback: (ThirdScreenResult?) -> Unit, + ) : SecondSideEffect } diff --git a/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondViewModel.kt b/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondViewModel.kt index 88343ee..dfd4197 100644 --- a/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondViewModel.kt +++ b/sample/feature-A/src/main/kotlin/com/turo/nibel/sample/featureA/secondscreen/SecondViewModel.kt @@ -28,12 +28,28 @@ class SecondViewModel @Inject constructor( fun onContinue(nextButton: SecondNextButton) { when (nextButton) { - SecondNextButton.SecondScreen -> - _sideEffects.tryEmit(SecondSideEffect.NavigateToThirdScreen(state.value.inputText)) + SecondNextButton.SecondScreen -> { + _sideEffects.tryEmit( + SecondSideEffect.NavigateToThirdScreen( + inputText = state.value.inputText, + callback = { result -> + // Handle result from ThirdScreen + result?.let { + onResultFromThirdScreen(it.inputText) + } + }, + ), + ) + } } } fun onInputTextChanged(inputText: String) { _state.value = _state.value.copy(inputText = inputText) } + + private fun onResultFromThirdScreen(inputText: String) { + // Update state with the result from ThirdScreen + _state.value = _state.value.copy(inputText = inputText) + } } diff --git a/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdScreen.kt b/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdScreen.kt index 5243c97..f7fc023 100644 --- a/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdScreen.kt +++ b/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdScreen.kt @@ -10,6 +10,7 @@ import com.turo.nibel.sample.navigation.FirstScreenDestination import com.turo.nibel.sample.navigation.FourthArgs import com.turo.nibel.sample.navigation.FourthScreenDestination import com.turo.nibel.sample.navigation.ThirdScreenDestination +import com.turo.nibel.sample.navigation.ThirdScreenResult import kotlinx.coroutines.flow.Flow import nibel.annotations.ImplementationType import nibel.annotations.UiExternalEntry @@ -18,6 +19,7 @@ import nibel.runtime.LocalImplementationType @UiExternalEntry( type = ImplementationType.Fragment, destination = ThirdScreenDestination::class, + result = ThirdScreenResult::class, ) @Composable fun ThirdScreen(viewModel: ThirdViewModel = hiltViewModel()) { @@ -40,7 +42,10 @@ fun ThirdScreen(viewModel: ThirdViewModel = hiltViewModel()) { private fun SideEffectHandler(sideEffects: Flow) { SideEffectHandler(sideEffects) { when (it) { - is ThirdSideEffect.NavigateBack -> navigateBack() + is ThirdSideEffect.NavigateBack -> { + cancelResultAndNavigateBack() + } + is ThirdSideEffect.NavigateToFourthScreen -> { val args = FourthArgs(it.inputText) navigateTo(FourthScreenDestination(args)) @@ -48,6 +53,12 @@ private fun SideEffectHandler(sideEffects: Flow) { is ThirdSideEffect.NavigateToFirstScreen -> navigateTo(FirstScreenDestination) + + is ThirdSideEffect.SetResultAndNavigateBack -> + setResultAndNavigateBack(it.result) + + is ThirdSideEffect.CancelResultAndNavigateBack -> + cancelResultAndNavigateBack() } } } diff --git a/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdSideEffect.kt b/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdSideEffect.kt index 4acee79..00b7127 100644 --- a/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdSideEffect.kt +++ b/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdSideEffect.kt @@ -1,5 +1,7 @@ package com.turo.nibel.sample.featureB.thirdscreen +import com.turo.nibel.sample.navigation.ThirdScreenResult + sealed interface ThirdSideEffect { object NavigateBack : ThirdSideEffect @@ -7,4 +9,8 @@ sealed interface ThirdSideEffect { data class NavigateToFourthScreen(val inputText: String) : ThirdSideEffect object NavigateToFirstScreen : ThirdSideEffect + + data class SetResultAndNavigateBack(val result: ThirdScreenResult) : ThirdSideEffect + + object CancelResultAndNavigateBack : ThirdSideEffect } diff --git a/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdState.kt b/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdState.kt index c53931f..a5cc567 100644 --- a/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdState.kt +++ b/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdState.kt @@ -8,12 +8,15 @@ data class ThirdState( val title: String = "Feature B | Third Screen" val nextButtons = listOf( + ThirdNextButton.ReturnResult, ThirdNextButton.FourthScreen, ThirdNextButton.FirstScreen, ) } sealed class ThirdNextButton(override val title: String) : NextButton { + object ReturnResult : ThirdNextButton("Return Result to SecondScreen") + object FourthScreen : ThirdNextButton("Fourth Screen (legacy fragment, external)") object FirstScreen : ThirdNextButton("First Screen (type.fragment, external, no args)") diff --git a/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdViewModel.kt b/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdViewModel.kt index 4ac6658..ee67259 100644 --- a/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdViewModel.kt +++ b/sample/feature-B/src/main/kotlin/com/turo/nibel/sample/featureB/thirdscreen/ThirdViewModel.kt @@ -3,6 +3,7 @@ package com.turo.nibel.sample.featureB.thirdscreen import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import com.turo.nibel.sample.navigation.ThirdArgs +import com.turo.nibel.sample.navigation.ThirdScreenResult import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow @@ -29,6 +30,13 @@ class ThirdViewModel @Inject constructor( fun onContinue(nextButton: ThirdNextButton) { when (nextButton) { + ThirdNextButton.ReturnResult -> { + val result = ThirdScreenResult( + inputText = state.value.inputText, + ) + _sideEffects.tryEmit(ThirdSideEffect.SetResultAndNavigateBack(result)) + } + ThirdNextButton.FourthScreen -> _sideEffects.tryEmit(ThirdSideEffect.NavigateToFourthScreen(state.value.inputText)) diff --git a/sample/navigation/src/main/kotlin/com/turo/nibel/sample/navigation/FeatureBNavigation.kt b/sample/navigation/src/main/kotlin/com/turo/nibel/sample/navigation/FeatureBNavigation.kt index e21c5f0..a81f5af 100644 --- a/sample/navigation/src/main/kotlin/com/turo/nibel/sample/navigation/FeatureBNavigation.kt +++ b/sample/navigation/src/main/kotlin/com/turo/nibel/sample/navigation/FeatureBNavigation.kt @@ -7,4 +7,6 @@ import nibel.annotations.DestinationWithArgs @Parcelize data class ThirdArgs(val inputText: String) : Parcelable +@Parcelize +data class ThirdScreenResult(val inputText: String) : Parcelable data class ThirdScreenDestination(override val args: ThirdArgs) : DestinationWithArgs