Skip to content

fix(signals): wake blocked readers after equal-value pending recovery - #3371

Merged
ryansolid merged 3 commits into
solidjs:nextfrom
DerpyCrabs:fix/settle-unchanged-pending-branches
Sep 11, 2026
Merged

fix(signals): wake blocked readers after equal-value pending recovery#3371
ryansolid merged 3 commits into
solidjs:nextfrom
DerpyCrabs:fix/settle-unchanged-pending-branches

Conversation

@DerpyCrabs

Copy link
Copy Markdown

A conditional memo can stop reading a pending dependency and recover to its previous value:

const enabled = createMemo(() => !disabled() && asyncData.enabled);

If a dependent tries to read while this memo is pending, setting disabled(true) should unblock it—even if enabled was already false before loading. Currently, equality suppresses notification and the dependent can remain stuck pending.

In our app, this leaves list rows missing after a successful fetch: their rendering is blocked on a conditional event-handler dependency that is no longer needed.

Reuse the pending-settlement walk to clear inherited pending state on equal-value recovery. Retry blocked readers without rerunning unchanged dependents or clearing pending state still required through another dependency path.

Seven regression cases pass across all three build modes; four fail on unpatched next.

@changeset-bot

changeset-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4659711

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

@DerpyCrabs
DerpyCrabs force-pushed the fix/settle-unchanged-pending-branches branch from 8a0f292 to b9be897 Compare September 11, 2026 09:40
The equal-value recovery loop retires every outgoing pending source from
the dependents it orphaned. When that set holds the node itself (its own
superseded flight, solidjs#3181), the wasPendingSource sweep below already runs
settlePendingSource(el) — skip it in the loop rather than walk the
dependents twice. Comment why the retryReaches early return in the settle
closure does not mark the node visited (converging unchanged branches).
Treeshake note updated to the measured 22,648.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid

Copy link
Copy Markdown
Member

Thanks — the bug and the fix shape both check out: the recovered node acts as the settling source for the subtree it orphaned, riding the existing settlePendingSource walk, so the _blocked reader is woken without re-running non-blocked dependents. (For the record: status already clears when the dropped source lands; the actual defect was that the parked reader was never re-enqueued.)

I pushed one commit on top (fdef2fb):

Local: signals dev suite 1751 passed, prod tier at the 238-failure baseline, solid/web green. Merging once CI agrees.

Claude via Cursor

Seven entries were 13–62 B brotli over after the +191 B minified core
change (retryReaches core-retained, settlePendingSource's source
parameter). Each cap carries the measured size and the prior cap.

Co-authored-by: Cursor <cursoragent@cursor.com>
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 34587503635

Warning

No base build found for commit b5bd6fb on next.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 71.842%

Details

  • Patch coverage: No coverable lines changed in this PR.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


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.01 hits per line

💛 - Coveralls

@codspeed-hq

codspeed-hq Bot commented Sep 11, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 54.44%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 159 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
projection derive: write one NESTED field (reference) 807.9 µs 523.1 µs +54.44%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing DerpyCrabs:fix/settle-unchanged-pending-branches (4659711) with next (b5bd6fb)

Open in CodSpeed

@ryansolid
ryansolid merged commit dd1d4ed into solidjs:next Sep 11, 2026
6 checks passed
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.

3 participants