feat: computed cyclic node#63
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7e30e312e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Run getter and only update if not part of the scc | ||
| try { | ||
| // REFACTOR: this currently causes the call to be recursive | ||
| const result = this.getter(); |
There was a problem hiding this comment.
When this dirty path calls getter directly, it skips the bookkeeping that update()/normal Computed.get() perform: the active subscriber remains the outer effect/watch and this node's Dirty flag is never cleared. In an effect that reads computedCyclic(() => a.value < 10), the effect subscribes directly to a and reruns on every a write even when the computed result is unchanged, while standalone reads also recompute the getter/stabilizer every time because shouldUpdate(this) stays true.
Useful? React with 👍 / 👎.
| const context = activeContext; | ||
| const hasCurrent = context?.stackNodes.has(this); |
There was a problem hiding this comment.
| const context = activeContext; | |
| const hasCurrent = context?.stackNodes.has(this); | |
| return !!activeContext?.stackNodes.has(this); |
| const currentStackIndex = context.stack.length - 1; | ||
|
|
||
| if (context.sccTail === undefined || context.sccHead === undefined) { | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
| const currentStackIndex = context.stack.length - 1; | |
| if (context.sccTail === undefined || context.sccHead === undefined) { | |
| return false; | |
| } | |
| if (context.sccTail === undefined || context.sccHead === undefined) { | |
| return false; | |
| } | |
| const currentStackIndex = context.stack.length - 1; | |
Since you're not using currentStackIndex before the if-statements below
| const context = activeContext!; | ||
|
|
||
| context.sccTail = context.stack.length - 1; | ||
| const prevOccuranceIndex = context.stack.indexOf(this); |
There was a problem hiding this comment.
| const prevOccuranceIndex = context.stack.indexOf(this); | |
| const prevOccurrenceIndex = context.stack.indexOf(this); |
| // if (this.update()) { | ||
| // const subs = this.subs; | ||
| // if (subs !== undefined) { | ||
| // shallowPropagate(subs); | ||
| // } | ||
| // } | ||
| // const subscriber = getActiveSubscriber(); | ||
| // if (subscriber !== undefined) { | ||
| // link(this, subscriber, getCurrentCycle()); | ||
| // } | ||
| // const value = this.currentValue; | ||
| // return value!; |
There was a problem hiding this comment.
Remember to delete the comments and logs when you're done with the checklist :)
| const previous = getActiveSubscriber(); | ||
| setActiveSubscriber(this); | ||
| const prevContext = setNewActiveContext(); | ||
| // biome-ignore lint/style/noNonNullAssertion: Non-null assertion because we just updated it in the line above |
| const constex = activeContext!; | ||
| constex.stack.push(this); | ||
| constex.stackNodes.add(this); |
There was a problem hiding this comment.
| const constex = activeContext!; | |
| constex.stack.push(this); | |
| constex.stackNodes.add(this); | |
| const context = activeContext!; | |
| context.stack.push(this); | |
| context.stackNodes.add(this); |
2 participants
Purpose
Part of #62
Summary
Adds a new
CyclicComputednode andcyclicComputedfunction.I am building a system where the user can configure a bunch of stuff, sometimes this can lead to cyclic dependencies, which alien-signals normally doesn't support.
The
CyclicComputednode gets a stabilizer which is called as soon as the same signal is met in one of the dependencies.I would like a review before moving on to the TODOs :)
TODO:
Verification
pnpm -s cicheck