Skip to content

fix(signals): OBSERVE.exclude covers store nodes and interaction writes - #3380

Open
ryansolid wants to merge 1 commit into
nextfrom
observe-exclude-writes
Open

fix(signals): OBSERVE.exclude covers store nodes and interaction writes#3380
ryansolid wants to merge 1 commit into
nextfrom
observe-exclude-writes

Conversation

@ryansolid

@ryansolid ryansolid commented Sep 12, 2026

Copy link
Copy Markdown
Member

What

OBSERVE.exclude(owner) now covers writes and stores, not only runs and diagnostics:

  • Store nodes are excluded subjects. registerGraph(proxy, owner) never landed on a store: the proxy's set trap swallows writes outside a draft, so isExcluded(storeNode) was always false. The root target now keys its creating owner (a WeakMap, one set per createStore) and the store's nodes (value, presence, key-set, deep) copy it into _owner as they are created — under the same attrHooks !== null gate as node naming, so node creation's disabled cost stays one null check. registerGraph still runs for the dev tier's owner signal lists / onGraph.
  • Excluded writes don't count toward an interaction. stampWrite routes a root write to an excluded subject into InteractionState.excludedWrites instead of event.writes.
  • An interaction that only touched the observer is forgotten. writes 0, runs 0, created 0, excludedWrites > 0 at settle → removed from the ring, never delivered on subscribe("interaction"). An idle click that wrote nothing at all is still recorded (that's a fact about the app).

Why

Found by the Sentry spike's record-driven adapter (rewritten on the rc.8 subscribe(type) channel): clicking the panel's own "clear" button produced a ui.interaction.click transaction with writes 1, runs 0, even with the panel root excluded and the write made under runWithOwner(panelOwner). The documented contract is that the engine records nothing about the observer's own subtree; this closes the two paths that leaked.

Adapter-side consequence, now in the doc paragraph: the store must be created under the excluded owner, not only written under it.

Size

Engine scenario 26888 → 26944 B (+56 B), limit 26.92 → 26.98 KB with a note. The observe CSR scenario did not move (no observe scenario bundles stores); prod tiers are __OBSERVE__-gated and unchanged.

Tests

  • observe-exclude.test.ts: "forgets an interaction whose only writes went to the panel's own store" — fails without the change (record present, outcome: idle), passes with it; also asserts a click that writes the app and the panel is recorded with writes 1.
  • signals 1752 / solid-js 595 / web 734 pass.

Companion router fix from the same spike run: solidjs/solid-router#604 (guard redirect in the landing flush recorded as a hop, RouterIntegration.inflight).

A root write to an excluded subject no longer counts toward the interaction
that made it, and an interaction whose writes all went to excluded subjects
with none of the app's work run — a click on a devtools panel's own button —
is dropped from the ring and never delivered.

Store nodes were never excluded subjects: registerGraph's `_owner` stamp on
the proxy is swallowed by the set trap outside a draft. The root target now
keys its creating owner and nodes copy it into `_owner` as they are created,
under the same engine-installed gate as node naming.

Found by the Sentry spike's record-driven adapter. Engine scenario
26888 -> 26944 B; observe CSR tier unchanged.

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

changeset-bot Bot commented Sep 12, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 2daff9c

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 34682058256

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

💛 - Coveralls

@codspeed-hq

codspeed-hq Bot commented Sep 12, 2026

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 158 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
fresh stores: create + first write + first commit (reporter shape) 17.4 ms 19.7 ms -11.67%
store reconcile tree reverse: 1111 keyed nodes 29 ms 24.8 ms +16.93%

Tip

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


Comparing observe-exclude-writes (2daff9c) with next (344ed05)

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