Skip to content

Commit a7fac0a

Browse files
committed
fix(dart): address PR #1055 review comments
Resolves all issues raised by Greptile, CodeRabbit, and Cubic bots: rules/dart/coding-style.md: - Replace ?.let (Kotlin idiom) with Dart-native null-safe operators - Soften "Never use !" to "Avoid !" — reserve for programming-error assertions - Clarify import guidance: ban relative imports, no self-contradiction rules/dart/patterns.md: - Add missing getAll/save/delete implementations to UserRepositoryImpl - Remove Uuid() from domain UseCase — inject IdGenerator instead (clean arch) - Fix GoRouter redirect: context.read<AuthState>() -> context.read<AuthCubit>() - Add refreshListenable: GoRouterRefreshStream to wire reactive auth state rules/dart/security.md: - Replace deprecated pre-v4 WebView API with WebViewController/WebViewWidget - Update bullet points to reference new v4+ API rules/dart/testing.md: - Change async => _map[key] = val to async { } block to avoid non-void return skills/dart-flutter-patterns/SKILL.md: - Add required "When to Use", "How It Works", "Examples" sections - Fix firstWhere -> firstWhereOrNull to prevent StateError - Add refreshListenable to GoRouter pattern - Add one-time retry guard to Dio 401 interceptor to prevent infinite loops README.md: - Fix directory tree "30 specialized subagents" -> "31" commands/flutter-review.md: - Add Prerequisites section (build/test/analyze must pass before review) commands/flutter-build.md: - Fix Cubit/Bloc API mix: CartCubit.add(event) -> CartCubit.addItem(item) manifests/install-profiles.json: - Add flutter-dart to full profile (fixes install manifest validator) https://claude.ai/code/session_01BQbvf8xFtJyaFiuEgdnsin
1 parent 50d40e2 commit a7fac0a

File tree

9 files changed

+148
-32
lines changed

9 files changed

+148
-32
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ everything-claude-code/
295295
| |-- plugin.json # Plugin metadata and component paths
296296
| |-- marketplace.json # Marketplace catalog for /plugin marketplace add
297297
|
298-
|-- agents/ # 30 specialized subagents for delegation
298+
|-- agents/ # 31 specialized subagents for delegation
299299
| |-- planner.md # Feature implementation planning
300300
| |-- architect.md # System design decisions
301301
| |-- tdd-guide.md # Test-driven development

commands/flutter-build.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,9 @@ state.items.add(item);
9797
```
9898
To:
9999
```dart
100-
context.read<CartCubit>().add(CartItemAdded(item));
100+
context.read<CartCubit>().addItem(item);
101+
// Note: Cubit exposes named methods (addItem, removeItem);
102+
// .add(event) is the BLoC event API — don't mix them.
101103
```
102104
103105
```

commands/flutter-review.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,18 @@ This command invokes the **flutter-reviewer** agent to review Flutter/Dart code
1414
4. **Full Review**: Apply the complete review checklist
1515
5. **Report Findings**: Output issues grouped by severity with fix guidance
1616

17+
## Prerequisites
18+
19+
Before running `/flutter-review`, ensure:
20+
1. **Build passes** — run `/flutter-build` first; a review on broken code is incomplete
21+
2. **Tests pass** — run `/flutter-test` to confirm no regressions
22+
3. **No merge conflicts** — resolve all conflicts so the diff reflects only intentional changes
23+
4. **`flutter analyze` is clean** — fix analyzer warnings before review
24+
1725
## When to Use
1826

1927
Use `/flutter-review` when:
20-
- Before submitting a PR with Flutter/Dart changes
28+
- Before submitting a PR with Flutter/Dart changes (after build and tests pass)
2129
- After implementing a new feature to catch issues early
2230
- When reviewing someone else's Flutter code
2331
- To audit a widget, state management component, or service class

manifests/install-profiles.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
"media-generation",
7171
"orchestration",
7272
"swift-apple",
73+
"flutter-dart",
7374
"agentic-patterns",
7475
"devops-infra",
7576
"supply-chain-domain",

rules/dart/coding-style.md

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -43,18 +43,28 @@ Follow Dart conventions:
4343

4444
## Null Safety
4545

46-
- Never use `!` (bang operator) — prefer `?.`, `??`, `if (x != null)`, or Dart 3 pattern matching
47-
- Use `?.let` style with null checks over forced unwrapping
46+
- Avoid `!` (bang operator) — prefer `?.`, `??`, `if (x != null)`, or Dart 3 pattern matching; reserve `!` only where a null value is a programming error and crashing is the right behaviour
4847
- Avoid `late` unless initialization is guaranteed before first use (prefer nullable or constructor init)
4948
- Use `required` for constructor parameters that must always be provided
5049

5150
```dart
52-
// BAD
51+
// BAD — crashes at runtime if user is null
5352
final name = user!.name;
5453
55-
// GOOD
54+
// GOOD — null-aware operators
5655
final name = user?.name ?? 'Unknown';
57-
final name = switch (user) { User(:final name) => name, null => 'Unknown' };
56+
57+
// GOOD — Dart 3 pattern matching (exhaustive, compiler-checked)
58+
final name = switch (user) {
59+
User(:final name) => name,
60+
null => 'Unknown',
61+
};
62+
63+
// GOOD — early-return null guard
64+
String getUserName(User? user) {
65+
if (user == null) return 'Unknown';
66+
return user.name; // promoted to non-null after the guard
67+
}
5868
```
5969

6070
## Sealed Types and Pattern Matching (Dart 3+)
@@ -138,9 +148,9 @@ await fetchData(); // or properly awaited
138148

139149
## Imports
140150

141-
- Use `package:` imports, not relative imports for consistency across the codebase
142-
- Order: dart:, package: (external), package: (internal), relative (only for same-package src files)
143-
- No unused imports — `dart analyze` enforces this
151+
- Use `package:` imports throughout — never relative imports (`../`) for cross-feature or cross-layer code
152+
- Order: `dart:`external `package:`internal `package:` (same package)
153+
- No unused imports — `dart analyze` enforces this with `unused_import`
144154

145155
## Code Generation
146156

rules/dart/patterns.md

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,26 @@ class UserRepositoryImpl implements UserRepository {
3333
return remote;
3434
}
3535
36+
@override
37+
Future<List<User>> getAll() async {
38+
final remote = await _remote.getAll();
39+
for (final user in remote) {
40+
await _local.save(user);
41+
}
42+
return remote;
43+
}
44+
3645
@override
3746
Stream<List<User>> watchAll() => _local.watchAll();
47+
48+
@override
49+
Future<void> save(User user) => _local.save(user);
50+
51+
@override
52+
Future<void> delete(String id) async {
53+
await _remote.delete(id);
54+
await _local.delete(id);
55+
}
3856
}
3957
```
4058

@@ -161,12 +179,13 @@ class GetUserUseCase {
161179
}
162180
163181
class CreateUserUseCase {
164-
const CreateUserUseCase(this._repository);
182+
const CreateUserUseCase(this._repository, this._idGenerator);
165183
final UserRepository _repository;
184+
final IdGenerator _idGenerator; // injected — domain layer must not depend on uuid package directly
166185
167186
Future<void> call(CreateUserInput input) async {
168187
// Validate, apply business rules, then persist
169-
final user = User(id: const Uuid().v4(), name: input.name, email: input.email);
188+
final user = User(id: _idGenerator.generate(), name: input.name, email: input.email);
170189
await _repository.save(user);
171190
}
172191
}
@@ -224,8 +243,10 @@ final router = GoRouter(
224243
},
225244
),
226245
],
246+
// refreshListenable re-evaluates redirect whenever auth state changes
247+
refreshListenable: GoRouterRefreshStream(authCubit.stream),
227248
redirect: (context, state) {
228-
final isLoggedIn = context.read<AuthState>().isLoggedIn;
249+
final isLoggedIn = context.read<AuthCubit>().state is AuthAuthenticated;
229250
if (!isLoggedIn && !state.matchedLocation.startsWith('/login')) {
230251
return '/login';
231252
}

rules/dart/security.md

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -101,22 +101,30 @@ if (uri != null && uri.host == 'myapp.com' && _allowedPaths.contains(uri.path))
101101

102102
## WebView Security
103103

104-
- Disable JavaScript in WebView unless explicitly required
104+
- Use `webview_flutter` v4+ (`WebViewController` / `WebViewWidget`) — the legacy `WebView` widget is removed
105+
- Disable JavaScript unless explicitly required (`JavaScriptMode.disabled`)
105106
- Validate URLs before loading — never load arbitrary URLs from deep links
106107
- Never expose Dart callbacks to JavaScript unless absolutely needed and carefully sandboxed
107-
- Use `navigationDelegate` to intercept and validate navigation requests
108+
- Use `NavigationDelegate.onNavigationRequest` to intercept and validate navigation requests
108109

109110
```dart
110-
WebView(
111-
javascriptMode: JavascriptMode.disabled, // default to disabled
112-
navigationDelegate: (request) {
113-
final uri = Uri.tryParse(request.url);
114-
if (uri == null || uri.host != 'trusted.example.com') {
115-
return NavigationDecision.prevent;
116-
}
117-
return NavigationDecision.navigate;
118-
},
119-
)
111+
// webview_flutter v4+ API (WebViewController + WebViewWidget)
112+
final controller = WebViewController()
113+
..setJavaScriptMode(JavaScriptMode.disabled) // disabled unless required
114+
..setNavigationDelegate(
115+
NavigationDelegate(
116+
onNavigationRequest: (request) {
117+
final uri = Uri.tryParse(request.url);
118+
if (uri == null || uri.host != 'trusted.example.com') {
119+
return NavigationDecision.prevent;
120+
}
121+
return NavigationDecision.navigate;
122+
},
123+
),
124+
);
125+
126+
// In your widget tree:
127+
WebViewWidget(controller: controller)
120128
```
121129

122130
## Obfuscation and Build Security

rules/dart/testing.md

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,14 @@ class FakeUserRepository implements UserRepository {
128128
Stream<List<User>> watchAll() => Stream.value(_users.values.toList());
129129
130130
@override
131-
Future<void> save(User user) async => _users[user.id] = user;
131+
Future<void> save(User user) async {
132+
_users[user.id] = user;
133+
}
132134
133135
@override
134-
Future<void> delete(String id) async => _users.remove(id);
136+
Future<void> delete(String id) async {
137+
_users.remove(id);
138+
}
135139
136140
void addUser(User user) => _users[user.id] = user;
137141
}

skills/dart-flutter-patterns/SKILL.md

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,63 @@ origin: ECC
66

77
# Dart/Flutter Patterns
88

9+
## When to Use
10+
11+
Use this skill when:
12+
- Starting a new Flutter feature and need idiomatic patterns for state management, navigation, or data access
13+
- Reviewing or writing Dart code and need guidance on null safety, sealed types, or async composition
14+
- Setting up a new Flutter project and choosing between BLoC, Riverpod, or Provider
15+
- Implementing secure HTTP clients, WebView integration, or local storage
16+
- Writing tests for Flutter widgets, Cubits, or Riverpod providers
17+
- Wiring up GoRouter with authentication guards
18+
19+
## How It Works
20+
21+
This skill provides copy-paste-ready Dart/Flutter code patterns organized by concern:
22+
1. **Null safety** — avoid `!`, prefer `?.`/`??`/pattern matching
23+
2. **Immutable state** — sealed classes, `freezed`, `copyWith`
24+
3. **Async composition** — concurrent `Future.wait`, safe `BuildContext` after `await`
25+
4. **Widget architecture** — extract to classes (not methods), `const` propagation, scoped rebuilds
26+
5. **State management** — BLoC/Cubit events, Riverpod notifiers and derived providers
27+
6. **Navigation** — GoRouter with reactive auth guards via `refreshListenable`
28+
7. **Networking** — Dio with interceptors, token refresh with one-time retry guard
29+
8. **Error handling** — global capture, `ErrorWidget.builder`, crashlytics wiring
30+
9. **Testing** — unit (BLoC test), widget (ProviderScope overrides), fakes over mocks
31+
32+
## Examples
33+
34+
```dart
35+
// Sealed state — prevents impossible states
36+
sealed class AsyncState<T> {}
37+
final class Loading<T> extends AsyncState<T> {}
38+
final class Success<T> extends AsyncState<T> { final T data; const Success(this.data); }
39+
final class Failure<T> extends AsyncState<T> { final Object error; const Failure(this.error); }
40+
41+
// GoRouter with reactive auth redirect
42+
final router = GoRouter(
43+
refreshListenable: GoRouterRefreshStream(authCubit.stream),
44+
redirect: (context, state) {
45+
final authed = context.read<AuthCubit>().state is AuthAuthenticated;
46+
if (!authed && !state.matchedLocation.startsWith('/login')) return '/login';
47+
return null;
48+
},
49+
routes: [...],
50+
);
51+
52+
// Riverpod derived provider with safe firstWhereOrNull
53+
@riverpod
54+
double cartTotal(Ref ref) {
55+
final cart = ref.watch(cartNotifierProvider);
56+
final products = ref.watch(productsProvider).valueOrNull ?? [];
57+
return cart.fold(0.0, (total, item) {
58+
final product = products.firstWhereOrNull((p) => p.id == item.productId);
59+
return total + (product?.price ?? 0) * item.quantity;
60+
});
61+
}
62+
```
63+
64+
---
65+
966
Practical, production-ready patterns for Dart and Flutter applications. Library-agnostic where possible, with explicit coverage of the most common ecosystem packages.
1067

1168
---
@@ -345,8 +402,9 @@ double cartTotal(Ref ref) {
345402
final cart = ref.watch(cartNotifierProvider);
346403
final products = ref.watch(productsProvider).valueOrNull ?? [];
347404
return cart.fold(0.0, (total, item) {
348-
final product = products.firstWhere((p) => p.id == item.productId);
349-
return total + product.price * item.quantity;
405+
// firstWhereOrNull (from collection package) avoids StateError when product is missing
406+
final product = products.firstWhereOrNull((p) => p.id == item.productId);
407+
return total + (product?.price ?? 0) * item.quantity;
350408
});
351409
}
352410
```
@@ -358,6 +416,8 @@ double cartTotal(Ref ref) {
358416
```dart
359417
final router = GoRouter(
360418
initialLocation: '/',
419+
// refreshListenable re-evaluates redirect whenever auth state changes
420+
refreshListenable: GoRouterRefreshStream(authCubit.stream),
361421
redirect: (context, state) {
362422
final isLoggedIn = context.read<AuthCubit>().state is AuthAuthenticated;
363423
final isGoingToLogin = state.matchedLocation == '/login';
@@ -402,10 +462,12 @@ dio.interceptors.add(InterceptorsWrapper(
402462
handler.next(options);
403463
},
404464
onError: (error, handler) async {
405-
if (error.response?.statusCode == 401) {
406-
// Token expired — attempt refresh
465+
// Guard against infinite retry loops: only attempt refresh once per request
466+
final isRetry = error.requestOptions.extra['_isRetry'] == true;
467+
if (!isRetry && error.response?.statusCode == 401) {
407468
final refreshed = await attemptTokenRefresh();
408469
if (refreshed) {
470+
error.requestOptions.extra['_isRetry'] = true;
409471
return handler.resolve(await dio.fetch(error.requestOptions));
410472
}
411473
}

0 commit comments

Comments
 (0)