Skip to content

refactor(signals): shared read predicates — readerSeesCommitted, visibleOverride, one hasActiveOverride - #3515

Open
ryansolid wants to merge 1 commit into
nextfrom
refactor/shared-read-predicates
Open

ryansolid wants to merge 1 commit into
nextfrom
refactor/shared-read-predicates

Conversation

@ryansolid

Copy link
Copy Markdown
Member

Step 1 of DESIGN-CONSOLIDATION move 3b (one implementation per rule). Zero semantic change — the gate is the point of the PR, not the code.

What moved

  • readerSeesCommitted(el, c, owner, noCommitted) (core.ts): the full committed-vs-staged arm — untracked / lane-reads-committed (Existing and newly revealed latest readers display different values #3460) / nothing staged / children-forbidden (A32) / stale-of-foreign (heldFromStale) / HELD-truth mask (2.0.0-rc until doesnt entangle the update resolving it with the action update #3164) — that read()'s slow tail used to inline. Documented as the rule it enforces; the store's backing selection and the verdict arms adopt it in step 2.
  • hasActiveOverride: existed twice (lanes.ts, store.ts). One definition in core.ts now; lanes.ts and store.ts re-export.
  • visibleOverride (hasActiveOverride && !unflushedOverride, A28 (5)): was the store's helper, restated by hand at latestRead, computePendingState ×2 and the store's reader selection. One definition in core.ts; five call sites.

Hot paths untouched: readNodeFast, read()'s fast block, setSignal, recompute are byte-identical. read()'s override arm keeps its shape (the AUTHORITATIVE_OBSERVED mark still fires for an unflushed override) — only the inline test became hasActiveOverride(el).

Gate

check result
signals suite 197 files / 2746 pass
visibility oracles (signal, store) report byte-identical to next
posture matrix (621 cells, 738-line report) byte-identical to next
fuzzer #3446, seed 3289 × 1000 994 pass / 0 fail / 6 policy — same as next
solid-js 623 pass
web same 12 pre-existing server-diagnostics failures as origin/next in this worktree (component-location naming, unrelated)
minified (treeshake fixture) core +13 B, +createStore −38 B, isPending/latest +34 B, full bundle −72 B
brotli, pure-signals fixtures −4 / −29 / +26 / −5 B

Three size-limit caps ratcheted (+9 / +44 / +21 B brotli on esbuild app bundles) with notes: the ten scenarios moved −19…+45 B in both directions, compressor layout, not code.

Local wall-clock microbench A/B was order-dependent (−7% one order, +4% the other) — CodSpeed on this PR is the perf verdict; structurally only read()'s slow tail gained a call (small, monomorphic, inlined).

Next (step 2)

serve(el, reader): the store's pendingBackingVisible and serveDataKey adopt readerSeesCommitted; then readerLive(reporter, verdict); then the verdict-deferred list.

Made with Cursor

…bleOverride, one hasActiveOverride

Zero-semantic-change step 1 of DESIGN-CONSOLIDATION move 3b (one implementation
per rule). read()'s slow tail called the full committed-vs-staged arm inline;
it is now readerSeesCommitted(el, c, owner, noCommitted), documented as the rule
it enforces, for the store's backing selection and the verdict arms to adopt.
hasActiveOverride existed twice (lanes.ts, store.ts) and the composite
'hasActiveOverride && !unflushedOverride' was restated at five sites
(latestRead, computePendingState ×2, the store's reader selection, plus the
store's own visibleOverride) — one visibleOverride in core now.

Gate: signals suite green; visibility oracles and the 738-line posture matrix
report are byte-identical to next.

Co-authored-by: Claude via Cursor <noreply@cursor.com>
@changeset-bot

changeset-bot Bot commented Sep 17, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6c86076

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

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 35199249975

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage remained the same at 71.46%

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: 1018
Covered Lines: 772
Line Coverage: 75.83%
Relevant Branches: 790
Covered Branches: 520
Branch Coverage: 65.82%
Branches in Coverage %: Yes
Coverage Strength: 14.93 hits per line

💛 - Coveralls

@codspeed

codspeed Bot commented Sep 17, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 175 untouched benchmarks


Comparing refactor/shared-read-predicates (6c86076) with next (a1a3bc6)

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