Skip to content

fix(signals): parent the pending companion's lane before poking it (#3379) - #3400

Merged
ryansolid merged 1 commit into
nextfrom
fix/pending-companion-lane-parent
Sep 13, 2026
Merged

ryansolid merged 1 commit into
nextfrom
fix/pending-companion-lane-parent

Conversation

@ryansolid

Copy link
Copy Markdown
Member

Fixes #3379 (Cluster 3 of gabbev's 2026-09-11 batch; its sibling #3378 is fixed by #3397). Independent of #3381/#3392 — based on next.

#3379isPending(details) false while details loads the optimistic value through an async memo

isPending/latest companions ride child lanes that deliberately do not merge with the owner's lane (A14), so an indicator can flush while the owner's async is in flight. The child lane takes its parent from the owner's _optimisticLane when it is created (getOrCreateLane).

In notifyStatus, the companion poke (_updatePendingSignal) ran before assignOrMergeLane(el, lane). When copy's pending status propagated onto details, details had no lane yet, so the companion lane it created was born parentless. The Pending effect depends on both details (the value lane) and the companion (the orphan lane); with no parent/child relation between them, assignOrMergeLane merged the companion lane into the held value lane and the indicator's run sat in that lane's effect queue until the async landed. Reading value() directly in details "worked" because details joined the lane through insertSubs before the poke.

Fix: assign the lane first, inside the same !blockStatus block. A reorder; −4 B minified.

Frames after the fix (the View effect reads details, so it correctly holds 0 / 0 until the derivation lands; the indicator no longer waits):

1000: Pending: false | View: 0 / 0
3000: Pending: true
4000: Pending: false | View: 1 / 1
5000: Pending: true
6000: Pending: false | View: 0 / 0

Before the fix, 3000: Pending: true is missing and 4000 shows View: 1 / 1 alone — the issue's symptom.

Size

Core floor 22,648 → 22,644 B. Four size-limit caps bumped by one step with notes; all brotli noise (next's caps sit within ~10–30 B of their measurements).

Verification

signals 1752/1752 (+1 skipped), solid 595, web 734; tsc clean for src/ and the new test; size-limit passes. No existing test expectation changed.

Co-authored-by: Claude via Cursor

@changeset-bot

changeset-bot Bot commented Sep 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: fec8f85

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@solidjs/signals Patch
test-integration Patch
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/compiler Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
solid-js Patch
@solidjs/universal Patch

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

…3379)

`notifyStatus` poked a node's companions (`_updatePendingSignal`) before
assigning the node's optimistic lane. The companion's child lane takes its
parent from the node's lane at creation, so a memo made pending by
propagation before it rode the lane got a parentless companion lane; the
isPending effect, which also depends on the memo, merged that lane into the
held lane and its run waited on the async it reports. The lane is now
assigned first inside the same `!blockStatus` block.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid
ryansolid force-pushed the fix/pending-companion-lane-parent branch from e1e8e5c to fec8f85 Compare September 13, 2026 00:09
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 34727239915

Coverage remained the same at 71.842%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1007
Covered Lines: 772
Line Coverage: 76.66%
Relevant Branches: 790
Covered Branches: 519
Branch Coverage: 65.7%
Branches in Coverage %: Yes
Coverage Strength: 15.02 hits per line

💛 - Coveralls

@ryansolid
ryansolid merged commit 9da7f0a into next Sep 13, 2026
6 checks passed
@codspeed

codspeed Bot commented Sep 13, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 160 untouched benchmarks


Comparing fix/pending-companion-lane-parent (fec8f85) with next (1af28a1)

Open in CodSpeed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants