fix: effect re-entrancy#174
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughEffect.run() now opens a reactive batch after switching to the internal subscriber and closes it in finally (before restoring the previous subscriber), preventing signal writes during the first effect run from re-entering the effect. Tests validate both re-entrancy and late-initialization edge cases; package versions and changelogs are updated. ChangesEffect re-entrancy batching fix
🎯 2 (Simple) | ⏱️ ~10 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #174 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 18 18
Lines 937 939 +2
=========================================
+ Hits 937 939 +2
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/solidart/test/reentrancy_test.dart (1)
4-72: ⚡ Quick winConsider resetting
SolidartConfig.autoDisposein tearDown.Both test cases modify
SolidartConfig.autoDisposebut don't reset it, which could affect test isolation if tests run sequentially in the same process.♻️ Proposed fix to reset config after each test
void main() { group('effect re-entrancy during initial run', () { + late bool originalAutoDispose; + + setUp(() { + originalAutoDispose = SolidartConfig.autoDispose; + }); + + tearDown(() { + SolidartConfig.autoDispose = originalAutoDispose; + }); + test( '''a signal write during the effect first run does not re-enter the effect callback while it is still on the stack''', () {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/solidart/test/reentrancy_test.dart` around lines 4 - 72, Each test mutates SolidartConfig.autoDispose and doesn't restore it; capture the original value (e.g. var _prev = SolidartConfig.autoDispose) at the start of each test and register an addTearDown that restores it (addTearDown(() => SolidartConfig.autoDispose = _prev)); update both tests (the ones creating Effect and using SolidartConfig.autoDispose) so the global config is restored after each run.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/solidart/test/reentrancy_test.dart`:
- Around line 4-72: Each test mutates SolidartConfig.autoDispose and doesn't
restore it; capture the original value (e.g. var _prev =
SolidartConfig.autoDispose) at the start of each test and register an
addTearDown that restores it (addTearDown(() => SolidartConfig.autoDispose =
_prev)); update both tests (the ones creating Effect and using
SolidartConfig.autoDispose) so the global config is restored after each run.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85e3f424-5a34-49be-9b9f-b8ddb85a150c
📒 Files selected for processing (8)
packages/flutter_solidart/CHANGELOG.mdpackages/flutter_solidart/pubspec.yamlpackages/solidart/CHANGELOG.mdpackages/solidart/lib/src/core/effect.dartpackages/solidart/pubspec.yamlpackages/solidart/test/reentrancy_test.dartpackages/solidart_hooks/CHANGELOG.mdpackages/solidart_hooks/pubspec.yaml
Summary by CodeRabbit
Bug Fixes
Tests
Chores
Documentation