feat: useToggle #55
feat: useToggle #55
Conversation
🦋 Changeset detectedLatest commit: 8f13f7b The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded@Shinigami92 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 18 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 (8)
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant useToggle
participant Signal/Ref
User->>useToggle: Call useToggle(initialValue, options?)
useToggle->>Signal/Ref: Create or use reactive state
useToggle-->>User: Return [state, toggleFn] or toggleFn
User->>toggleFn: Call toggleFn() or toggleFn(value)
toggleFn->>Signal/Ref: Update state (toggle or set explicit value)
Signal/Ref-->>User: Updated state
Possibly related PRs
Poem
✨ Finishing Touches
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 (
|
475c77a to
8163ec1
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 8163ec1 in 1 minute and 11 seconds. Click for details.
- Reviewed
318lines of code in5files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/unuse/src/useToggle/index.ts:63
- Draft comment:
Clarify behavior when toggle is called with an explicit undefined. The use of arguments.length to differentiate may be ambiguous if undefined is intentionally passed. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The code's behavior is actually clear - it treats any passed value (including undefined) as a direct set operation, while no arguments triggers the toggle behavior. This is a common and valid pattern in JavaScript. The type signature (value?: T) makes it clear that undefined is a valid input. The behavior isn't ambiguous - it's intentionally designed this way. The comment raises a valid point about API design clarity. Some developers might expect undefined to always trigger the toggle behavior. While it's a thoughtful observation, the current implementation follows established patterns and is well-typed. The behavior is consistent and predictable once understood. The comment should be deleted as it suggests clarifying something that is already clear and well-implemented. The current code follows common JavaScript patterns.
2. packages/unuse/src/useToggle/index.ts:76
- Draft comment:
Document the dual return pattern (toggle function vs tuple) based on whether the initial value is a ref. This behavior could be surprising to some users. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. packages/unuse/src/useToggle/index.vue.spec.ts:10
- Draft comment:
Avoid excessive type assertions in tests. Consider refining the overload/types so that manual casts (as unknown as tuple) aren’t needed. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_yqLE5Yrt8PUDHC7X
You can customize 
8163ec1 to
85f9319
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 85f9319 in 1 minute and 19 seconds. Click for details.
- Reviewed
680lines of code in8files - Skipped
0files when reviewing. - Skipped posting
4draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .changeset/fluffy-nights-try.md:2
- Draft comment:
Consider removing unnecessary quotes from keys in the changeset; unquoted keys may offer a more standard YAML style. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. packages/unuse/src/useToggle/index.react.spec.ts:45
- Draft comment:
Several tests here are marked as 'todo'. Ensure these tests are implemented once the React toggling behavior is finalized. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that tests are implemented in the future, which is not specific enough to be useful. It doesn't provide a specific suggestion or point out a specific issue with the current code. It also doesn't ask for a specific test to be written now, which would be acceptable.
3. packages/unuse/src/useToggle/index.ts:76
- Draft comment:
The overload returning just the toggle function when the initial value is a reactive reference (isUnRef true) may be confusing. Consider adding documentation to clarify this behavior. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The behavior is already documented through TypeScript overloads which clearly show the two different return types. The function is also documented with JSDoc comments. The behavior follows a consistent pattern where UnRef values get simpler returns. Adding more documentation about this pattern would be redundant since it's a framework convention. The comment might have a point that users unfamiliar with the UnRef pattern could be confused by the different return types. The TypeScript types already provide clear documentation of this behavior, and it follows framework conventions. Additional documentation would be redundant. Delete the comment as the behavior is already well-documented through types and follows framework patterns.
4. packages/unuse/src/useToggle/index.react.spec.ts:39
- Draft comment:
For consistency with the default test, consider checking that the value is an array via Array.isArray instead of using toBeTypeOf('object'). - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_gbbLD03FAvyOqvjf
You can customize
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/unuse/src/useToggle/index.ts (1)
53-53: Consider improving type safety for the default value.The default value
false as Tcould cause type issues if users provide custom truthy/falsy values that don't includefalse. While the type constraints should catch this at compile time, consider a more type-safe approach.Consider this alternative approach:
- initialValue: MaybeUnRef<T> = false as T, + initialValue: MaybeUnRef<T> = (false as any) as T,Or better yet, make the default conditional:
- initialValue: MaybeUnRef<T> = false as T, + initialValue?: MaybeUnRef<T>,And handle the undefined case in the implementation by using
falsyValueas the default.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.changeset/fluffy-nights-try.md(1 hunks)packages/unuse/src/index.ts(1 hunks)packages/unuse/src/useToggle/index.angular.spec.ts(1 hunks)packages/unuse/src/useToggle/index.react.spec.ts(1 hunks)packages/unuse/src/useToggle/index.solid.spec.ts(1 hunks)packages/unuse/src/useToggle/index.spec.ts(1 hunks)packages/unuse/src/useToggle/index.ts(1 hunks)packages/unuse/src/useToggle/index.vue.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/unuse/src/index.ts
- .changeset/fluffy-nights-try.md
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/unuse/src/useToggle/index.solid.spec.ts (2)
packages/unuse/src/_testUtils/solid.ts (1)
describeSolid(11-26)packages/unuse/src/useToggle/index.ts (2)
useToggle(48-81)ToggleFn(7-11)
packages/unuse/src/useToggle/index.react.spec.ts (2)
packages/unuse/src/_testUtils/react.ts (1)
describeReact(11-26)packages/unuse/src/useToggle/index.ts (2)
useToggle(48-81)ToggleFn(7-11)
packages/unuse/src/useToggle/index.ts (1)
packages/unuse/src/unAccess/index.ts (2)
MaybeUnRef(128-128)unAccess(148-191)
packages/unuse/src/useToggle/index.spec.ts (2)
packages/unuse/src/useToggle/index.ts (1)
useToggle(48-81)packages/unuse-reactivity/src/unSignal/index.ts (3)
isUnSignal(69-79)UnSignal(17-34)unSignal(45-64)
packages/unuse/src/useToggle/index.vue.spec.ts (2)
packages/unuse/src/_testUtils/vue.ts (1)
describeVue(15-30)packages/unuse/src/useToggle/index.ts (2)
useToggle(48-81)ToggleFn(7-11)
🪛 ESLint
packages/unuse/src/useToggle/index.solid.spec.ts
[error] 2-2: Unable to resolve path to module 'solid-js'.
(import-x/no-unresolved)
packages/unuse/src/useToggle/index.react.spec.ts
[error] 3-3: Unable to resolve path to module '@testing-library/react'.
(import-x/no-unresolved)
[error] 4-4: Unable to resolve path to module 'react'.
(import-x/no-unresolved)
packages/unuse/src/useToggle/index.angular.spec.ts
[error] 4-4: Unable to resolve path to module '@angular/core'.
(import-x/no-unresolved)
[error] 5-5: Unable to resolve path to module '@angular/core/testing'.
(import-x/no-unresolved)
[error] 9-9: Unable to resolve path to module '@angular/platform-browser/testing'.
(import-x/no-unresolved)
packages/unuse/src/useToggle/index.spec.ts
[error] 2-2: Unable to resolve path to module 'unuse-reactivity'.
(import-x/no-unresolved)
packages/unuse/src/useToggle/index.vue.spec.ts
[error] 3-3: Unable to resolve path to module 'vue'.
(import-x/no-unresolved)
🪛 GitHub Check: Lint: node-24, ubuntu-latest
packages/unuse/src/useToggle/index.react.spec.ts
[warning] 98-98:
The use of .todo is not recommended
[warning] 77-77:
The use of .todo is not recommended
[warning] 61-61:
The use of .todo is not recommended
[warning] 45-45:
The use of .todo is not recommended
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (9)
packages/unuse/src/useToggle/index.spec.ts (1)
1-98: Excellent comprehensive test coverage for the generic useToggle implementation.This test suite thoroughly covers all the key functionality:
- Default behavior and structure validation
- Initial value handling (both explicit and default)
- Toggle functionality (both parameter-less and with explicit values)
- External signal integration
- Custom truthy/falsy value configuration
The test assertions are well-structured and correctly validate both the return types and the reactive behavior of the hook.
packages/unuse/src/useToggle/index.solid.spec.ts (1)
1-96: Well-implemented Solid.js test suite with complete coverage.This test suite properly validates the
useTogglehook's integration with Solid.js:
- Correctly uses
createSignalfor testing signal-based initial values- Proper type casting to
Signal<boolean>andToggleFn- Comprehensive coverage of all functionality including custom truthy/falsy values
- No incomplete TODO tests, unlike the React version
The implementation demonstrates good understanding of Solid.js reactivity patterns.
packages/unuse/src/useToggle/index.vue.spec.ts (1)
1-96: Comprehensive Vue test suite with proper reactive integration.This test suite effectively validates the
useTogglehook's Vue integration:
- Appropriate use of
shallowReffor testing reactive references- Correct type casting to
Ref<boolean>and validation withisRef- Complete test coverage including external ref integration and custom values
- Consistent test patterns with other framework implementations
The Vue-specific reactive behavior is properly tested and validated.
packages/unuse/src/useToggle/index.angular.spec.ts (1)
1-126: Well-structured Angular test suite with proper signal integration.This test suite correctly implements Angular-specific testing patterns:
- Proper test environment initialization with
BrowserTestingModuleandZonelessTestModule- Appropriate use of
getTestBed().runInInjectionContext()for signal-based tests- Correct type casting to
WritableSignal<boolean>and validation withisSignal- Comprehensive coverage of all scenarios including external signal integration
The Angular testing patterns are correctly applied and the suite provides complete coverage.
packages/unuse/src/useToggle/index.ts (5)
1-6: LGTM: Clean imports and proper dependencies.The imports are well-organized and include all necessary utilities for the toggle implementation.
7-24: Excellent type definitions with comprehensive generics.The type definitions provide great flexibility while maintaining type safety. The
ToggleFn,UseToggleReturn, andUseToggleOptionstypes properly handle the generic constraints for custom truthy/falsy values.
26-41: Well-designed function overloads for different use cases.The overloads correctly differentiate between:
- When
initialValueis a reactive reference (UnRef<T>) - returns only the toggle function- When
initialValueis a regular value - returns tuple with reactive value and toggle functionThis provides excellent developer experience and type safety.
62-74: Excellent resolution of thearguments.lengthissue.The toggle function correctly uses a function declaration instead of an arrow function, which resolves the previous issue with
arguments.lengthmentioned in the past review comments. The logic is sound:
- When arguments are provided, it sets the value directly
- When no arguments, it toggles between truthy and falsy values
The use of
_value.get()after set/update operations ensures the fresh value is returned.
76-80: Smart return logic based on input type.The conditional return logic is well-designed:
- If the initial value was a reactive reference, return only the toggle function (user already has reactive access)
- Otherwise, return both the readonly reactive value and toggle function
This provides optimal API ergonomics for different use cases.
85f9319 to
069494b
Compare
069494b to
8f13f7b
Compare
Important
Introduces
useToggleutility for managing toggleable state with tests and documentation updates.useToggleutility inindex.tsfor managing toggleable state with customizable values.useTogglein package exports.useToggleinindex.spec.ts,index.angular.spec.ts,index.react.spec.ts,index.solid.spec.ts, andindex.vue.spec.ts.fluffy-nights-try.mdfor documenting the new feature and minor version updates.This description was created by
for 85f9319. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
useToggleutility for managing toggleable state with customizable values.useTogglepublicly available in the package exports.Tests
useTogglefunctionality, including support for custom values and integration with reactive references across multiple frameworks.Chores