Draft
Conversation
Reuse the subscriber local through the loop and collapse the optimistic branch so the dirty flag is set in one place without changing queue behavior.
Remove the single-use reassignPendingTransition() helper function and inline its 4-line loop at both call sites (scheduler.ts:275, 299). The function adds unnecessary indirection without providing clarity or reusability. This reduces cognitive overhead and makes the transition reassignment logic directly visible where it's used, improving code simplicity.
Replace do-while with break pattern with a clearer while loop that uses early returns. This eliminates the confusing break-on-depsTail pattern, making the early-exit strategy immediately obvious. The logic is identical— both implementations iterate the dep chain and return true on match or false when reaching depsTail—but the while-with-returns is more direct and easier to reason about. No behavior change; all tests pass, benchmarks stable.
Simplify the reactive core by eliminating duplicate code in insertSubs and queueStashedOptimisticEffects. Both functions had identical tracked effect handling logic that checked for EFFECT_TRACKED and enqueued them directly to the effect queue. Extract this logic into a dedicated helper function enqueueTrackedEffect() that returns a boolean indicating whether the effect was tracked. This removes the duplication, makes the intent clearer, and reduces overall complexity in the scheduler without changing behavior or affecting performance.
Return early from transitionComplete instead of threading a mutable done flag through both pending-node scans. This keeps the completion conditions identical while making the control flow easier to read.
Keep dependency-link validation as a single linear scan so the stop condition lives in the loop structure instead of an extra branch in the loop body.
Use early returns in unlinkSubs so the common removal path stays linear and the last-subscriber cleanup path no longer sits inside nested else blocks.
Use a direct max-based length assignment so heap growth no longer needs a separate guard while preserving the same no-shrink behavior.
Keep the dependency unlink traversal in the for-loop header so unobserved no longer needs a separate mutable loop variable.
Return the memo accessor directly from computed() so the wrapper no longer needs a temporary node variable.
Remove the one-use dep temporary in adjustHeight so the firewall fallback stays on a single line with the height check.
Skip empty lane effect queues with an early continue so runLaneEffects keeps the work path linear without changing scheduling behavior.
Keep createTrackedEffect as a single forwarding expression so the wrapper stays as direct as the primitive it delegates to.
Let the createReaction cleanup slot rely on its optional type instead of an explicit undefined initializer to keep the wrapper state smaller.
Use const for read-only heap flag snapshots and simplify scheduler queue helpers so the hot-path control flow stays more direct without changing reactive behavior.
Use direct iteration in small scheduler helpers so the code carries fewer mutable loop locals while preserving queue and transition behavior.
Replace manual index loops in transition bookkeeping with direct iteration so the scheduler carries less mutable state while preserving queue and transition order.
Let markNode's child traversal loop handle the null case directly so the recursion path needs one less branch and one less nesting level.
Use an early return in Queue.removeChild so the actual unlink path stays flat without changing parent-child queue semantics.
Use an early return in enqueueTrackedEffect so the tracked-effect path stays linear without changing queueing behavior.
The ternary `completingTransition ? lane._transition === completingTransition : !lane._transition` always reduces to `lane._transition === completingTransition`. When completingTransition is null, `lane._transition === null` is identical to `!lane._transition` because _transition is typed as `Transition | null` and can only hold an object reference or null. The two branches of the ternary compute the same comparison in both cases, so the ternary is redundant. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The condition `dep._height >= newHeight` before assigning `dep._height + 1` is equivalent to using Math.max directly. Both produce the same result for all inputs, but Math.max expresses the "keep the maximum" intent more directly and removes the explicit comparison operator. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…irectly The `let newHeight` local was used to accumulate the max dep height and then assigned back to `el._height` at the end. Since Math.max is monotonically non-decreasing, updating `el._height` in-place produces identical results. This removes the intermediate mutable variable and the trailing assignment, making the intent (update the node's height if any dep is taller) more direct. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The indexed for loop maintained a separate `let i` counter and two local consts just to access the same positions in two arrays and guard against missing children. Using stub._children.entries() with destructuring gives both index and value in one declaration (removing the mutable `let i`) and optional chaining replaces the explicit `if (child)` null guard, eliminating two lines with no behavior change. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The pattern `if (queue._min > sub._height) queue._min = sub._height` appeared twice in scheduler.ts (insertSubs and queueStashedOptimisticEffects). Both sites just clamp _min down to the new subscriber height, which is exactly what Math.min expresses without an explicit branch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The loop body needed both the index and the child element. Using entries() replaces the manual `let i` counter and the explicit `const child = this._children[i]` lookup with a single destructured binding, matching the pattern already in use in restoreQueues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
`runQueue([])` iterates nothing, so the `if (!effects.length) continue` guard before swapping and running the effect queue is a no-op. Removing it makes the drain path unconditional and removes one branch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The if-guard already asserts !(computed._flags & REACTIVE_ZOMBIE), so the inner ternary `computed._flags & REACTIVE_ZOMBIE ? zombieQueue : dirtyQueue` always resolved to dirtyQueue. Replace with dirtyQueue directly and drop the now-unused zombieQueue import. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The CHECK branch cleared both CHECK and DIRTY then immediately OR-ed DIRTY back in. Clearing a bit you set in the same expression is a no-op, so drop REACTIVE_DIRTY from the clear mask for clarity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Both computed() and createOwner() checked whether _firstChild was null before prepending a new node. Both branches assigned _firstChild = self; the only difference was the else also set _nextSibling. Since _nextSibling is already initialized to null, assigning null is a no-op, so we can unconditionally set both fields and drop the branch + temporary variable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e + assertion The do…while + toRemove! pattern required a mutable local, a non-null assertion, and (in dispose) a redundant || null coercion. Replace both sites with the for-loop idiom already used inside unlinkSubs itself. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.