feat: DI - Added first-class Dependency Injection support to FlutterInit with a new wizard option and generated code wiring.#21
Open
noeljabraham wants to merge 3 commits intoArjun544:mainfrom
Conversation
|
@noeljabraham is attempting to deploy a commit to the arjun544's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds optional GetIt dependency-injection support to the Flutter scaffold generator: new config option, wizard UI, generator flag, templates for service locator, DI-aware wiring in auth/routing/state templates, and conditional inclusion of Changes
Sequence DiagramsequenceDiagram
participant App as App Startup
participant Config as AppConfig
participant DI as initDependencies()
participant SL as ServiceLocator (GetIt)
participant AuthService as AuthService
participant Repo as AuthRepository
App->>Config: await AppConfig.init()
Config-->>App: config ready
alt dependencyInjection == "get_it"
App->>DI: await initDependencies()
DI->>SL: GetIt.instance / register singletons
SL->>AuthService: register AuthService
SL->>Repo: register AuthRepository (depends on AuthService)
DI-->>App: DI initialized
end
App->>App: runApp(MyApp)
App->>Repo: createAuthRepository()
alt uses GetIt
Repo->>SL: sl<AuthRepository>()
SL-->>Repo: returns singleton
else
Repo->>AuthService: AuthRepositoryImpl(authService: AuthService.instance)
Repo-->>App: new AuthRepositoryImpl
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
templates/flutter/base/lib/main.dart.hbs (1)
26-26: Coupling note: unconditional call requires the function to always exist.
await initDependencies()is invoked unconditionally, which assumesservice_locator.dart.hbsis always generated and always exposesinitDependencies()(with the body gated byflags.usesGetItDi). That matches the current template, but ifservice_locator.dart.hbsis ever made conditional onflags.usesGetItDi, this call site will break the build for thenonepath. Consider either keeping the template unconditional (current design) or gating this call with{{#ifflags.usesGetItDi}}to make the coupling explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/flutter/base/lib/main.dart.hbs` at line 26, The unconditional call to initDependencies() couples this template to service_locator.dart.hbs; guard the invocation with the feature flag by wrapping the call in the template conditional {{`#if` flags.usesGetItDi}}...{{/if}} so the call to initDependencies() only exists when service_locator.dart.hbs (and its exported initDependencies function) is generated; reference initDependencies() and service_locator.dart.hbs when making the change.templates/flutter/base/lib/src/di/service_locator.dart.hbs (1)
10-22: Minor: emptyinitDependencies()when DI is disabled.When
flags.usesGetItDiis false the body collapses to an emptyasyncfunction thatmain.dartstill awaits. It's harmless, but you could either:
- only emit
initDependencies()(and itsawaitcall inmain.dart) when DI is enabled, or- keep it unconditional as an intentional future-proof hook and add a one-line comment noting that.
Also consider registering
AuthServiceby constructing the type directly (e.g.,() => AuthService()) rather than() => AuthService.instance, otherwise you're just wrapping an existing singleton in GetIt and bothAuthService.instanceandsl<AuthService>()remain live references — not incorrect, just redundant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/flutter/base/lib/src/di/service_locator.dart.hbs` around lines 10 - 22, The generated initDependencies() becomes an empty async function when flags.usesGetItDi is false; either stop emitting initDependencies() (and its await in main.dart) when flags.usesGetItDi is false, or keep it always emitted but add a one-line comment inside to clarify it is an intentional no-op hook; additionally, when registering AuthService in the block guarded by flags.usesGetItDi prefer registering a new instance (use () => AuthService()) instead of wrapping AuthService.instance to avoid keeping two live singleton references—update the registration lines for AuthService and keep the AuthRepository registration using AuthRepositoryImpl(authService: sl<AuthService>()) as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/components/wizard/steps/MiscStep.tsx`:
- Around line 249-279: The info <button> is nested inside the <label>, which is
invalid and breaks a11y; refactor the map so each option is a wrapper <div> (or
other non-interactive container) that contains a <label> scoped only to the
RadioGroupItem and its text (use dependencyInjectionOptions and
RadioGroupItem/option.label/option.description), and place the info button
(HugeiconsIcon with InformationCircleIcon) as a sibling element outside the
<label>; keep the existing onClick handler that calls setSelectedItem but ensure
the button still calls e.preventDefault()/e.stopPropagation() so it doesn't
toggle the radio, and preserve the original conditional className logic that
uses config.dependencyInjection to maintain the visual selected state; move the
key={option.value} to the outer wrapper so list keys remain stable.
In `@templates/flutter/base/pubspec.yaml.hbs`:
- Around line 28-33: The get_it dependency is incorrectly listed under the
"State Management" section and uses an outdated constraint; when
flags.usesGetItDi is true, remove the get_it: ^8.0.3 entry from under the State
Management block and instead add a new commented section "# Dependency
Injection" (or similar) where you include get_it with an updated constraint (at
least ^9.0.0, e.g., ^9.0.0 or ^9.2.1); update the Handlebars template around the
conditional (flags.usesGetItDi) so the get_it line is emitted under that new
section rather than next to fpdart/equatable.
In `@templates/flutter/partials/features/auth/auth_logic.hbs`:
- Around line 7-9: The top comment is incorrect: it claims the provider returns
a single instance of AuthRepositoryImpl, but authRepositoryProvider actually
returns createAuthRepository() which may resolve via GetIt and not necessarily
AuthRepositoryImpl; update the comment above authRepositoryProvider to remove
the guarantee of AuthRepositoryImpl and instead state that the provider exposes
an AuthRepository resolved by createAuthRepository() (which may use GetIt).
In `@templates/flutter/partials/features/auth/forgot_password_screen.hbs`:
- Around line 46-47: The template currently sets authStore to null when
flags.usesFlutterHooks is false which causes a null dereference on
authStore.isLoading; change the conditional so you emit two distinct
declarations: when flags.usesFlutterHooks is true generate a memoized store
(useMemoized(() => AuthStore(repository: createAuthRepository()), [])) and when
false generate a plain instance (AuthStore(repository: createAuthRepository()));
ensure the variable name authStore remains the same so subsequent references
like authStore.isLoading compile.
In `@templates/flutter/partials/features/auth/login_screen.hbs`:
- Around line 51-52: The template currently renders authStore as null when
flags.usesFlutterHooks is false because the inline comment breaks the
expression; change the conditional so authStore is always initialized: if
flags.usesFlutterHooks render useMemoized(() => AuthStore(repository:
createAuthRepository()), []), else render AuthStore(repository:
createAuthRepository()); remove the inline comment that caused the null and
ensure usages like isLoading and handleLogin (which reference authStore) work
with the non-null AuthStore instance.
In `@templates/flutter/partials/features/auth/signup_screen.hbs`:
- Around line 56-57: The template currently emits invalid Dart when MobX is
enabled but Flutter Hooks are disabled by assigning authStore to null; instead,
change the template logic around authStore so that when flags.usesFlutterHooks
is true you keep the existing useMemoized invocation, and when false you
instantiate AuthStore directly (AuthStore(repository: createAuthRepository()));
ensure subsequent usages like authStore.isLoading and other authStore references
work with the instantiated AuthStore; update the conditional in
signup_screen.hbs (and mirror the same change in login_screen.hbs and
forgot_password_screen.hbs) to branch between useMemoized(...) and new
AuthStore(...) rather than null.
---
Nitpick comments:
In `@templates/flutter/base/lib/main.dart.hbs`:
- Line 26: The unconditional call to initDependencies() couples this template to
service_locator.dart.hbs; guard the invocation with the feature flag by wrapping
the call in the template conditional {{`#if` flags.usesGetItDi}}...{{/if}} so the
call to initDependencies() only exists when service_locator.dart.hbs (and its
exported initDependencies function) is generated; reference initDependencies()
and service_locator.dart.hbs when making the change.
In `@templates/flutter/base/lib/src/di/service_locator.dart.hbs`:
- Around line 10-22: The generated initDependencies() becomes an empty async
function when flags.usesGetItDi is false; either stop emitting
initDependencies() (and its await in main.dart) when flags.usesGetItDi is false,
or keep it always emitted but add a one-line comment inside to clarify it is an
intentional no-op hook; additionally, when registering AuthService in the block
guarded by flags.usesGetItDi prefer registering a new instance (use () =>
AuthService()) instead of wrapping AuthService.instance to avoid keeping two
live singleton references—update the registration lines for AuthService and keep
the AuthRepository registration using AuthRepositoryImpl(authService:
sl<AuthService>()) as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c5a91b9-2e99-401d-9922-fef41fe8dc88
📒 Files selected for processing (19)
app/components/wizard/PackageInfoPanel.tsxapp/components/wizard/steps/GenerateStep.tsxapp/components/wizard/steps/MiscStep.tsxapp/lib/config/schema.tsapp/lib/generator/index.tsdocs/configuration.mdtemplates/flutter/base/README.md.hbstemplates/flutter/base/lib/main.dart.hbstemplates/flutter/base/lib/src/di/service_locator.dart.hbstemplates/flutter/base/lib/src/imports/core_imports.dart.hbstemplates/flutter/base/lib/src/routing/(isGetX)@app_bindings.dart.hbstemplates/flutter/base/lib/src/shared/wrappers/state_wrapper.dart.hbstemplates/flutter/base/pubspec.yaml.hbstemplates/flutter/partials/features/auth/auth_logic.hbstemplates/flutter/partials/features/auth/auth_repository_impl.hbstemplates/flutter/partials/features/auth/forgot_password_screen.hbstemplates/flutter/partials/features/auth/login_screen.hbstemplates/flutter/partials/features/auth/session_provider.hbstemplates/flutter/partials/features/auth/signup_screen.hbs
Comment on lines
+249
to
+279
| {dependencyInjectionOptions.map((option) => ( | ||
| <label | ||
| key={option.value} | ||
| className={cn( | ||
| "flex items-center justify-between rounded-xl border p-3 cursor-pointer transition-all duration-200", | ||
| config.dependencyInjection === option.value | ||
| ? "border-primary/50 bg-primary/5 shadow-md shadow-primary/5 ring-1 ring-primary/20" | ||
| : "border-border/40 bg-card/30 hover:bg-card/50 hover:border-primary/20" | ||
| )} | ||
| > | ||
| <div className="flex items-center gap-3"> | ||
| <RadioGroupItem value={option.value} className="text-primary border-primary" /> | ||
| <div className="space-y-1"> | ||
| <span className="text-sm font-semibold text-foreground/90">{option.label}</span> | ||
| <p className="text-xs text-muted-foreground">{option.description}</p> | ||
| </div> | ||
| </div> | ||
| <button | ||
| type="button" | ||
| onClick={(e) => { | ||
| e.preventDefault() | ||
| e.stopPropagation() | ||
| setSelectedItem(option.value) | ||
| }} | ||
| className="p-1.5 rounded-full hover:bg-primary/20 text-muted-foreground hover:text-primary transition-colors focus:outline-hidden cursor-pointer" | ||
| title="View details" | ||
| > | ||
| <HugeiconsIcon icon={InformationCircleIcon} size={18} /> | ||
| </button> | ||
| </label> | ||
| ))} |
There was a problem hiding this comment.
Avoid nesting a <button> inside a <label> (invalid HTML / a11y).
The HTML spec disallows interactive descendants other than a single labelable control inside <label>. Nesting the info <button> inside the label can trigger accessibility warnings and inconsistent click behavior across browsers (some will still activate the radio even with preventDefault/stopPropagation). Prefer a sibling layout: wrap the row in a <div> and keep the <label> scoped to the radio + text only.
♻️ Proposed refactor
- {dependencyInjectionOptions.map((option) => (
- <label
- key={option.value}
- className={cn(
- "flex items-center justify-between rounded-xl border p-3 cursor-pointer transition-all duration-200",
- config.dependencyInjection === option.value
- ? "border-primary/50 bg-primary/5 shadow-md shadow-primary/5 ring-1 ring-primary/20"
- : "border-border/40 bg-card/30 hover:bg-card/50 hover:border-primary/20"
- )}
- >
- <div className="flex items-center gap-3">
- <RadioGroupItem value={option.value} className="text-primary border-primary" />
- <div className="space-y-1">
- <span className="text-sm font-semibold text-foreground/90">{option.label}</span>
- <p className="text-xs text-muted-foreground">{option.description}</p>
- </div>
- </div>
- <button
- type="button"
- onClick={(e) => {
- e.preventDefault()
- e.stopPropagation()
- setSelectedItem(option.value)
- }}
- className="p-1.5 rounded-full hover:bg-primary/20 text-muted-foreground hover:text-primary transition-colors focus:outline-hidden cursor-pointer"
- title="View details"
- >
- <HugeiconsIcon icon={InformationCircleIcon} size={18} />
- </button>
- </label>
- ))}
+ {dependencyInjectionOptions.map((option) => {
+ const id = `di-${option.value}`
+ const selected = config.dependencyInjection === option.value
+ return (
+ <div
+ key={option.value}
+ className={cn(
+ "flex items-center justify-between rounded-xl border p-3 transition-all duration-200",
+ selected
+ ? "border-primary/50 bg-primary/5 shadow-md shadow-primary/5 ring-1 ring-primary/20"
+ : "border-border/40 bg-card/30 hover:bg-card/50 hover:border-primary/20"
+ )}
+ >
+ <label htmlFor={id} className="flex items-center gap-3 cursor-pointer flex-1">
+ <RadioGroupItem id={id} value={option.value} className="text-primary border-primary" />
+ <div className="space-y-1">
+ <span className="text-sm font-semibold text-foreground/90">{option.label}</span>
+ <p className="text-xs text-muted-foreground">{option.description}</p>
+ </div>
+ </label>
+ <button
+ type="button"
+ aria-label={`View details for ${option.label}`}
+ onClick={() => setSelectedItem(option.value)}
+ className="p-1.5 rounded-full hover:bg-primary/20 text-muted-foreground hover:text-primary transition-colors focus:outline-hidden cursor-pointer"
+ title="View details"
+ >
+ <HugeiconsIcon icon={InformationCircleIcon} size={18} />
+ </button>
+ </div>
+ )
+ })}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {dependencyInjectionOptions.map((option) => ( | |
| <label | |
| key={option.value} | |
| className={cn( | |
| "flex items-center justify-between rounded-xl border p-3 cursor-pointer transition-all duration-200", | |
| config.dependencyInjection === option.value | |
| ? "border-primary/50 bg-primary/5 shadow-md shadow-primary/5 ring-1 ring-primary/20" | |
| : "border-border/40 bg-card/30 hover:bg-card/50 hover:border-primary/20" | |
| )} | |
| > | |
| <div className="flex items-center gap-3"> | |
| <RadioGroupItem value={option.value} className="text-primary border-primary" /> | |
| <div className="space-y-1"> | |
| <span className="text-sm font-semibold text-foreground/90">{option.label}</span> | |
| <p className="text-xs text-muted-foreground">{option.description}</p> | |
| </div> | |
| </div> | |
| <button | |
| type="button" | |
| onClick={(e) => { | |
| e.preventDefault() | |
| e.stopPropagation() | |
| setSelectedItem(option.value) | |
| }} | |
| className="p-1.5 rounded-full hover:bg-primary/20 text-muted-foreground hover:text-primary transition-colors focus:outline-hidden cursor-pointer" | |
| title="View details" | |
| > | |
| <HugeiconsIcon icon={InformationCircleIcon} size={18} /> | |
| </button> | |
| </label> | |
| ))} | |
| {dependencyInjectionOptions.map((option) => { | |
| const id = `di-${option.value}` | |
| const selected = config.dependencyInjection === option.value | |
| return ( | |
| <div | |
| key={option.value} | |
| className={cn( | |
| "flex items-center justify-between rounded-xl border p-3 transition-all duration-200", | |
| selected | |
| ? "border-primary/50 bg-primary/5 shadow-md shadow-primary/5 ring-1 ring-primary/20" | |
| : "border-border/40 bg-card/30 hover:bg-card/50 hover:border-primary/20" | |
| )} | |
| > | |
| <label htmlFor={id} className="flex items-center gap-3 cursor-pointer flex-1"> | |
| <RadioGroupItem id={id} value={option.value} className="text-primary border-primary" /> | |
| <div className="space-y-1"> | |
| <span className="text-sm font-semibold text-foreground/90">{option.label}</span> | |
| <p className="text-xs text-muted-foreground">{option.description}</p> | |
| </div> | |
| </label> | |
| <button | |
| type="button" | |
| aria-label={`View details for ${option.label}`} | |
| onClick={() => setSelectedItem(option.value)} | |
| className="p-1.5 rounded-full hover:bg-primary/20 text-muted-foreground hover:text-primary transition-colors focus:outline-hidden cursor-pointer" | |
| title="View details" | |
| > | |
| <HugeiconsIcon icon={InformationCircleIcon} size={18} /> | |
| </button> | |
| </div> | |
| ) | |
| })} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/components/wizard/steps/MiscStep.tsx` around lines 249 - 279, The info
<button> is nested inside the <label>, which is invalid and breaks a11y;
refactor the map so each option is a wrapper <div> (or other non-interactive
container) that contains a <label> scoped only to the RadioGroupItem and its
text (use dependencyInjectionOptions and
RadioGroupItem/option.label/option.description), and place the info button
(HugeiconsIcon with InformationCircleIcon) as a sibling element outside the
<label>; keep the existing onClick handler that calls setSelectedItem but ensure
the button still calls e.preventDefault()/e.stopPropagation() so it doesn't
toggle the radio, and preserve the original conditional className logic that
uses config.dependencyInjection to maintain the visual selected state; move the
key={option.value} to the outer wrapper so list keys remain stable.
Comment on lines
28
to
+33
| # State Management | ||
| fpdart: ^1.2.0 | ||
| equatable: ^2.0.7 | ||
| {{#if flags.usesGetItDi}} | ||
| get_it: ^8.0.3 | ||
| {{/if}} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🌐 Web query:
What is the latest stable version of the get_it Dart/Flutter package on pub.dev?
💡 Result:
The latest stable version of the get_it Dart/Flutter package on pub.dev is 9.2.1.
Citations:
- 1: https://pub.dev/packages/get_it
- 2: https://pub.dev/packages/get_it/versions
- 3: https://github.com/flutter-it/get_it/blob/main/CHANGELOG.md
- 4: https://pub.dev/packages/get_it/changelog
Move get_it to a dedicated "Dependency Injection" section and update version constraint.
get_it is a DI/service-locator package, not a state management library. Grouping it under # State Management is misleading. Additionally, the version constraint ^8.0.3 is outdated; the latest stable version is 9.2.1. Update to at least ^9.0.0.
♻️ Proposed fix
# State Management
fpdart: ^1.2.0
equatable: ^2.0.7
- {{`#if` flags.usesGetItDi}}
- get_it: ^8.0.3
- {{/if}}
{{`#if` flags.usesFlutterHooks}}
flutter_hooks: ^0.20.5
{{/if}}
@@
{{`#if` flags.isMobX}}
mobx: ^2.6.0
flutter_mobx: ^2.3.0
{{/if}}
+
+ # Dependency Injection
+ {{`#if` flags.usesGetItDi}}
+ get_it: ^9.0.0
+ {{/if}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@templates/flutter/base/pubspec.yaml.hbs` around lines 28 - 33, The get_it
dependency is incorrectly listed under the "State Management" section and uses
an outdated constraint; when flags.usesGetItDi is true, remove the get_it:
^8.0.3 entry from under the State Management block and instead add a new
commented section "# Dependency Injection" (or similar) where you include get_it
with an updated constraint (at least ^9.0.0, e.g., ^9.0.0 or ^9.2.1); update the
Handlebars template around the conditional (flags.usesGetItDi) so the get_it
line is emitted under that new section rather than next to fpdart/equatable.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Comment on lines
7
to
+9
| // Provides the single instance of AuthRepositoryImpl | ||
| final authRepositoryProvider = Provider<AuthRepository>((ref) { | ||
| return AuthRepositoryImpl(); | ||
| return createAuthRepository(); |
There was a problem hiding this comment.
Update the stale repository comment.
This provider no longer guarantees an AuthRepositoryImpl; with GetIt enabled it resolves through createAuthRepository().
📝 Proposed comment update
-// Provides the single instance of AuthRepositoryImpl
+// Provides the configured AuthRepository instance.
final authRepositoryProvider = Provider<AuthRepository>((ref) {
return createAuthRepository();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Provides the single instance of AuthRepositoryImpl | |
| final authRepositoryProvider = Provider<AuthRepository>((ref) { | |
| return AuthRepositoryImpl(); | |
| return createAuthRepository(); | |
| // Provides the configured AuthRepository instance. | |
| final authRepositoryProvider = Provider<AuthRepository>((ref) { | |
| return createAuthRepository(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@templates/flutter/partials/features/auth/auth_logic.hbs` around lines 7 - 9,
The top comment is incorrect: it claims the provider returns a single instance
of AuthRepositoryImpl, but authRepositoryProvider actually returns
createAuthRepository() which may resolve via GetIt and not necessarily
AuthRepositoryImpl; update the comment above authRepositoryProvider to remove
the guarantee of AuthRepositoryImpl and instead state that the provider exposes
an AuthRepository resolved by createAuthRepository() (which may use GetIt).
Comment on lines
+46
to
47
| final authStore = {{#if flags.usesFlutterHooks}}useMemoized{{else}}null; // Add your MobX store here{{/if}}(() => AuthStore(repository: createAuthRepository()), []); | ||
| final isLoading = authStore.isLoading; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "forgot_password_screen.hbs" -type fRepository: Arjun544/flutter_init
Length of output: 134
🏁 Script executed:
cat -n ./templates/flutter/partials/features/auth/forgot_password_screen.hbsRepository: Arjun544/flutter_init
Length of output: 16534
Fix MobX code generation when Flutter Hooks are disabled.
The non-hooks MobX code path has a critical bug: the template assigns null to authStore on line 46, then attempts to dereference it on line 47 with authStore.isLoading. This generates invalid Dart that fails at compile time.
When flags.usesFlutterHooks is false, line 46 currently renders as:
final authStore = null; // Add your MobX store here(...);
The template should separate the conditional to generate valid code for both paths:
Template fix
{{else if flags.isMobX}}
- final authStore = {{`#if` flags.usesFlutterHooks}}useMemoized{{else}}null; // Add your MobX store here{{/if}}(() => AuthStore(repository: createAuthRepository()), []);
+ {{`#if` flags.usesFlutterHooks}}
+ final authStore = useMemoized(() => AuthStore(repository: createAuthRepository()), []);
+ {{else}}
+ final authStore = AuthStore(repository: createAuthRepository());
+ {{/if}}
final isLoading = authStore.isLoading;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final authStore = {{#if flags.usesFlutterHooks}}useMemoized{{else}}null; // Add your MobX store here{{/if}}(() => AuthStore(repository: createAuthRepository()), []); | |
| final isLoading = authStore.isLoading; | |
| {{else if flags.isMobX}} | |
| {{`#if` flags.usesFlutterHooks}} | |
| final authStore = useMemoized(() => AuthStore(repository: createAuthRepository()), []); | |
| {{else}} | |
| final authStore = AuthStore(repository: createAuthRepository()); | |
| {{/if}} | |
| final isLoading = authStore.isLoading; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@templates/flutter/partials/features/auth/forgot_password_screen.hbs` around
lines 46 - 47, The template currently sets authStore to null when
flags.usesFlutterHooks is false which causes a null dereference on
authStore.isLoading; change the conditional so you emit two distinct
declarations: when flags.usesFlutterHooks is true generate a memoized store
(useMemoized(() => AuthStore(repository: createAuthRepository()), [])) and when
false generate a plain instance (AuthStore(repository: createAuthRepository()));
ensure the variable name authStore remains the same so subsequent references
like authStore.isLoading compile.
Comment on lines
+51
to
52
| final authStore = {{#if flags.usesFlutterHooks}}useMemoized{{else}}null; // Add your MobX store here{{/if}}(() => AuthStore(repository: createAuthRepository()), []); | ||
| final isLoading = authStore.isLoading; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "login_screen.hbs" -type fRepository: Arjun544/flutter_init
Length of output: 124
🏁 Script executed:
cat -n ./templates/flutter/partials/features/auth/login_screen.hbsRepository: Arjun544/flutter_init
Length of output: 35200
Fix MobX generation when Flutter Hooks are disabled.
When MobX is used without Flutter Hooks, the template renders authStore as null due to an inline comment that consumes the function initialization. This causes a null dereference on line 52 when accessing authStore.isLoading and again at line 105 in the handleLogin function.
🐛 Proposed template fix
{{else if flags.isMobX}}
// In a real app, MobX stores might be provided via Provider/GetIt.
// Here we use a locally created memoized instance for the template.
- final authStore = {{`#if` flags.usesFlutterHooks}}useMemoized{{else}}null; // Add your MobX store here{{/if}}(() => AuthStore(repository: createAuthRepository()), []);
+ {{`#if` flags.usesFlutterHooks}}
+ final authStore = useMemoized(() => AuthStore(repository: createAuthRepository()), []);
+ {{else}}
+ final authStore = AuthStore(repository: createAuthRepository());
+ {{/if}}
final isLoading = authStore.isLoading;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final authStore = {{#if flags.usesFlutterHooks}}useMemoized{{else}}null; // Add your MobX store here{{/if}}(() => AuthStore(repository: createAuthRepository()), []); | |
| final isLoading = authStore.isLoading; | |
| {{`#if` flags.usesFlutterHooks}} | |
| final authStore = useMemoized(() => AuthStore(repository: createAuthRepository()), []); | |
| {{else}} | |
| final authStore = AuthStore(repository: createAuthRepository()); | |
| {{/if}} | |
| final isLoading = authStore.isLoading; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@templates/flutter/partials/features/auth/login_screen.hbs` around lines 51 -
52, The template currently renders authStore as null when flags.usesFlutterHooks
is false because the inline comment breaks the expression; change the
conditional so authStore is always initialized: if flags.usesFlutterHooks render
useMemoized(() => AuthStore(repository: createAuthRepository()), []), else
render AuthStore(repository: createAuthRepository()); remove the inline comment
that caused the null and ensure usages like isLoading and handleLogin (which
reference authStore) work with the non-null AuthStore instance.
Comment on lines
+56
to
57
| final authStore = {{#if flags.usesFlutterHooks}}useMemoized{{else}}null; // Add your MobX store here{{/if}}(() => AuthStore(repository: createAuthRepository()), []); | ||
| final isLoading = authStore.isLoading; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find MobX template branches that render `authStore` as null when hooks are disabled.
rg -n -C2 '\{\{`#if` flags\.usesFlutterHooks\}\}useMemoized\{\{else\}\}null; // Add your MobX store here\{\{/if\}\}' templates/flutter/partials/features/authRepository: Arjun544/flutter_init
Length of output: 2084
🏁 Script executed:
# Read the signup_screen.hbs file around the problematic lines to understand context
cat -n templates/flutter/partials/features/auth/signup_screen.hbs | sed -n '50,65p'Repository: Arjun544/flutter_init
Length of output: 972
🏁 Script executed:
# Read the login_screen.hbs file around the problematic lines
cat -n templates/flutter/partials/features/auth/login_screen.hbs | sed -n '45,60p'Repository: Arjun544/flutter_init
Length of output: 1118
🏁 Script executed:
# Read the forgot_password_screen.hbs file around the problematic lines
cat -n templates/flutter/partials/features/auth/forgot_password_screen.hbs | sed -n '40,55p'Repository: Arjun544/flutter_init
Length of output: 972
🏁 Script executed:
# Check if there are any other similar patterns with null assignments in auth templates
rg -n 'null; //' templates/flutter/partials/features/authRepository: Arjun544/flutter_init
Length of output: 768
Fix MobX generation when Flutter Hooks are disabled in all auth screens.
For isMobX without hooks, the template renders invalid syntax: final authStore = null; followed by authStore.isLoading, causing compilation failure. This affects signup_screen.hbs (line 56), login_screen.hbs (line 51), and forgot_password_screen.hbs (line 46).
Restructure these templates to properly instantiate AuthStore when hooks are disabled, following the pattern already used in the NoneState branches:
🐛 Proposed template fix (apply to all 3 auth screens)
{{else if flags.isMobX}}
- final authStore = {{`#if` flags.usesFlutterHooks}}useMemoized{{else}}null; // Add your MobX store here{{/if}}(() => AuthStore(repository: createAuthRepository()), []);
+ {{`#if` flags.usesFlutterHooks}}
+ final authStore = useMemoized(() => AuthStore(repository: createAuthRepository()), []);
+ {{else}}
+ final authStore = AuthStore(repository: createAuthRepository());
+ {{/if}}
final isLoading = authStore.isLoading;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| final authStore = {{#if flags.usesFlutterHooks}}useMemoized{{else}}null; // Add your MobX store here{{/if}}(() => AuthStore(repository: createAuthRepository()), []); | |
| final isLoading = authStore.isLoading; | |
| {{`#if` flags.usesFlutterHooks}} | |
| final authStore = useMemoized(() => AuthStore(repository: createAuthRepository()), []); | |
| {{else}} | |
| final authStore = AuthStore(repository: createAuthRepository()); | |
| {{/if}} | |
| final isLoading = authStore.isLoading; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@templates/flutter/partials/features/auth/signup_screen.hbs` around lines 56 -
57, The template currently emits invalid Dart when MobX is enabled but Flutter
Hooks are disabled by assigning authStore to null; instead, change the template
logic around authStore so that when flags.usesFlutterHooks is true you keep the
existing useMemoized invocation, and when false you instantiate AuthStore
directly (AuthStore(repository: createAuthRepository())); ensure subsequent
usages like authStore.isLoading and other authStore references work with the
instantiated AuthStore; update the conditional in signup_screen.hbs (and mirror
the same change in login_screen.hbs and forgot_password_screen.hbs) to branch
between useMemoized(...) and new AuthStore(...) rather than null.
Owner
🐛 Missing Import in
|
Author
|
Hey, i have fixed the issue, |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
templates/flutter/partials/features/auth/session_provider.hbs (1)
6-6: Optional: collapse the duplicatelayer-first/mvcbranches.Both produce the identical string
../../di/service_locator.dart; merging them removes a future drift hazard if one path changes without the other.♻️ Proposed simplification
-import '{{`#if` (eq architecture "layer-first")}}../../di/service_locator.dart{{else if (eq architecture "mvc")}}../../di/service_locator.dart{{else if (eq architecture "mvvm")}}../../../di/service_locator.dart{{else}}../../../../di/service_locator.dart{{/if}}'; +import '{{`#if` (eq architecture "mvvm")}}../../../di/service_locator.dart{{else if (eq architecture "layer-first")}}../../di/service_locator.dart{{else if (eq architecture "mvc")}}../../di/service_locator.dart{{else}}../../../../di/service_locator.dart{{/if}}';(Or, cleaner, expose a precomputed
flags.serviceLocatorImportfrom the generator and just writeimport '{{flags.serviceLocatorImport}}';here and in any other partial that needs it.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/flutter/partials/features/auth/session_provider.hbs` at line 6, The import conditional duplicates the same path for "layer-first" and "mvc"; simplify by collapsing those branches into a single check (or better, replace the whole conditional with a precomputed template variable like flags.serviceLocatorImport) so the import line (the import '{{`#if` (eq architecture "layer-first")}}...') no longer repeats identical outputs—update the conditional around the import in templates/flutter/partials/features/auth/session_provider.hbs (or use flags.serviceLocatorImport) to return '../../di/service_locator.dart' for both cases in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/lib/generator/index.ts`:
- Around line 102-103: zipRootDir derived from context.flags.appSlug is too
permissive (only strips slashes); tighten sanitization in the assignment for
zipRootDir and any other uses (see occurrences around where zipRootDir is set
and passed to zipDirectory and the code block at 321-334). Replace the lax strip
with a strict sanitizer that rejects or normalizes path-traversal and
filesystem-hostile characters (remove sequences like "..", leading/trailing
dots, control chars, colons, backticks, and collapse repeated dots), allow only
a safe whitelist (e.g., alphanumerics, dashes, underscores), and if the result
is empty/invalid fall back to "flutter-app"; apply the same sanitizer to
context.flags.appSlug before passing into zipDirectory(workingDir, zipRootDir)
and other places that build archive entry names.
In `@templates/flutter/partials/features/auth/session_provider.hbs`:
- Around line 5-7: The relative import paths for the Riverpod block are wrong
for mvc and mvvm; update the import inside the flags.isRiverpod branch so that
for architecture "mvc" it imports '../../../di/service_locator.dart' and for
"mvvm" it imports '../../../../di/service_locator.dart' (leave "layer-first" and
default/feature-first as-is); this ensures the generated session_provider files
can resolve service_locator.dart and keep createAuthRepository() wiring intact.
---
Nitpick comments:
In `@templates/flutter/partials/features/auth/session_provider.hbs`:
- Line 6: The import conditional duplicates the same path for "layer-first" and
"mvc"; simplify by collapsing those branches into a single check (or better,
replace the whole conditional with a precomputed template variable like
flags.serviceLocatorImport) so the import line (the import '{{`#if` (eq
architecture "layer-first")}}...') no longer repeats identical outputs—update
the conditional around the import in
templates/flutter/partials/features/auth/session_provider.hbs (or use
flags.serviceLocatorImport) to return '../../di/service_locator.dart' for both
cases in one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 507d856b-2182-4c8c-b0a6-b36a53c8a81f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
app/lib/generator/index.tstemplates/flutter/partials/features/auth/session_provider.hbs
Comment on lines
+102
to
+103
| const zipRootDir = context.flags.appSlug || "flutter-app" | ||
| const zipBuffer = await zipDirectory(workingDir, zipRootDir) |
There was a problem hiding this comment.
Zip root sanitization is permissive — consider tightening.
zipRoot only strips / and \. Other path-traversal-ish or filesystem-hostile characters (e.g., .., leading/trailing dots, colons, control chars) pass through into archive entry names. Since appSlug is derived from user-supplied appName (only lowercased + whitespace→-), an appName like "my..app" produces a zip root of my..app/.... Not exploitable by itself (entries are still under one folder), but it can produce surprising or invalid directory names on some hosts/clients.
🛡️ Suggested hardening
- const zipRoot = rootFolderName.trim().replace(/[/\\]/g, "") || "flutter-app"
+ const zipRoot =
+ rootFolderName
+ .trim()
+ .replace(/[/\\:*?"<>|\x00-\x1f]/g, "")
+ .replace(/\.+/g, ".")
+ .replace(/^\.+|\.+$/g, "") || "flutter-app"Also applies to: 321-334
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/lib/generator/index.ts` around lines 102 - 103, zipRootDir derived from
context.flags.appSlug is too permissive (only strips slashes); tighten
sanitization in the assignment for zipRootDir and any other uses (see
occurrences around where zipRootDir is set and passed to zipDirectory and the
code block at 321-334). Replace the lax strip with a strict sanitizer that
rejects or normalizes path-traversal and filesystem-hostile characters (remove
sequences like "..", leading/trailing dots, control chars, colons, backticks,
and collapse repeated dots), allow only a safe whitelist (e.g., alphanumerics,
dashes, underscores), and if the result is empty/invalid fall back to
"flutter-app"; apply the same sanitizer to context.flags.appSlug before passing
into zipDirectory(workingDir, zipRootDir) and other places that build archive
entry names.
Comment on lines
5
to
+7
| {{#if flags.isRiverpod}} | ||
| import 'package:{{flags.appSnake}}/src/{{#if (eq architecture "layer-first")}}data/repositories/{{else if (eq architecture "mvc")}}services/{{else if (eq architecture "mvvm")}}data/repositories/{{else}}features/auth/data/repositories/{{/if}}auth_repository_impl.dart'; | ||
| import '{{#if (eq architecture "layer-first")}}../../di/service_locator.dart{{else if (eq architecture "mvc")}}../../di/service_locator.dart{{else if (eq architecture "mvvm")}}../../../di/service_locator.dart{{else}}../../../../di/service_locator.dart{{/if}}'; | ||
| {{/if}} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) Find where session_provider.hbs is wired into the generator (output path per architecture)
rg -nP -C3 'session_provider' --type=ts --type=js --type=hbs
# 2) Confirm whether service_locator.dart.hbs is unconditionally written
rg -nP -C3 'service_locator' --type=ts --type=js
fd -t f 'service_locator'
# 3) Inspect any conditions guarding emission of service_locator on dependencyInjection
rg -nP -C5 'usesGetItDi|dependencyInjection|service_locator' --type=ts --type=js
# 4) Inspect output path mapping by architecture for the auth feature/session provider
rg -nP -C5 '"layer-first"|"mvc"|"mvvm"|feature-first' --type=ts --type=jsRepository: Arjun544/flutter_init
Length of output: 25674
🏁 Script executed:
#!/bin/bash
# 1. Find where service_locator.dart.hbs is referenced in generator files
rg -nP 'service_locator' app/lib --type=ts --type=js | head -30
# 2. Check the actual overlay directory structure to confirm render paths
find templates/flutter/overlays/architecture -name '*session_provider*' -o -name '*.hbs' | grep -E '(session_provider|controllers|presentation)' | sort
# 3. Examine the generator's file copying logic to see if service_locator is conditionally copied
rg -nP -B5 -A5 'service_locator\.dart\.hbs' app/lib --type=ts --type=js
# 4. Check if there's a condition around DI template generation
rg -nP -B10 -A10 'usesGetItDi|dependencyInjection.*get_it' app/lib/generator --type=ts | head -50Repository: Arjun544/flutter_init
Length of output: 9428
🏁 Script executed:
#!/bin/bash
# Check the base template generation logic
cat app/lib/generator/index.ts | head -100
# Search for where base templates are processed
rg -nP 'copyDir|base|service_locator' app/lib/generator/index.ts
# Look for template context building to see if DI affects base templates
rg -nP -B3 -A15 'buildTemplateContext' app/lib/generator/index.tsRepository: Arjun544/flutter_init
Length of output: 5749
🏁 Script executed:
#!/bin/bash
# Read the service_locator.dart.hbs file to see its structure
cat templates/flutter/base/lib/src/di/service_locator.dart.hbs
# Also check the mvvm architecture file location for MVVM providers
cat templates/flutter/overlays/architecture/mvvm/lib/src/ui/auth/\(isProvider,isRiverpod\)@providers/session_provider.dart.hbs 2>/dev/null || echo "File not found with that exact name"
# Try listing the mvvm auth providers directory
find templates/flutter/overlays/architecture/mvvm/lib/src/ui/auth -name '*provider*' -o -name '*providers*'Repository: Arjun544/flutter_init
Length of output: 1658
Fix the relative import paths for mvc and mvvm architectures—they are too shallow and will cause import resolution failures.
The import paths have critical errors:
mvc: Currently../../di/service_locator.dart, but the generated file sits atlib/src/controllers/auth/session_provider.dart, which is 3 directories deep fromlib/src/. Correct path:../../../di/service_locator.dart.mvvm: Currently../../../di/service_locator.dart, but the generated file sits atlib/src/ui/auth/providers/session_provider.dart, which is 4 directories deep fromlib/src/. Correct path:../../../../di/service_locator.dart.
The layer-first and feature-first paths are correct.
No DI guard is needed on this import block—service_locator.dart.hbs is always generated (the base template includes it unconditionally), and the createAuthRepository() function is always defined, branching internally on flags.usesGetItDi to handle both DI modes.
As a minor improvement, the duplicate layer-first and mvc branches in the original code are redundant since they both use ../../di/, but this becomes moot once the mvc path is corrected to ../../../di/.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@templates/flutter/partials/features/auth/session_provider.hbs` around lines 5
- 7, The relative import paths for the Riverpod block are wrong for mvc and
mvvm; update the import inside the flags.isRiverpod branch so that for
architecture "mvc" it imports '../../../di/service_locator.dart' and for "mvvm"
it imports '../../../../di/service_locator.dart' (leave "layer-first" and
default/feature-first as-is); this ensures the generated session_provider files
can resolve service_locator.dart and keep createAuthRepository() wiring intact.
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
2 participants
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.
[. ] - Introduced dependencyInjection config option with values: none, get_it.
[. ] - Added DI selection UI in the Misc step and surfaced it in Review & Generate.
[ .] - Wired DI flag through generator context (usesGetItDi) so templates can branch correctly.
[. ] - Added generated DI bootstrap file: lib/src/di/service_locator.dart.
[ .] - App startup now initializes DI via initDependencies() in main.dart.
[. ] - Added conditional get_it dependency in generated pubspec.yaml.
[. ] - Refactored templates to use createAuthRepository() instead of direct AuthRepositoryImpl() construction, enabling centralized DI wiring.
[. ] - Updated docs/config reference and generated README to include DI strategy.
[. ] - Added package info metadata for get_it in the wizard info panel.
Summary by CodeRabbit
New Features
Behavior
Documentation