Skip to content

fix(signals): preserve signed zero in staged signal reads - #4538

Merged
ryansolid merged 3 commits into
solidjs:nextfrom
GabbeV:fix/signed-zero-staged-read
Oct 31, 2026
Merged

ryansolid merged 3 commits into
solidjs:nextfrom
GabbeV:fix/signed-zero-staged-read

Conversation

@GabbeV

@GabbeV GabbeV commented Oct 31, 2026 •

Copy link
Copy Markdown

Summary

After #4498, an observed createMemo(() => 1 / value()) over createSignal(-0, { equals: Object.is }) stays at -Infinity after setValue(0), even though the signal commits +0. With an async action holding the write, the same equality shortcut also reports the write as not pending and loses the reader's replay at reveal. Both signed-zero directions and equals: false are affected.

Require Object.is as well as the existing strict comparison when identifying netted staging, in the fast read, strict slow read and verdict-routing paths. The test-only reader-seat table uses the same criterion. Opposite signed zeros remain distinct proposals; an accepted NaN rewrite retains its previous behavior because the strict comparison remains. User comparators, netting priority and allocation/state layout are unchanged.

Adds 22 regression/control cases: ordinary derivations, held plain reads versus latest, tracked isPending, both write locations around an action, strict reads, default equality and an accepted NaN rewrite, plus a patch changeset.

How did you test this change?

  • On unchanged next@215c3652, 20 new cases fail and two controls pass. All 22 pass with the fix. Together with the existing netting regressions, all 31 focused tests pass in development, production and observe configurations.
  • Signals JavaScript builds for all three tiers and source declarations pass (npm run build:js, npm run types in packages/signals).
  • Full signals suite: 431 files / 6,192 tests pass, with 21 expected failures and two skips (vitest run --maxWorkers=2).
  • Prettier checks, git diff --check and node scripts/check-release-invariants.mjs pass.
  • Rolldown 1.2.11: +22 minified bytes in the core/createStore/until-refresh scenarios, +44 with isPending/latest. Brotli changes range from -7 to +13 bytes. All four signals size gates pass; no caps changed.
  • Local production timings on Node 24.19.0 remain noisy. Eight fresh-process pairs per configuration gave median paired chain differences of -1.5% for native modules and +0.7% for the bundled core. Other bundled chain/diamond/netted measurements ranged from -4.9% to +5.3%. These measurements do not establish performance equivalence or application performance.

Split the netted-read return from the committed-screen return introduced by solidjs#4498. Strict equality identifies opposite signed zeros, so a write accepted by Object.is or equals: false must serve the staged payload itself. Keep the existing netting order and comparator behavior.

Add observed reciprocal regressions for both signed-zero directions and both comparator options, plus a default-comparator control.
@changeset-bot

changeset-bot Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 6b2a19e

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

This PR includes changesets to release 15 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
board-example Patch
canvas-example Patch
server-dbmon-example Patch
todos-server-example 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

Guard netting with SameValue as well as strict equality. Apply it to fast and strict reads, verdict routing and the seat oracle, preserving accepted NaN rewrites. Cover pending/reveal atomicity with plain and latest readers across action write locations.
@codspeed

codspeed Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

1 warning

⚠️ 2 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 198 untouched benchmarks
⏩ 5 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ spread store record × 500 31.1 ms 33.2 ms -6.32%
⚡ build + consume 608.9 µs 287.9 µs ×2.1

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing GabbeV:fix/signed-zero-staged-read (6b2a19e) with next (215c365)

Open in CodSpeed

Footnotes

  1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@ryansolid
ryansolid merged commit 50dec1b into solidjs:next Oct 31, 2026
7 of 9 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.

2 participants