Give every feature an api and a presentation module - #88
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Adopt the module shape used by Novix and MENA-mobile: every feature is a pair of modules, a contract and an implementation, so a consumer depends on what a feature offers rather than on how it is built.
What changed
All seven features are now
api+presentation::feature:{onboarding,settings,projects,terminal,editor}:apiare new;gitandbuildrunalready had contracts.:feature:<name>module is now:feature:<name>:presentation.Routesobject and anXxxFeatureApiinterface of@Composableentry points —WelcomeEntry,HubEntry,RootEntry,TerminalEntry,EditorEntryand so on.presentationmodule holds aninternal XxxFeatureApiImpland binds its own contract in its own Koin module.:appresolves contracts withkoinInject()and no longer imports a single screen composable or nav-graph function from any feature.apimodule hasexplicitApi()on and depends only on:domainplus Compose.The one deliberate difference from the references
Novix and MENA each give a feature its own nested
NavHostbehind its entry point. This keeps a single app-levelNavHost: ASL is one window with a deep cross-feature back stack (editor → git diff → editor) and saved-state results between screens, and per-feature nav hosts give up a unified back stack, handle predictive back per feature, and lose state when switching. Theapishape is the same; the back stack stays whole.Why the DI change had to come first
Both references bind every
XxxApiImplcentrally in the app module — Novix inapp/di/ApiModule.kt, MENA incomposeApp/di/apiModule.kt. That leaves the app compile-dependent on every implementation and stops any impl from beinginternal, which defeats the split. Here each feature binds itself, so all sevenXxxFeatureApiImplclasses areinternal.Behavior
No behavior change. Route strings, destination registration order, arguments and callbacks are unchanged; only who declares them moved.
Some callback names moved to present tense to satisfy the Compose naming rule on the new public surfaces (
onGetStarted→onStart,onPickedFolderConsumed→onConsumePickedFolder,onCreated→onCreate,onFolderSelected→onSelectFolder,onConflictPathOpened→onConsumeConflictPath). The implementations map them back to the unchangedRouteparameters, so no screen signature changed. These were fixed rather than baselined because they are declarations added here.Tests
KoinModuleGraphTestverifies all seven new contract bindings resolve.verifyModuleBoundaries: 0 baseline entries, 0 violations. The contract check added earlier now guards sevenapimodules instead of two — no@Composableinterface member may take default arguments, which is the crash this migration already shipped once.Verification
./gradlew test detekt verifyModuleBoundaries :app:assembleDebug— BUILD SUCCESSFUL.Verified on an emulator, exercising every new contract:
OnboardingFeatureApi/ProjectsFeatureApi— hub renders; create project → browse location → select folder returns/storage/emulated/0/Documentsinto Save location, so saved-state results still cross the contractSettingsFeatureApi— Preferences renders, Git & GitHub opensEditorFeatureApi— project opens, editor rendersTerminalFeatureApi— the Terminal tab renders a live session inside the editor's bottom panel, through the slot the editor exposes rather than a dependencyAbstractMethodError,NoBeanDefFoundExceptionorInstanceCreationExceptionin logcat