Feat/solidart hooks#137
Conversation
|
Warning Rate limit exceeded@nank1ro has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (5)
WalkthroughAdds a new Flutter package Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Example as HookWidget (Example)
participant Hooks as solidart_hooks
participant Signals as Solidart Signals
participant Flutter as Flutter Framework
User->>Example: open screen
Example->>Hooks: useSignal(0)
Hooks->>Signals: create Signal<int>
Example->>Hooks: useComputed(() => count.value*2)
Hooks->>Signals: create Computed<int>
Example->>Hooks: useSolidartEffect(cb)
Hooks->>Signals: subscribe effect
User->>Example: tap FAB
Example->>Signals: count.value++
Signals-->>Hooks: notify listeners
Hooks-->>Example: trigger rebuild
Example->>Flutter: rebuild UI with updated values
Signals-->>Hooks: run Effect callback
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #137 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 17 17
Lines 940 940
=========================================
Hits 940 940
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
packages/solidart_hooks/solidart_hooks_example/.gitignore (1)
26-34: Same ignore tweak as the package-level .gitignore.Mirror the
.flutter-pluginsaddition here for consistency..dart_tool/ .flutter-plugins-dependencies +.flutter-plugins .pub-cache/
🧹 Nitpick comments (17)
examples/todos/analysis_options.yaml (1)
6-6: LGTM. Add a brief rationale comment for the ignore.Change is consistent with repo-wide lint updates. Consider adding a one-liner explaining why
specify_nonobvious_property_typesis ignored to avoid future drift.packages/flutter_solidart/analysis_options.yaml (1)
6-6: LGTM. Keep ignores consistent and documented.Good to align with the examples. Consider a short comment noting the reason for ignoring
specify_nonobvious_property_types.packages/flutter_solidart/lib/src/widgets/signal_builder.dart (1)
18-19: Clarified disposal note: verify default is accurate.Text says autoDispose is enabled “by default.” Please confirm Signal/Effect defaults in Flutter contexts to avoid misleading users.
packages/solidart_hooks/solidart_hooks_example/README.md (1)
1-16: README is fine for a starter example.Optional: add a link back to the solidart_hooks docs page for quick discovery.
.vscode/launch.json (1)
17-23: Fix JSON trailing comma inside object (Biome parse error).There’s a trailing comma after the last property inside the new object, which strict parsers flag. Keep the comma after
}(array element separator), remove the one after"cwd".Apply this diff:
{ "name": "solidart_hooks Example", "type": "dart", "request": "launch", "program": "lib/main.dart", - "cwd": "packages/solidart_hooks/solidart_hooks_example", + "cwd": "packages/solidart_hooks/solidart_hooks_example" },.github/workflows/publish_solidart_hooks.yaml (1)
1-18: Workflow looks correct; consider a dry-run and concurrency guard.
- Optional: add
concurrencyto avoid duplicate publishes on rapid tag pushes.- Optional: add a manual
workflow_dispatchdry-run job (pub publish --dry-run) for verification before tagging.I can draft the additions if you want.
packages/solidart_hooks/pubspec.yaml (2)
11-14: Align Flutter SDK constraint with Dart constraint or drop it.You require Dart
^3.9.2but allow Flutter>=1.17.0(which bundles much older Dart). Either:
- raise the minimum Flutter version that first includes Dart ≥ 3.9.2, or
- remove the
flutter:constraint and rely on the Dart SDK constraint.Suggested change:
environment: sdk: ^3.9.2 - flutter: ">=1.17.0"
23-27: very_good_analysis present but not used.You depend on
very_good_analysisbut analysis_options includesflutter_lints. Either switch to VGV config or remove the dev dep to avoid drift.Proposed switch in
analysis_options.yaml:-include: package:flutter_lints/flutter.yaml +include: package:very_good_analysis/analysis_options.yamldocs-v2/src/content/docs/getting-started/setup.mdx (2)
32-34: Fix typo.“functionatilies” → “functionalities”.
Apply:
-This package extends the functionatilies of `solidart` and is meant to be used only in Flutter applications. +This package extends the functionalities of `solidart` and is meant to be used only in Flutter applications.
49-49: Tighten wording for hooks description.Improve clarity/capitalization.
Apply:
-`flutter_hooks` bindings for solidart good for ephemeral state. +Flutter Hooks bindings for Solidart, suitable for ephemeral state.packages/solidart_hooks/.gitignore (1)
26-34: Add.flutter-pluginsto ignores.You ignore
.flutter-plugins-dependenciesbut not.flutter-plugins. Add it to avoid local plugin noise.Apply:
.dart_tool/ .flutter-plugins-dependencies +.flutter-plugins .pub-cache/packages/solidart_hooks/solidart_hooks_example/pubspec.yaml (1)
1-4: Polish the package metadata.Replace the placeholder description with something specific (e.g., “Example app for solidart_hooks”) to improve discoverability in IDEs.
packages/solidart_hooks/README.md (2)
35-35: Clarify ownership/disposal semantics.Document that signals created via
useSignal/useComputedare owned by the hook and disposed on unmount, whileuseExistingSignalmust not dispose the passed signal. Align this note with the implementation fix below.
51-52: Copy edit.“The value will auto rebuild the widget…” → “The value will auto‑rebuild the widget…”
docs-v2/src/content/docs/flutter/solidart_hooks.mdx (1)
40-40: Clarify ownership/disposal semantics in docs.Add a short callout: signals created by hooks are disposed on unmount;
useExistingSignaldoes not dispose the provided signal.packages/solidart_hooks/lib/solidart_hooks.dart (2)
147-155: Initialize superclass first and guardsetState.Call
super.initHook()first and guardsetStatewithmountedto avoid rare teardown races.- @override - void initHook() { - // ignore: implicit_call_tearoffs - _cleanup = Effect(() { - _instance.value; - setState(() {}); - }); - super.initHook(); - } + @override + void initHook() { + super.initHook(); + // ignore: implicit_call_tearoffs + _cleanup = Effect(() { + _instance.value; + if (mounted) setState(() {}); + }); + }
157-161: Dispose in the right order and only if owned.Unsubscribe the effect first, then dispose the instance if we own it.
@override void dispose() { - _instance.dispose(); - _cleanup.call(); + _cleanup.call(); + if (hook.disposeOnUnmount) { + _instance.dispose(); + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
.github/workflows/publish_solidart_hooks.yaml(1 hunks).gitignore(1 hunks).vscode/launch.json(1 hunks)docs-v2/src/content/docs/flutter/solidart_hooks.mdx(1 hunks)docs-v2/src/content/docs/getting-started/setup.mdx(3 hunks)examples/counter/pubspec.yaml(1 hunks)examples/github_search/pubspec.yaml(1 hunks)examples/todos/analysis_options.yaml(1 hunks)examples/todos/pubspec.yaml(1 hunks)examples/toggle_theme/pubspec.yaml(1 hunks)packages/flutter_solidart/analysis_options.yaml(1 hunks)packages/flutter_solidart/example/pubspec.yaml(1 hunks)packages/flutter_solidart/lib/src/widgets/signal_builder.dart(1 hunks)packages/flutter_solidart/pubspec.yaml(1 hunks)packages/solidart/pubspec.yaml(1 hunks)packages/solidart_devtools_extension/pubspec.yaml(1 hunks)packages/solidart_hooks/.gitignore(1 hunks)packages/solidart_hooks/.metadata(1 hunks)packages/solidart_hooks/README.md(1 hunks)packages/solidart_hooks/analysis_options.yaml(1 hunks)packages/solidart_hooks/lib/solidart_hooks.dart(1 hunks)packages/solidart_hooks/pubspec.yaml(1 hunks)packages/solidart_hooks/solidart_hooks_example/.gitignore(1 hunks)packages/solidart_hooks/solidart_hooks_example/.metadata(1 hunks)packages/solidart_hooks/solidart_hooks_example/README.md(1 hunks)packages/solidart_hooks/solidart_hooks_example/analysis_options.yaml(1 hunks)packages/solidart_hooks/solidart_hooks_example/lib/main.dart(1 hunks)packages/solidart_hooks/solidart_hooks_example/pubspec.yaml(1 hunks)packages/solidart_lint/pubspec.yaml(1 hunks)pubspec.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
packages/solidart_lint/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When changing linting rules, test the
packages/solidart_lint/package examples
Files:
packages/solidart_lint/pubspec.yaml
examples/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Build and run example applications (counter, todos, github_search, toggle_theme) to validate real user scenarios
Files:
examples/toggle_theme/pubspec.yamlexamples/todos/pubspec.yamlexamples/counter/pubspec.yamlexamples/todos/analysis_options.yamlexamples/github_search/pubspec.yaml
packages/{solidart,flutter_solidart}/lib/**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
packages/{solidart,flutter_solidart}/lib/**/*.dart: Use Signals for state, Effects for side effects, and Computed for derived values in library code
Useuntracked()for reads that should not trigger reactivity
Ensure proper memory management: Signals auto-dispose in Flutter contexts; manually dispose in pure Dart code
Files:
packages/flutter_solidart/lib/src/widgets/signal_builder.dart
packages/flutter_solidart/{lib,test}/**/*.dart
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For Flutter Solidart changes, ensure widget tests cover
SignalBuilder,Provider, andShowbehaviors
Files:
packages/flutter_solidart/lib/src/widgets/signal_builder.dart
docs-v2/src/content/docs/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
When making API changes, update documentation in
docs-v2/src/content/docs/
Files:
docs-v2/src/content/docs/getting-started/setup.mdxdocs-v2/src/content/docs/flutter/solidart_hooks.mdx
docs-v2/**
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
For documentation changes, build with
npm run buildand serve locally withnpm run devfor verification
Files:
docs-v2/src/content/docs/getting-started/setup.mdxdocs-v2/src/content/docs/flutter/solidart_hooks.mdx
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/flutter_solidart/test/flutter_solidart_test.dart : After Flutter integration changes, always run `packages/flutter_solidart/test/flutter_solidart_test.dart`
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/flutter_solidart/{lib,test}/**/*.dart : For Flutter Solidart changes, ensure widget tests cover `SignalBuilder`, `Provider`, and `Show` behaviors
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/solidart_test.dart : After Signal changes, always run `packages/solidart/test/solidart_test.dart`
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Ensure proper memory management: Signals auto-dispose in Flutter contexts; manually dispose in pure Dart code
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart_lint/** : When changing linting rules, test the `packages/solidart_lint/` package examples
Applied to files:
packages/solidart_lint/pubspec.yamlpubspec.yaml
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/flutter_solidart/test/flutter_solidart_test.dart : After Flutter integration changes, always run `packages/flutter_solidart/test/flutter_solidart_test.dart`
Applied to files:
.vscode/launch.jsonexamples/toggle_theme/pubspec.yamlpackages/flutter_solidart/example/pubspec.yamlpackages/solidart_hooks/analysis_options.yamlpackages/flutter_solidart/pubspec.yamlpackages/solidart_devtools_extension/pubspec.yamlpackages/solidart_hooks/solidart_hooks_example/pubspec.yamlpackages/solidart_hooks/.metadataexamples/todos/pubspec.yamldocs-v2/src/content/docs/getting-started/setup.mdxpackages/solidart_hooks/pubspec.yamlpackages/solidart_hooks/solidart_hooks_example/README.mdpackages/solidart_hooks/solidart_hooks_example/.metadatapackages/solidart_hooks/solidart_hooks_example/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/flutter_solidart/{lib,test}/**/*.dart : For Flutter Solidart changes, ensure widget tests cover `SignalBuilder`, `Provider`, and `Show` behaviors
Applied to files:
.vscode/launch.jsonpackages/flutter_solidart/example/pubspec.yamlpackages/solidart_hooks/analysis_options.yamlpubspec.yamlpackages/flutter_solidart/pubspec.yamlpackages/solidart_devtools_extension/pubspec.yamlpackages/solidart_hooks/solidart_hooks_example/pubspec.yamlpackages/flutter_solidart/lib/src/widgets/signal_builder.dartexamples/todos/pubspec.yamldocs-v2/src/content/docs/getting-started/setup.mdxpackages/solidart_hooks/pubspec.yamlpackages/solidart_hooks/README.mddocs-v2/src/content/docs/flutter/solidart_hooks.mdxpackages/solidart_hooks/solidart_hooks_example/README.mdpackages/solidart_hooks/.gitignorepackages/solidart_hooks/solidart_hooks_example/.metadatapackages/solidart_hooks/lib/solidart_hooks.dartpackages/solidart_hooks/solidart_hooks_example/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Run `flutter analyze packages` to analyze all packages; accept the known 4 lint issues as non-errors
Applied to files:
examples/toggle_theme/pubspec.yamlpackages/flutter_solidart/example/pubspec.yamlpackages/solidart_hooks/analysis_options.yamlpackages/solidart_devtools_extension/pubspec.yamlexamples/counter/pubspec.yamlpackages/solidart_hooks/solidart_hooks_example/analysis_options.yamlexamples/github_search/pubspec.yaml
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/solidart_test.dart : After Signal changes, always run `packages/solidart/test/solidart_test.dart`
Applied to files:
packages/flutter_solidart/example/pubspec.yamlpackages/solidart_hooks/solidart_hooks_example/pubspec.yamlpackages/flutter_solidart/lib/src/widgets/signal_builder.dartexamples/todos/pubspec.yamlpackages/solidart_hooks/pubspec.yamlpackages/solidart_hooks/solidart_hooks_example/README.mdpackages/solidart_hooks/lib/solidart_hooks.dartpackages/solidart_hooks/solidart_hooks_example/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Ensure proper memory management: Signals auto-dispose in Flutter contexts; manually dispose in pure Dart code
Applied to files:
packages/flutter_solidart/lib/src/widgets/signal_builder.dartdocs-v2/src/content/docs/getting-started/setup.mdxpackages/solidart_hooks/solidart_hooks_example/README.mdpackages/solidart_hooks/lib/solidart_hooks.dartpackages/solidart_hooks/solidart_hooks_example/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Use Signals for state, Effects for side effects, and Computed for derived values in library code
Applied to files:
packages/flutter_solidart/lib/src/widgets/signal_builder.dartdocs-v2/src/content/docs/getting-started/setup.mdxpackages/solidart_hooks/pubspec.yamlpackages/solidart_hooks/README.mddocs-v2/src/content/docs/flutter/solidart_hooks.mdxpackages/solidart_hooks/lib/solidart_hooks.dartpackages/solidart_hooks/solidart_hooks_example/lib/main.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Use `untracked()` for reads that should not trigger reactivity
Applied to files:
docs-v2/src/content/docs/getting-started/setup.mdx
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to coverage/lcov.info : Include `coverage/lcov.info` in CI reports
Applied to files:
.gitignore
🪛 Biome (2.1.2)
.vscode/launch.json
[error] 23-23: Expected a property but instead found '}'.
Expected a property here.
(parse)
⏰ 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: build
🔇 Additional comments (20)
examples/github_search/pubspec.yaml (1)
52-52: flutter_lints ^6.0.0: verify no new issues.
Automated check failed: sandbox missing Flutter ("flutter: command not found"). Run locally (from repo root) and paste analyzer output:pushd examples/github_search flutter pub get flutter analyze popdpackages/solidart_devtools_extension/pubspec.yaml (1)
25-25: Run flutter analyze for packages/solidart_devtools_extension (sanity-check flutter_lints ^6.0.0)
Sandbox CI lacked Flutter SDK ('flutter: command not found'). Run from repo root and paste the output:
cd packages/solidart_devtools_extension && flutter pub get && flutter analyzeexamples/counter/pubspec.yaml (1)
51-51: flutter_lints ^6.0.0: analyzer check — manual verification requiredSandbox lacked Flutter (error: "flutter: command not found"). Run locally in examples/counter and paste the output:
flutter pub get && flutter analyze
packages/flutter_solidart/pubspec.yaml (1)
28-28: VGA v9 in Flutter package — manual verification requiredSandbox verification failed: "/bin/bash: line 3: dart: command not found". Confirm SignalBuilder/Provider/Show widget tests pass.
Location: packages/flutter_solidart/pubspec.yaml (line 28 — very_good_analysis: ^9.0.0)
Run locally: dart pub get --directory packages/flutter_solidart && flutter analyze packages/flutter_solidart && flutter test packages/flutter_solidart/test/flutter_solidart_test.dart
packages/flutter_solidart/example/pubspec.yaml (1)
49-49: flutter_lints ^6.0.0: check for new warnings.
Likely compatible with Dart ^3.6.0; confirm no new violations in the example.Verification here failed: 'flutter' not found. From the repo root run and attach output:
cd packages/flutter_solidart/example flutter pub get flutter analyzepackages/solidart_lint/pubspec.yaml (1)
23-23: Upgrade to lints ^6.0.0 — sandbox verification failed; run compatibility checks locally/CI.Sandbox error: "/bin/bash: line 3: dart: command not found" — analysis/tests could not be executed.
File: packages/solidart_lint/pubspec.yaml (lints: ^6.0.0)Run (from repo root) and confirm the plugin loads, analyzer/custom_lint compatibility, and examples pass:
#!/bin/bash set -euo pipefail dart pub get --directory packages/solidart_lint dart analyze packages/solidart_lint dart test packages/solidart_lint # If the lint package has example projects, analyze them too. fd -t d '^examples?$' packages/solidart_lint -x bash -lc 'pushd "{}" >/dev/null; dart pub get || true; dart analyze || true; popd >/dev/null'packages/solidart/pubspec.yaml (1)
24-24: VGA v9 in core package: include present; analysis & tests couldn't be executed here — manual verification required.
- packages/solidart/analysis_options.yaml includes "package:very_good_analysis/analysis_options.yaml" (line 5).
- packages/solidart/pubspec.yaml declares very_good_analysis: ^9.0.0.
- Sandbox error: "/bin/bash: line 4: dart: command not found" — could not run dart pub get / analyze / test.
Run locally/CI and report results:
dart pub get --directory packages/solidart
dart analyze packages/solidart
dart test packages/solidart/test/solidart_test.dartexamples/todos/pubspec.yaml (1)
23-23: Verify analyzer + CI after VGA v9 bumpSandbox lacked dart/flutter so analysis couldn't be run — run locally or in CI and paste results:
#!/bin/bash set -euo pipefail dart --version flutter --version # Verify analysis_options include and analyze the example pushd examples/todos >/dev/null rg -n 'include:\s*package:very_good_analysis/analysis_options' -S analysis_options.yaml || true flutter pub get flutter analyze popd >/dev/null
- Confirm examples/todos/pubspec.yaml (line ~23) uses very_good_analysis: ^9.0.0 and that analysis_options (example or repo-level) includes package:very_good_analysis/analysis_options.
.gitignore (1)
37-37: Linux platform ignore: confirm no platform sources need committing.Matches existing ignores for other platforms. Just verify no package/plugin in this repo requires committing
linux/scaffolding.examples/toggle_theme/pubspec.yaml (1)
49-49: Bump to flutter_lints ^6.0.0 looks fine.SDK constraint (^3.6.0) is compatible. Ensure the example’s analysis_options picks up the v6 ruleset (or intended superset) without conflicts.
packages/solidart_hooks/analysis_options.yaml (1)
10-26: LGTM on baseline lints.Config is standard and consistent with Flutter packages. No blockers.
packages/solidart_hooks/solidart_hooks_example/analysis_options.yaml (1)
10-26: LGTM.Example inherits Flutter lints; fine for samples.
packages/solidart_hooks/pubspec.yaml (1)
3-4: Docs vs publish settings: ensure release plan is consistent.Docs instruct
flutter pub add solidart_hooks, butpublish_to: noneblocks publishing. Confirm whether docs merge waits for the publish workflow to remove this flag.packages/solidart_hooks/solidart_hooks_example/.metadata (1)
10-20: LGTM.Standard Flutter metadata for the example app.
docs-v2/src/content/docs/getting-started/setup.mdx (2)
97-102: Document or remove nonstandard lintspecify_nonobvious_property_typesfrom the docs sample.This rule is not a core Dart lint — it appears in the repo (packages/flutter_solidart/analysis_options.yaml and examples/todos/analysis_options.yaml) and examples/todos enables the custom_lint plugin. For the docs snippet either include the required plugin (so users won’t get “unrecognized lint rule” warnings) or replace/remove the rule.
7-12: Verify docs build & anchors (manual verification required).Build failed in the verification environment (missing package.json). From the repo root run in docs-v2/: npm install && npm run build && npm run dev — open Setup (docs-v2/src/content/docs/getting-started/setup.mdx lines 7–12) and confirm the TOC updates and that the #solidart_hooks anchor resolves.
packages/solidart_hooks/.metadata (1)
10-11: Confirm project_type (app vs package) for packages/solidart_hookspackages/solidart_hooks/.metadata (line 10) contains
project_type: app; library packages normally useproject_type: package(e.g., packages/flutter_solidart/.metadata ispackage). Confirm the intended type and change topackageif this is a library to avoid tooling quirks.packages/solidart_hooks/solidart_hooks_example/lib/main.dart (1)
9-19: LGTM.Example app wiring looks correct and minimal. Good demo for the hooks API.
docs-v2/src/content/docs/flutter/solidart_hooks.mdx (1)
1-6: Build docs locally and verify the new MDX page renders & links.
Run: npm ci && npm run build && npm run dev
Affected file: docs-v2/src/content/docs/flutter/solidart_hooks.mdx (lines 1–6)packages/solidart_hooks/solidart_hooks_example/pubspec.yaml (1)
6-10: Confirm workspace resolution & CI toolchain for Dart ^3.9.2
- dependency_overrides found at examples/todos/pubspec.yaml:25 and packages/solidart_lint/example/pubspec_overrides.yaml:1 — these can force package versions across the workspace.
- flutter_hooks is pinned to ^0.21.3+1 in packages/solidart_hooks/pubspec.yaml:21 and packages/solidart_hooks/solidart_hooks_example/pubspec.yaml:14.
- Action: ensure CI matrix includes Dart 3.9.2 (and matching Flutter versions), run a workspace resolution check (e.g., dart pub upgrade --dry-run or flutter pub get at repo root) to confirm no downgrades, and remove or document dependency_overrides if they are unintentional.
Deploying solidart-docs with
|
| Latest commit: |
671c657
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c8eed204.solidart.pages.dev |
| Branch Preview URL: | https://feat-solidart-hooks.solidart.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
packages/solidart_hooks/lib/solidart_hooks.dart (5)
1-1: Invalid library directive — breaks compilation.Replace or remove the bare
library;.-library; +library solidart_hooks;
16-53: Leak risk: hooks should always dispose signals they create.
disposeOnUnmount: autoDispose ?? trueskips disposing whenautoDispose == false. Ownership is independent of Solidart’s autoDispose flag.return use( - _SignalHook('useSignal', target, disposeOnUnmount: autoDispose ?? true), + _SignalHook('useSignal', target, disposeOnUnmount: true), );
56-95: Stale selector captured inuseComputed+ same dispose issue.Pass a wrapper to always call the latest selector and always dispose owned computed.
- final instance = useRef(selector); - instance.value = selector; + final instance = useRef(selector); + instance.value = selector; final target = useMemoized( () => Computed<T>( - instance.value, + () => instance.value(), autoDispose: autoDispose, name: name, equals: equals, trackInDevTools: trackInDevTools, comparator: comparator, trackPreviousValue: trackPreviousValue, ), [], ); return use( - _SignalHook('useComputed', target, disposeOnUnmount: autoDispose ?? true), + _SignalHook('useComputed', target, disposeOnUnmount: true), );
97-136: Stale callback inuseSolidartEffect.Effect captures the initial
cb. Wrap to read the latest ref.final instance = useRef(cb); instance.value = cb; useEffect( () => Effect( - instance.value, + () => instance.value(), onError: onError, name: name, delay: delay, autoDispose: autoDispose, detach: detach, autorun: autorun, ).dispose, [], );
153-184: Hook state doesn’t handle instance swaps and misses super.dispose().
_instanceislate final, souseExistingSignalwon’t react to a new signal instance.dispose()doesn’t callsuper.dispose().Fix by making
_instancemutable, handlingdidUpdateHook, resubscribing, and callingsuper.dispose().-class _SignalHookState<T, S extends ReadSignal<T>> - extends HookState<S, _SignalHook<T, S>> { - late final _instance = hook.initialData; - late DisposeEffect _cleanup; +class _SignalHookState<T, S extends ReadSignal<T>> + extends HookState<S, _SignalHook<T, S>> { + late S _instance; + DisposeEffect? _cleanup; @override void initHook() { - // ignore: implicit_call_tearoffs - _cleanup = Effect(() { - _instance.value; - setState(() {}); - }); + _instance = hook.initialData; + // ignore: implicit_call_tearoffs + _cleanup = Effect(() { + _instance.value; + setState(() {}); + }); super.initHook(); } + @override + void didUpdateHook(_SignalHook<T, S> oldHook) { + if (!identical(oldHook.initialData, hook.initialData)) { + _cleanup?.call(); + _instance = hook.initialData; + // ignore: implicit_call_tearoffs + _cleanup = Effect(() { + _instance.value; + setState(() {}); + }); + } + super.didUpdateHook(oldHook); + } + @override void dispose() { - _cleanup.call(); + _cleanup?.call(); if (hook.disposeOnUnmount) { - _instance.dispose(); + _instance.dispose(); } + super.dispose(); } @override - S build(BuildContext context) => _instance; + S build(BuildContext context) => _instance;
🧹 Nitpick comments (3)
packages/solidart_hooks/.gitignore (1)
26-36: Ignore pubspec.lock in package directory.Prevents accidental commit of lockfile for a published package.
# Flutter/Dart/Pub related **/doc/api/ **/ios/Flutter/.last_build_id .dart_tool/ .flutter-plugins-dependencies .flutter-plugins .pub-cache/ .pub/ +/pubspec.lock /build/ /coverage/packages/solidart_hooks/README.md (2)
39-39: Clarify disposal semantics for externally-owned signals.
useExistingSignalshould not dispose external signals; spell it out.-All of the signals and effects created will get cleaned up when the widget gets unmounted. +Signals and effects created by hooks are disposed on unmount. +`useExistingSignal` does not dispose externally‑owned signals.
55-56: Tighten phrasing.Minor grammar polish.
-The value will auto rebuild the widget when it changes. +The widget will automatically rebuild when the value changes.Also applies to: 72-73, 109-109
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.vscode/launch.json(1 hunks)docs-v2/src/content/docs/getting-started/setup.mdx(3 hunks)packages/solidart_hooks/.gitignore(1 hunks)packages/solidart_hooks/README.md(1 hunks)packages/solidart_hooks/lib/solidart_hooks.dart(1 hunks)packages/solidart_hooks/pubspec.yaml(1 hunks)packages/solidart_hooks/solidart_hooks_example/.gitignore(1 hunks)packages/solidart_hooks/solidart_hooks_example/analysis_options.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/solidart_hooks/pubspec.yaml
- .vscode/launch.json
- docs-v2/src/content/docs/getting-started/setup.mdx
- packages/solidart_hooks/solidart_hooks_example/.gitignore
- packages/solidart_hooks/solidart_hooks_example/analysis_options.yaml
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/flutter_solidart/test/flutter_solidart_test.dart : After Flutter integration changes, always run `packages/flutter_solidart/test/flutter_solidart_test.dart`
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/flutter_solidart/{lib,test}/**/*.dart : For Flutter Solidart changes, ensure widget tests cover `SignalBuilder`, `Provider`, and `Show` behaviors
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/solidart_test.dart : After Signal changes, always run `packages/solidart/test/solidart_test.dart`
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Use Signals for state, Effects for side effects, and Computed for derived values in library code
Applied to files:
packages/solidart_hooks/README.mdpackages/solidart_hooks/lib/solidart_hooks.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/flutter_solidart/{lib,test}/**/*.dart : For Flutter Solidart changes, ensure widget tests cover `SignalBuilder`, `Provider`, and `Show` behaviors
Applied to files:
packages/solidart_hooks/README.mdpackages/solidart_hooks/.gitignorepackages/solidart_hooks/lib/solidart_hooks.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/solidart_test.dart : After Signal changes, always run `packages/solidart/test/solidart_test.dart`
Applied to files:
packages/solidart_hooks/lib/solidart_hooks.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/solidart/test/**/*.dart : Write unit tests covering Signal operations and state transitions
Applied to files:
packages/solidart_hooks/lib/solidart_hooks.dart
📚 Learning: 2025-09-10T15:36:53.390Z
Learnt from: CR
PR: nank1ro/solidart#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-10T15:36:53.390Z
Learning: Applies to packages/{solidart,flutter_solidart}/lib/**/*.dart : Ensure proper memory management: Signals auto-dispose in Flutter contexts; manually dispose in pure Dart code
Applied to files:
packages/solidart_hooks/lib/solidart_hooks.dart
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
packages/solidart_hooks/lib/solidart_hooks.dart (1)
138-151: Good: explicit disposal policy per hook.
1 participant
closes #109
Summary by CodeRabbit
New Features
Documentation
Chores