feat: framework fallback none #33
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 3ffae19 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 |
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant FrameworkHandler
User->>App: Calls function (e.g., unResolve)
App->>FrameworkHandler: Determine framework (default to 'none' if undefined)
alt Framework is 'none'
FrameworkHandler-->>App: Return framework-agnostic behavior
else Framework is a supported framework
FrameworkHandler-->>App: Import/use specific framework logic
end
App-->>User: Return result
Possibly related issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ 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 (
|
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 00b5721 in 1 minute and 57 seconds. Click for details.
- Reviewed
313lines of code in8files - Skipped
0files when reviewing. - Skipped posting
12draft 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/thick-candles-doubt.md:9
- Draft comment:
Ambiguous commit message; clarify whether this is a feature or fix. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to clarify the commit message, which is similar to asking them to update the PR description. This violates the rule against asking for updates to the PR description or similar clarifications.
2. packages/unuse/src/_framework/index.ts:46
- Draft comment:
Returning early in the 'none' case avoids unused imports; ensure downstream code safely handles lack of framework features. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. packages/unuse/src/_framework/index.ts:104
- Draft comment:
Returning an empty object in the 'none' case may mask errors if framework methods are later called; document or safeguard 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 = 20% vs. threshold = 50% The comment raises a valid concern - returning {} could lead to runtime errors if framework methods are called. However, this appears to be an intentional design choice given the type casting. The type system should catch incorrect usage at compile time. The comment doesn't provide a clear actionable alternative. I might be putting too much faith in the TypeScript type system to catch misuse. The runtime behavior could still be dangerous. While runtime safety is important, the type casting suggests this is an intentional escape hatch for the 'none' case. Adding documentation would be nice but not essential. Delete the comment as it questions an intentional design choice without providing clear actionable improvements.
4. packages/unuse/src/toUnSignal/index.ts:48
- Draft comment:
Defaulting UNUSE_FRAMEWORK to 'none' ensures predictable behavior when no framework is set. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
5. packages/unuse/src/tryOnScopeDispose/index.ts:27
- Draft comment:
Using the same fallback to 'none' here standardizes behavior; this is appropriate for tryOnScopeDispose. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
6. packages/unuse/src/unAccess/index.ts:72
- Draft comment:
Fallback to 'none' in isUnRef is consistent; ensure documentation clearly explains behavior when no framework is specified. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
7. packages/unuse/src/unRefElement/index.ts:28
- Draft comment:
Vue-specific handling in unRefElement is clear; consider adding similar commentary for non-Vue paths to aid future maintenance. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
8. packages/unuse/src/unResolve/index.spec.ts:15
- Draft comment:
Explicitly specifying ' 8000 none' as the framework in tests improves clarity and consistency. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
9. packages/unuse/src/unResolve/index.ts:100
- Draft comment:
Defaulting the framework option to 'none' clarifies behavior; ensure this change is highlighted in the documentation. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
10. packages/unuse/src/unResolve/index.ts:116
- Draft comment:
Directly overriding state methods (e.g., in Angular case) is a hack; add inline comments to document potential pitfalls for maintenance. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. packages/unuse/src/unResolve/index.ts:160
- Draft comment:
The React state update hack is fragile; ensure thorough documentation to avoid future maintenance issues. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. .changeset/thick-candles-doubt.md:9
- Draft comment:
On line 9, the commit message reads "feat: framework fallback'none'". The combination of backticks and single quotes around none appears unusual—was this intentional, or should it be formatted as either'none'` or simply none? - 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% This is a purely cosmetic issue in a changeset file. The message is still perfectly readable and understandable regardless of the quote style. The actual functionality or code behavior is not affected. This seems like an extremely minor formatting preference. The unusual formatting might cause confusion when the changeset is processed, or it might be against some established changeset formatting conventions. Changesets are primarily for documentation purposes, and even if there are formatting conventions, this is too minor an issue to warrant a comment. The meaning is clear regardless of quote style. Delete this comment as it's purely about cosmetic formatting in documentation and doesn't affect functionality or code quality.
Workflow ID: wflow_ss5kHl2tSB8QdtOO
You can customize 
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/unuse/src/_framework/index.ts (1)
55-62: Consider updating the type definition to handle'none'case.The
ImportedFrameworkReturn<TFramework>type doesn't explicitly handle the'none'case. While the current implementation works, consider updating the type for completeness:export type ImportedFrameworkReturn<TFramework extends SupportedFramework> = TFramework extends 'angular' ? typeof import('@angular/core') : TFramework extends 'react' ? typeof import('react') : TFramework extends 'solid' ? typeof import('solid-js') + : TFramework extends 'none' + ? {} - : typeof import('vue'); + : typeof import('vue');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.changeset/thick-candles-doubt.md(1 hunks)packages/unuse/src/_framework/index.ts(3 hunks)packages/unuse/src/toUnSignal/index.ts(1 hunks)packages/unuse/src/tryOnScopeDispose/index.ts(1 hunks)packages/unuse/src/unAccess/index.ts(1 hunks)packages/unuse/src/unRefElement/index.ts(1 hunks)packages/unuse/src/unResolve/index.spec.ts(3 hunks)packages/unuse/src/unResolve/index.ts(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/unuse/src/unRefElement/index.ts (1)
packages/unuse/src/_framework/index.ts (1)
SupportedFramework(1-1)
packages/unuse/src/unResolve/index.spec.ts (2)
packages/unuse/src/unResolve/index.ts (1)
unResolve(86-237)packages/unuse/src/unSignal/index.ts (2)
isUnSignal(69-79)unSignal(45-64)
packages/unuse/src/tryOnScopeDispose/index.ts (1)
packages/unuse/src/_framework/index.ts (1)
SupportedFramework(1-1)
packages/unuse/src/toUnSignal/index.ts (1)
packages/unuse/src/_framework/index.ts (1)
SupportedFramework(1-1)
packages/unuse/src/unAccess/index.ts (1)
packages/unuse/src/_framework/index.ts (2)
SupportedFramework(1-1)importedFramework(64-110)
packages/unuse/src/unResolve/index.ts (1)
packages/unuse/src/_framework/index.ts (1)
SupportedFramework(1-1)
🔇 Additional comments (14)
.changeset/thick-candles-doubt.md (1)
1-10: LGTM! Well-structured changeset documentation.The changeset properly documents the feature addition with appropriate version bumps and clear description.
packages/unuse/src/unRefElement/index.ts (1)
27-29: LGTM! Consistent framework fallback implementation.The change properly implements the new
'none'fallback pattern, ensuring the framework variable is always defined and type-safe.packages/unuse/src/unResolve/index.spec.ts (4)
15-15: LGTM! Test correctly updated for new framework fallback.The test properly uses
'none'instead ofundefined, aligning with the type system changes.
23-23: LGTM! Consistent test update.
34-34: LGTM! Consistent test update.
44-44: LGTM! Consistent test update.packages/unuse/src/tryOnScopeDispose/index.ts (1)
26-29: LGTM! Improved clarity with explicit framework comparison.The change from a falsy check to explicit
'none'comparison improves code clarity and aligns with the new type-safe framework handling pattern.packages/unuse/src/_framework/index.ts (3)
1-1: LGTM! Type extension properly adds'none'option.The
SupportedFrameworktype correctly includes the new'none'fallback option.
46-48: LGTM! Proper handling of'none'case.The
initFrameworkImportfunction correctly handles the'none'case by returning early without attempting to import any framework.
104-106: ```shell
#!/bin/bashSearch for globalThis.UNUSE_FRAMEWORK usages
rg "UNUSE_FRAMEWORK" -n packages/unuse/src
</details> <details> <summary>packages/unuse/src/toUnSignal/index.ts (1)</summary> `47-52`: **LGTM! Clean framework fallback implementation.** The changes correctly implement the 'none' fallback pattern by: - Ensuring `framework` always has a value by defaulting to `'none'` - Explicitly handling the `'none'` case to return a plain `unSignal` - Using appropriate ESLint disable comment for the necessary condition check This provides consistent, framework-agnostic behavior when no specific framework is configured. </details> <details> <summary>packages/unuse/src/unAccess/index.ts (1)</summary> `71-121`: **Excellent refactoring with improved framework detection.** The changes enhance the `isUnRef` function by: - Implementing consistent framework defaulting to `'none'` - Using framework-specific detection methods (`Angular.isSignal`, `Vue.isRef`, etc.) - Properly structuring the switch statement with explicit cases - Adding appropriate `'none'` case handling This makes the framework detection more reliable and maintainable while preserving the original detection logic. </details> <details> <summary>packages/unuse/src/unResolve/index.ts (2)</summary> `19-19`: **Improved type safety by constraining framework type.** Removing `undefined` from the `TFramework` constraint improves type safety by ensuring it's always a valid `SupportedFramework` value. This aligns with the new framework fallback pattern where `'none'` serves as the explicit default. --- `99-103`: **Consistent framework defaulting implementation.** The framework option now follows the same pattern as other files in the codebase: - Defaults to `'none'` when `globalThis.__UNUSE_FRAMEWORK__` is falsy - Maintains type safety with proper casting - Includes appropriate ESLint disable comment This ensures predictable behavior across the entire framework detection system. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
<
8000
div data-view-component="true" class="comment-reactions js-reactions-container js-reaction-buttons-container social-reactions reactions-container d-flex">
00b5721 to
3ffae19
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 3ffae19 in 1 minute and 29 seconds. Click for details.
- Reviewed
332lines of code in8files - Skipped
0files when reviewing. - Skipped posting
10draft 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/thick-candles-doubt.md:9
- Draft comment:
Consider clarifying in the commit message whether this change is a feature or a fix; the title is ambiguous. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
2. packages/unuse/src/_framework/index.ts:11
- Draft comment:
Good use of 'Exclude<SupportedFramework, "none">' to restrict framework initialization; this reduces misuse of the 'none' fallback. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
3. packages/unuse/src/toUnSignal/index.ts:48
- Draft comment:
Repeated fallback logic (using globalThis.UNUSE_FRAMEWORK || 'none') appears in several files; consider abstracting this logic into a helper function for consistency and maintainability. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
4. packages/unuse/src/unResolve/index.ts:115
- Draft comment:
Overriding Angular signal's 'set' and 'update' methods is risky; consider safer synchronization approaches to avoid unexpected side effects. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
5. packages/unuse/src/unResolve/index.ts:142
- Draft comment:
Directly mutating the React state array (assigning to state[0] and overriding state[1]) is dangerous; this might bypass React's state management and could lead to unexpected behavior. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
6. packages/unuse/src/unResolve/index.ts:206
- Draft comment:
In the Solid branch, using an effect to call state[1](() => signal.get()) might be redundant; verify if this extra update is necessary or if a more direct syncing mechanism can be used. - Reason this comment was not posted:
Confidence changes required:50%<= threshold50%None
7. packages/unuse/src/unResolve/index.ts:227
- Draft comment:
In the default branch (for 'none'), returning either the raw signal or an unComputed value is acceptable, but ensure this behavior is well-documented so that users understand the fallback behavior when no framework is used. - Reason this comment was not posted:
Confidence changes required:30%<= threshold50%None
8. packages/unuse/src/unResolve/index.ts:70
- Draft comment:
Multiple '@ts-expect-error' comments indicate underlying type safety issues. Consider revisiting the type definitions to reduce these overrides if possible. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. packages/unuse/src/unResolve/index.spec.ts:15
- Draft comment:
Using an explicit framework 'none' in tests improves clarity and reproducibility compared to an undefined fallback. - Reason this comment was not posted:
Confidence changes required:10%<= threshold50%None
10. .changeset/thick-candles-doubt.md:9
- Draft comment:
On line 9, the commit message includes an unusual mix of backticks and single quotes:feat: framework fallback'none'. Please confirm if the intended formatting was to have the fallback value formatted as code (e.g.none`) or simply quoted (e.g. 'none'). - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to confirm their intention regarding the formatting of a commit message. This violates the rule against asking the author to confirm their intention or to explain something. The comment does not provide a specific code suggestion or ask for a test to be written, which would be acceptable.
Workflow ID: wflow_ozfYDLqph8CizEJC
You can customize
5 tasks
JounQin
reviewed
Jun 24, 2025
JounQin
reviewed
Jun 24, 2025
JounQin
approved these changes
Jun 24, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #32
I'm not sure if this is a fix or feat 🤔
Important
Added 'none' framework fallback to allow operation without a specific framework, updating behavior and tests accordingly.
'none'as aSupportedFrameworkinindex.tsto allow operation without a specific framework.initFrameworkImport()andimportedFramework()to exclude'none'.'none'intoUnSignal(),tryOnScopeDispose(),unAccess(),unRefElement(), andunResolve().unResolve(), added handling for'none'framework to returnUnSignalorUnComputedbased onreadonlyoption.unResolve/index.spec.tsto use'none'framework for testingUnSignalandUnComputedbehavior.thick-candles-doubt.mddocumenting the new'none'framework feature.This description was created by
for 3ffae19. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
'none'framework fallback, allowing the library to operate without a specific framework.'none'framework option for more consistent testing.