Add StacScreen annotation and annotations barrel; re-export in barrels #359
Conversation
WalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Consumer code
participant Top as stac_core.dart (barrel)
participant Core as core/core.dart (barrel)
participant Anns as annotations/annotations.dart (barrel)
participant Screen as stac_screen.dart
Note over App,Top: Prior usage: imported barrels to access widgets/actions
App->>Top: import 'package:stac_core/stac_core.dart'
Top->>Core: re-exports existing core symbols
Core->>Top: (re-exported)
Note over Top,Anns: New re-export chain
Top->>Anns: re-export annotations/annotations.dart
Core->>Anns: re-export ../annotations/annotations.dart
Anns->>Screen: export 'stac_screen.dart'
App->>Screen: uses @StacScreen(...) via top-level barrel
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (3)
Comment |
coderabbitai
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stac_core/lib/core/core.dart (1)
1-1: Invalidlibrary;directiveSame issue here: remove or name the library.
Apply this diff:
-library; +
🧹 Nitpick comments (2)
packages/stac_core/lib/stac_core.dart (1)
4-4: Export looks good; watch for redundant re‑exportsThis is fine. Note core/core.dart also re‑exports annotations; consider keeping only one path to avoid redundancy in the public API surface.
packages/stac_core/lib/core/core.dart (1)
5-5: Re‑export is OK, but it duplicates top‑level exportThis makes annotations available via core/, which is fine. Since stac_core.dart also exports annotations, decide if you want both or a single canonical export path.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/stac_core/lib/annotations/annotations.dart(1 hunks)packages/stac_core/lib/annotations/stac_screen.dart(1 hunks)packages/stac_core/lib/core/core.dart(1 hunks)packages/stac_core/lib/stac_core.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: analyze
🔇 Additional comments (3)
packages/stac_core/lib/annotations/annotations.dart (1)
3-3: LGTM: barrel exportExporting
stac_screen.dartfrom the annotations barrel looks good.packages/stac_core/lib/annotations/stac_screen.dart (2)
14-20: Solid minimal annotationConst, final field, and clear docs are good.
14-20: Constrain usage and validate input (non‑breaking enhancements)Limit where the annotation can be applied and guard against empty names; add the meta import and dependency, and update any functions that should be annotated (examples/stac_gallery/lib/main.dart:60 currently returns StacWidget without @StacScreen).
Apply this diff to packages/stac_core/lib/annotations/stac_screen.dart:
+@Target({TargetKind.method, TargetKind.function}) class StacScreen { /// Creates a [StacScreen] annotation with the given screen name. - const StacScreen(this.name); + const StacScreen(this.name) : assert(name.isNotEmpty, 'StacScreen name must be non-empty'); /// The name identifier for this screen. final String name; }Add at the top of that file:
import 'package:meta/meta_meta.dart';Add
metato packages/stac_core/pubspec.yaml under dependencies. Annotate any StacWidget-returning functions that represent screens (e.g., examples/stac_gallery/lib/main.dart:60).
| @@ -0,0 +1,20 @@ | |||
| /// Annotation to mark methods that return StacWidget instances. | |||
| /// | |||
| /// This annotation is used to identify screen-level widgets in the STAC framework. | |||
There was a problem hiding this comment.
Do not make Stac all caps, use "Stac"
| const StacScreen(this.name); | ||
|
|
||
| /// The name identifier for this screen. | ||
| final String name; |
There was a problem hiding this comment.
Should we call this variable "routeName" as we are using the same name in Stac() class when fetching the screen? 🤔
There was a problem hiding this comment.
routename conflicts with flutter routes no?
maybe we should make it stacRouteName @divyanshub024?
Potatomonsta
force-pushed
the
mn/stac-screen-annotation-cli
branch
from
b4adb45 to
2203430
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stac_core/lib/core/core.dart (1)
1-1: Unnamedlibrary;directive is invalid Dart.Either remove the directive or give it a proper name (e.g.,
library stac_core.core;). Leaving it aslibrary;will not parse.
🧹 Nitpick comments (2)
packages/stac_core/lib/annotations/stac_screen.dart (1)
1-13: Polish dartdoc: link types and tighten example.
- Link to symbols so dartdoc auto-links.
- Use a concise arrow function in the example.
Apply this diff:
-/// Annotation to mark methods that return StacWidget instances. +/// Annotation to mark methods that return [StacWidget] instances. @@ -/// Methods that return StacWidget should be annotated with this to indicate +/// Methods that return [StacWidget] should be annotated with this to indicate @@ -/// StacWidget buildHomeScreen() { -/// return StacWidget(jsonData: {'type': 'scaffold', 'body': '...'}); -/// } +/// StacWidget buildHomeScreen() => +/// StacWidget(jsonData: {'type': 'scaffold', 'body': '...'});packages/stac_core/lib/core/core.dart (1)
5-5: Remove redundant annotation export: In packages/stac_core/lib/core/core.dart (line 5), dropexport '../annotations/annotations.dart';— it’s already re-exported in lib/stac_core.dart.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/stac_core/lib/annotations/annotations.dart(1 hunks)packages/stac_core/lib/annotations/stac_screen.dart(1 hunks)packages/stac_core/lib/core/core.dart(1 hunks)packages/stac_core/lib/stac_core.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/stac_core/lib/annotations/annotations.dart
- packages/stac_core/lib/stac_core.dart
🔇 Additional comments (1)
packages/stac_core/lib/annotations/stac_screen.dart (1)
19-19: Name alignment: considerrouteName(orstacRouteName) for consistency.Prior feedback suggested
routeNameto match the Stac() class usage; author raised Flutter route collision concerns and proposedstacRouteName. Pick one and use consistently across docs/APIs.Example diff if choosing
routeName:- const StacScreen(this.screenName); + const StacScreen(this.routeName); @@ - final String screenName; + final String routeName;
| class StacScreen { | ||
| /// Creates a [StacScreen] annotation with the given screen name. | ||
| const StacScreen(this.screenName); | ||
|
|
||
| /// The name identifier for this screen. | ||
| final String screenName; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Restrict where the annotation can be applied (methods only).
Add @Target({TargetKind.method}) to prevent accidental usage on classes/fields. Requires package:meta.
If meta isn’t already a dep, add it. Then:
+import 'package:meta/meta_meta.dart';
+
+@Target({TargetKind.method})
class StacScreen {
/// Creates a [StacScreen] annotation with the given screen name.
const StacScreen(this.screenName);
@@
/// The name identifier for this screen.
final String screenName;
}Verify meta availability:
🏁 Script executed:
#!/bin/bash
rg -nP '^\s*meta\s*:' pubspec.yaml -n -C2 || rg -nP 'package:meta' -C2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Show where dependencies are declared and check for any "meta" entries
rg -nP '^(dependencies|dev_dependencies):' -n -C3 pubspec.yaml
rg -nP '^\s*meta\s*:' -n -C3 pubspec.yamlLength of output: 86
🏁 Script executed:
#!/bin/bash
# Locate all pubspec.yaml files and check for meta dependency
find . -type f -name pubspec.yaml | while read f; do
echo "== $f =="
rg -nP '^\s*(dependencies|dev_dependencies):' -C3 "$f"
rg -nP '^\s*meta\s*:' -C3 "$f"
doneLength of output: 2797
Restrict annotation to methods only
Add meta to packages/stac_core/pubspec.yaml:
dependencies:
meta: ^1.10.0Then update lib/annotations/stac_screen.dart:
+import 'package:meta/meta_meta.dart';
+
@Target({TargetKind.method})
class StacScreen {
/// Creates a [StacScreen] annotation with the given screen name.
const StacScreen(this.screenName);
@@
/// The name identifier for this screen.
final String screenName;
}🤖 Prompt for AI Agents
In packages/stac_core/lib/annotations/stac_screen.dart around lines 14 to 20,
the StacScreen annotation should be restricted to methods only; add the meta
package to packages/stac_core/pubspec.yaml (dependencies: meta: ^1.10.0), run
pub get, then update stac_screen.dart to import package:meta/meta.dart and
annotate the StacScreen class with @Target({TargetKind.method}) so the
annotation can only be used on methods; keep the const constructor and existing
field unchanged.
…ation for marking methods that return StacWidget instances.
- Create new `packages/stac_core/lib/annotations/` directory
- Add `stac_screen.dart` with @StacScreen annotation class
- Add `annotations.dart` export file for the annotations package
- Update `core.dart` to export annotations package
```dart
import 'package:stac_core/stac_core.dart';
@StacScreen('home')
StacWidget buildHomeScreen() => StacWidget();
```
Potatomonsta
force-pushed
the
mn/stac-screen-annotation-cli
branch
from
2203430 to
ecc851d
Compare
2 participants
Description
packages/stac_core/lib/annotations/directorystac_screen.dartwith @StacScreen annotation classannotations.dartexport file for the annotations packagecore.dartto export annotations packageBreaking Changes:
Usage:
Type of Change
Summary by CodeRabbit