feat: add signal peek #51
Conversation
🦋 Changeset detectedLatest commit: d9c6ae5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 feature has been introduced to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Signal as unSignal
participant Effect as unEffect
User->>Signal: set(value)
Signal->>Effect: Notify reactive effects
User->>Signal: get()
Signal->>Effect: Register dependency, return value
User->>Signal: peek()
Signal-->>User: Return value (no dependency tracking)
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ 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 (
|
b46d3d2 to
8ea8fee
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 8ea8fee in 1 minute and 8 seconds. Click for details.
- Reviewed
133lines of code in4files - Skipped
0files when reviewing. - Skipped posting
2draft 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-reactivity/src/unSignal/index.ts:107
- Draft comment:
Inconsistency: The 'update' method returns the new value but the interface specifies a void return. Adjust the implementation or the type signature. - 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.
2. packages/unuse-reactivity/src/unComputed/index.ts:56
- Draft comment:
The PR description mentions adding a 'peek' function to unComputed, but no such method is implemented. Clarify the intent or update the description. - 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 intent or update the description, which violates the rule against asking the author to update the PR description or confirm their intention. The comment does not provide a specific code suggestion or ask for a test to be written. Therefore, it should be removed.
Workflow ID: wflow_lm0hjizdOZlLmXJL
You can customize 
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/unuse-reactivity/src/unSignal/index.ts (1)
117-127: Consider updating the type guard to include peek method.The
isUnSignalfunction checks forget,set, andupdatemethods but doesn't check for the newly addedpeekmethod. For complete type safety, consider adding a check forpeekas well.export function isUnSignal<T>(value: unknown): value is UnSignal<T> { return ( !!value && typeof value === 'object' && UN_SIGNAL in value && value[UN_SIGNAL] === true && typeof (value as UnSignal<T>).get === 'function' && + typeof (value as UnSignal<T>).peek === 'function' && typeof (value as UnSignal<T>).set === 'function' && typeof (value as UnSignal<T>).update === 'function' ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/dark-rats-ask.md(1 hunks)packages/unuse-reactivity/src/unComputed/index.ts(1 hunks)packages/unuse-reactivity/src/unSignal/index.spec.ts(4 hunks)packages/unuse-reactivity/src/unSignal/index.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/unuse-reactivity/src/unSignal/index.spec.ts (2)
packages/unuse-reactivity/src/unSignal/index.ts (1)
unSignal(63-112)packages/unuse-reactivity/src/unEffect/index.ts (1)
unEffect(15-39)
🔇 Additional comments (8)
packages/unuse-reactivity/src/unComputed/index.ts (1)
57-57: LGTM: Cosmetic formatting improvement.The whitespace addition improves code formatting consistency.
packages/unuse-reactivity/src/unSignal/index.ts (2)
38-41: LGTM: Well-documented interface addition.The
peek()method is properly documented and clearly explains its non-reactive behavior.
105-105: LGTM: Correct implementation of non-reactive value access.The implementation correctly returns the state value directly without any reactive tracking or linking, which is exactly the intended behavior for
peek()..changeset/dark-rats-ask.md (1)
1-11: LGTM: Proper changeset documentation.The changeset correctly documents this as a minor feature addition across all relevant packages with a clear description.
packages/unuse-reactivity/src/unSignal/index.spec.ts (4)
1-3: LGTM: Proper test setup with required imports.The imports correctly add the necessary testing utilities (
vifor mocking andunEffectfor reactive testing).
16-16: LGTM: Proper assertions for peek method existence.The tests correctly verify that the
peekmethod exists and is a function on both parameterized and non-parameterized signals.Also applies to: 27-27
57-85: LGTM: Comprehensive reactive behavior testing.The tests properly verify that
setandupdateoperations trigger reactive effects, with the spy being called twice (initial effect run + triggered by value change).
87-101: LGTM: Excellent test for non-reactive peek behavior.This test perfectly demonstrates the key feature of
peek()- it returns the current value without triggering reactive effects. The spy being called only once (initial effect run) whilesetcalls don't trigger additional effect runs proves the non-reactive behavior is working correctly.
caeee8f to
b3c3732
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed caeee8f in 1 minute and 15 seconds. Click for details.
- Reviewed
141lines of code in4files - Skipped
0files when reviewing. - Skipped posting
5draft 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/dark-rats-ask.md:10
- Draft comment:
Release notes file is formatted correctly with appropriate version bumps. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/unuse-reactivity/src/unComputed/index.ts:56
- Draft comment:
An extra blank line is added in the UnComputed interface; no peek is implemented here. Confirm if this change is intentional. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
3. packages/unuse-reactivity/src/unSignal/index.spec.ts:87
- Draft comment:
Test for 'peek' properly verifies that its call does not trigger reactive effects. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. packages/unuse-reactivity/src/unSignal/index.ts:105
- Draft comment:
The 'peek' implementation correctly returns the raw state value without triggering effects. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
5. packages/unuse-reactivity/src/unSignal/index.ts:107
- Draft comment:
The 'update' method returns a value even though its interface signature implies void; consider aligning the return type for consistency. - 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.
Workflow ID: wflow_JIGWXjyHF1Zny2Gh
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed b3c3732 in 1 minute and 7 seconds. Click for details.
- Reviewed
157lines of code in4files - Skipped
0files when reviewing. - Skipped posting
3draft comments. View those below. - Modify your setting 8000 s and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .changeset/dark-rats-ask.md:1
- Draft comment:
Changeset file looks good; version bumps and titles are correctly specified. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. packages/unuse-reactivity/src/unSignal/index.spec.ts:87
- Draft comment:
Good test for peek(): it confirms that using peek() does not trigger reactive effects. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. packages/unuse-reactivity/src/unSignal/index.ts:107
- Draft comment:
The update() method returns a value despite its interface specifying a void return. Consider removing 'return state.value' for consistency. - 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.
Workflow ID: wflow_mHareIEmlJHYKOg9
You can customize
Co-authored-by: teleskop150750 <62594983+teleskop150750@users.noreply.github.com>
b3c3732 to
d9c6ae5
Compare
|
Should we use untrack to avoid breaking the logic of the work? Signal.prototype.peek = function () {
const prevContext = evalContext;
evalContext = undefined;
try {
return this.value;
} finally {
evalContext = prevContext;
}
};Alien-signals untrack const src = signal(0);
let computedTriggerTimes = 0;
const c = computed(() => {
computedTriggerTimes++;
const currentSub = setCurrentSub(undefined);
const value = src();
setCurrentSub(currentSub);
return value;
}); |
|
In fact, I don't know yet what this code affects. If you remove it, all tests pass. |
adds the
peekfunction to unSignaland unComputedtaken from #27
this can later be used to retrieve a value without triggering inside an effect
I added @teleskop150750 so he gets credits for it and the next PRs from him can CI-run without approvals
Important
Add
peek()method toUnSignalfor non-reactive value access, with tests inindex.spec.ts.peek()method toUnSignalinindex.tsto access current signal value without triggering effects.index.spec.tsto include tests forpeek()method, ensuring non-reactive access pattern.index.ts.This description was created by
for b3c3732. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Tests