-
-
Notifications
You must be signed in to change notification settings - Fork 186
feat: add useThrottled
hooks.
#471
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughA new throttling hook, Changes
Sequence Diagram(s)sequenceDiagram
participant Widget
participant useThrottled
participant _Throttler
Widget->>useThrottled: Initialize with duration
useThrottled->>_Throttler: Create throttler instance
loop On User Tap
Widget->>useThrottled: Call throttled callback
useThrottled->>_Throttler: run(callback)
alt Timer not active
_Throttler->>Widget: Execute callback
_Throttler->>_Throttler: Start timer for duration
else Timer active
_Throttler-->>Widget: Ignore callback
end
Note over _Throttler: After duration, timer resets
end
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/flutter_hooks/lib/src/throttled.dart (2)
3-3
: Fix grammatical error in documentation.The documentation comment contains a grammatical error.
-/// widget ignore updates accordingly after a specified [duration] duration. +/// Throttles widget updates to occur at most once per specified [duration].
7-12
: Improve documentation example clarity.The current example could be more clear about the throttling behavior and use a more typical throttling scenario.
-/// String userInput = ''; // Your input value -/// -/// // Create a throttle callback -/// final throttle = useThrottle(duration: const Duration(milliseconds: 500)); -/// // Assume a fetch method fetchData(String query) exists -/// Button(onPressed: () => throttle(() => fetchData(userInput))); +/// final searchQuery = useState(''); +/// final throttle = useThrottle(duration: const Duration(milliseconds: 500)); +/// +/// TextField( +/// onChanged: (value) { +/// searchQuery.value = value; +/// // This will be throttled to run at most once every 500ms +/// throttle(() => performSearch(value)); +/// }, +/// );packages/flutter_hooks/test/use_throttle_test.dart (2)
40-41
: Remove redundant delay operations.Both
pumpAndSettle(_duration)
andFuture.delayed(_duration)
are unnecessary. ThepumpAndSettle
should be sufficient to advance the timer.-await tester.pumpAndSettle(_duration); -await Future<void>.delayed(_duration); +await Future<void>.delayed(_duration); +await tester.pump();
73-73
: Consider adding test for custom duration.The tests only cover the default duration. Consider adding a test case that verifies custom duration parameter works correctly.
Add a test case like:
+testWidgets('custom duration works correctly', (tester) async { + // Test with a shorter duration like 100ms + // to verify the duration parameter is respected +});Would you like me to generate a complete test case for custom duration validation?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/flutter_hooks/lib/src/hooks.dart
(1 hunks)packages/flutter_hooks/lib/src/throttled.dart
(1 hunks)packages/flutter_hooks/test/use_throttle_test.dart
(1 hunks)
🔇 Additional comments (6)
packages/flutter_hooks/lib/src/hooks.dart (1)
40-40
: LGTM! Proper integration of the new throttle hook.The part directive correctly follows the established pattern and alphabetical ordering of other hook inclusions in the library.
packages/flutter_hooks/lib/src/throttled.dart (3)
14-19
: LGTM! Clean function implementation.The
useThrottle
function is well-implemented:
- Proper use of
useMemoized
to maintain the throttler instance- Correct dependency array with
[duration]
- Clean return of the throttler's run method
21-26
: LGTM! Proper validation in constructor.The constructor correctly validates that the duration is positive and provides a clear assertion message.
34-41
: LGTM! Correct throttling implementation.The throttling logic is correctly implemented:
- Executes callback immediately if no timer is running (leading edge behavior)
- Starts timer to prevent subsequent executions during the duration
- Properly resets the timer when duration completes
This follows the standard throttling pattern effectively.
packages/flutter_hooks/test/use_throttle_test.dart (2)
7-31
: LGTM! Comprehensive throttling validation.This test correctly validates that multiple rapid taps are throttled, ensuring only the first tap within the duration period triggers a state update.
52-71
: LGTM! Well-structured test widget.The test widget properly demonstrates the
useThrottle
hook usage with state management and user interaction.
testWidgets('update number after duration', (tester) async { | ||
await tester.runAsync<void>(() async { | ||
await tester.pumpWidget(const _UseThrottleTestWidget()); | ||
|
||
final text = find.byType(GestureDetector); | ||
expect(find.text('1'), findsOneWidget); | ||
|
||
await tester.pumpAndSettle(_duration); | ||
await Future<void>.delayed(_duration); | ||
|
||
await tester.tap(text); | ||
await tester.pump(); | ||
|
||
expect(find.text('2'), findsOneWidget); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve test to better validate throttle reset behavior.
The current test doesn't fully validate that the throttle properly resets. It only tests a single tap after waiting, not that subsequent taps can occur after the throttle period.
testWidgets('update number after duration', (tester) async {
await tester.runAsync<void>(() async {
await tester.pumpWidget(const _UseThrottleTestWidget());
final text = find.byType(GestureDetector);
expect(find.text('1'), findsOneWidget);
+ // First tap should work immediately
+ await tester.tap(text);
+ await tester.pump();
+ expect(find.text('2'), findsOneWidget);
+
+ // Wait for throttle duration to pass
await tester.pumpAndSettle(_duration);
await Future<void>.delayed(_duration);
+ // Next tap should work after throttle resets
await tester.tap(text);
await tester.pump();
- expect(find.text('2'), findsOneWidget);
+ expect(find.text('3'), findsOneWidget);
});
});
🤖 Prompt for AI Agents
In packages/flutter_hooks/test/use_throttle_test.dart around lines 33 to 48, the
test only verifies a single tap after the throttle duration but does not confirm
that the throttle resets to allow subsequent taps. Modify the test to include
multiple taps separated by the throttle duration, verifying that the widget
updates correctly each time, thereby ensuring the throttle reset behavior is
properly validated.
/// Button(onPressed: () => throttle(() => fetchData(userInput))); | ||
/// ``` | ||
void Function(VoidCallback callback) useThrottle({ | ||
Duration duration = const Duration(milliseconds: 500), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This value seems arbitrary. I'd rather not have a default value and make the parameter required
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated with 4ebb7f7
/// // Assume a fetch method fetchData(String query) exists | ||
/// Button(onPressed: () => throttle(() => fetchData(userInput))); | ||
/// ``` | ||
void Function(VoidCallback callback) useThrottle({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be named useThrottled
to match `useDebounced
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated with 4ebb7f7
/// // Assume a fetch method fetchData(String query) exists | ||
/// Button(onPressed: () => throttle(() => fetchData(userInput))); | ||
/// ``` | ||
void Function(VoidCallback callback) useThrottle({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about the API
I'd like something that matches the useDebounced
API, for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think useDebounced
has big difference of usage.
useThrottled
should be callback method to use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/flutter_hooks/lib/src/throttled.dart
(1 hunks)packages/flutter_hooks/test/use_throttle_test.dart
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/flutter_hooks/test/use_throttle_test.dart
🔇 Additional comments (1)
packages/flutter_hooks/lib/src/throttled.dart (1)
14-19
: LGTM! Function signature and implementation are well-structured.The function correctly:
- Makes
duration
required (addressing past feedback)- Uses
useMemoized
with proper dependency tracking- Returns a clean callback interface
/// widget ignore updates accordingly after a specified [duration] duration. | ||
/// | ||
/// Example: | ||
/// ```dart | ||
/// String userInput = ''; // Your input value | ||
/// | ||
/// // Create a throttle callback | ||
/// final throttle = useThrottled(duration: const Duration(milliseconds: 500)); | ||
/// // Assume a fetch method fetchData(String query) exists | ||
/// Button(onPressed: () => throttle(() => fetchData(userInput))); | ||
/// ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve documentation clarity and correctness.
The documentation has grammatical issues and could better explain the throttling behavior:
-/// widget ignore updates accordingly after a specified [duration] duration.
+/// Creates a throttled callback that executes at most once per [duration].
+///
+/// The returned function will execute the provided callback immediately on the
+/// first call, then ignore subsequent calls until the duration has elapsed.
Also consider adding more context about the throttling behavior (leading edge vs trailing edge) to set proper expectations.
📝 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.
/// widget ignore updates accordingly after a specified [duration] duration. | |
/// | |
/// Example: | |
/// ```dart | |
/// String userInput = ''; // Your input value | |
/// | |
/// // Create a throttle callback | |
/// final throttle = useThrottled(duration: const Duration(milliseconds: 500)); | |
/// // Assume a fetch method fetchData(String query) exists | |
/// Button(onPressed: () => throttle(() => fetchData(userInput))); | |
/// ``` | |
/// Creates a throttled callback that executes at most once per [duration]. | |
/// | |
/// The returned function will execute the provided callback immediately on the | |
/// first call, then ignore subsequent calls until the duration has elapsed. | |
/// | |
/// Example: | |
/// |
🤖 Prompt for AI Agents
In packages/flutter_hooks/lib/src/throttled.dart around lines 3 to 13, improve
the documentation by correcting grammatical errors and clarifying the
explanation of the throttling behavior. Add details about whether the throttle
triggers on the leading edge, trailing edge, or both, to set proper expectations
for users. Ensure the example is clear and accurately demonstrates how to use
the throttle callback.
void run(VoidCallback callback) { | ||
if (!_isRunning) { | ||
_timer = Timer(duration, () { | ||
_timer = null; | ||
}); | ||
callback(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider edge case handling for invalid durations.
The current implementation doesn't handle edge cases like Duration.zero
or negative durations, which could lead to unexpected behavior.
void run(VoidCallback callback) {
+ if (duration <= Duration.zero) {
+ callback();
+ return;
+ }
+
if (!_isRunning) {
_timer = Timer(duration, () {
_timer = null;
});
callback();
}
}
For zero or negative durations, the callback should execute immediately without throttling.
📝 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.
void run(VoidCallback callback) { | |
if (!_isRunning) { | |
_timer = Timer(duration, () { | |
_timer = null; | |
}); | |
callback(); | |
} | |
} | |
void run(VoidCallback callback) { | |
if (duration <= Duration.zero) { | |
callback(); | |
return; | |
} | |
if (!_isRunning) { | |
_timer = Timer(duration, () { | |
_timer = null; | |
}); | |
callback(); | |
} | |
} |
🤖 Prompt for AI Agents
In packages/flutter_hooks/lib/src/throttled.dart around lines 30 to 37, the run
method does not handle zero or negative Duration values, which can cause
unexpected behavior. Modify the method to check if the duration is zero or
negative; if so, execute the callback immediately without setting a timer or
throttling. Otherwise, proceed with the existing throttling logic.
class _Throttler { | ||
_Throttler(this.duration); | ||
|
||
final Duration duration; | ||
|
||
Timer? _timer; | ||
|
||
bool get _isRunning => _timer != null; | ||
|
||
void run(VoidCallback callback) { | ||
if (!_isRunning) { | ||
_timer = Timer(duration, () { | ||
_timer = null; | ||
}); | ||
callback(); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify timer resource cleanup behavior.
The current implementation should handle timer cleanup correctly, but it's worth verifying that timers are properly disposed when the widget is unmounted.
🏁 Script executed:
#!/bin/bash
# Description: Check if there are examples of timer cleanup patterns in other hooks
# Expected: Find similar timer usage patterns and cleanup mechanisms
rg -A 10 -B 5 "Timer" --type dart packages/flutter_hooks/lib/src/
Length of output: 2978
🏁 Script executed:
#!/bin/bash
# Display _DebouncedHookState implementation and its dispose method if any
rg -A20 -B5 "class _DebouncedHookState" --type dart packages/flutter_hooks/lib/src/debounced.dart
echo "----Searching for dispose override in debounced.dart----"
rg "@override\s+void\s+dispose" --type dart packages/flutter_hooks/lib/src/debounced.dart
# Display _Throttler usage: look for HookState and dispose
rg -A20 -B5 "class _ThrottledHookState" --type dart packages/flutter_hooks/lib/src/throttled.dart || echo "No HookState class in throttled.dart"
echo "----Searching for dispose override in throttled.dart----"
rg "@override\s+void\s+dispose" --type dart packages/flutter_hooks/lib/src/throttled.dart
Length of output: 1224
🏁 Script executed:
#!/bin/bash
# Display full content of throttled.dart
sed -n '1,200p' packages/flutter_hooks/lib/src/throttled.dart
Length of output: 972
Cleanup pending timers on unmount
The _Throttler
in packages/flutter_hooks/lib/src/throttled.dart
schedules a Timer
but never cancels it when the widget is disposed. If that timer fires after unmount, it may invoke callbacks on a disposed widget. To address this:
- In
_Throttler
, add acancel()
method:void cancel() { _timer?.cancel(); _timer = null; }
- In
useThrottled
, register a cleanup effect:final throttler = useMemoized(() => _Throttler(duration), [duration]); useEffect(() { return () { throttler.cancel(); }; }, [throttler]); return throttler.run;
- (Optional) Review
packages/flutter_hooks/lib/src/debounced.dart
—it likewise never cancels its timer on dispose and should follow a similar pattern.
These changes ensure no stray timers fire after the hook is unmounted.
🤖 Prompt for AI Agents
In packages/flutter_hooks/lib/src/throttled.dart around lines 21 to 38, the
_Throttler class creates a Timer but does not cancel it when the widget
unmounts, risking callbacks firing on disposed widgets. Fix this by adding a
cancel() method to _Throttler that cancels and nullifies the timer. Then, in the
useThrottled hook, register a cleanup effect that calls throttler.cancel() on
unmount. This ensures timers are properly cleaned up and no callbacks run after
disposal.
Hi 👋, Thanks for the I’d like to suggest enhancing it with support for throttle modes ( Additionally, adding
These features are common in popular throttle/debounce utilities like Lodash, and including them would make I’m happy to help contribute these improvements if the maintainers are open to it. Thanks! |
useuseThrottle
hooks.useThrottled
hooks.
close. #472
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests