Skip to content

fix: handle runtime edge case regressions#2863

Closed
brenelz wants to merge 1 commit into
solidjs:nextfrom
brenelz:fix/runtime-edge-case-regressions
Closed

fix: handle runtime edge case regressions#2863
brenelz wants to merge 1 commit into
solidjs:nextfrom
brenelz:fix/runtime-edge-case-regressions

Conversation

@brenelz

@brenelz brenelz commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Used the new GPT 5.6 Sol model to hunt bugs and this what it came up with.

Feel free to take as many or as little as you want

Summary

  • reject actions correctly when generators throw falsy values and settle consumers of asynchronously empty iterables
  • preserve native array index semantics and keyed store identity with leading null entries
  • unlink disposed SSR owners before pooling so reused owners cannot be disposed through stale parent links
  • add regression coverage for each corrected path

Testing

  • pnpm test in packages/solid-signals (900 tests)
  • pnpm test in packages/solid (432 tests)
  • pnpm types in packages/solid-signals
  • pnpm types in packages/solid
  • pnpm test-types in packages/solid

@changeset-bot

changeset-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: d2f5576

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codspeed-hq

codspeed-hq Bot commented Jul 9, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by ×2.3

⚡ 1 improved benchmark
✅ 119 untouched benchmarks

Performance Changes

Benchmark BASE HEAD Efficiency
merge 166.9 µs 73.4 µs ×2.3

Tip

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


Comparing brenelz:fix/runtime-edge-case-regressions (d2f5576) with next (a99d6c8)

Open in CodSpeed

ryansolid pushed a commit that referenced this pull request Jul 10, 2026
ryansolid pushed a commit that referenced this pull request Jul 10, 2026
ryansolid pushed a commit that referenced this pull request Jul 10, 2026
Co-authored-by: Cursor <cursoragent@cursor.com>
@ryansolid

Copy link
Copy Markdown
Member

Thanks @brenelz — took you up on the à la carte offer. Here's the accounting:

Landed (committed to next, authored to you):

  • action.ts falsy generator throwsbcb0ca6. Confirmed red-first: throw undefined inside an action generator resolved the promise instead of rejecting. The explicit failed flag is the right shape — same bug class as 2.0.0-beta.16: Errored fallback receives the internal StatusError wrapper for bare rejections #2866.
  • store.ts strict array-index detectionfda28a9. Verified the repro: store["01"] = x on an empty array grew length to 2 via parseInt. Your canonical-index check (Number.isInteger + bounds + String(index) === property) matches the spec definition and subsumes the old symbol/length guards.
  • server/signals.ts owner pool unlink25a5685. This was the most serious of the four — a real use-after-recycle: the pool cleared _parent/_nextSibling on the pooled node but the parent's child chain still referenced it, so disposing the old parent could walk into whatever tree the recycled owner ended up in and dispose live owners. Your test proved it red-first. I ported it by hand (the diff had drifted against recent hydration changes) and verified subtree disposal stays O(1) per unlink and boundary retries (self=false) are unaffected.

Superseded:

Declined:

  • The reconcile.ts hasKeyedItem change. The leading-null inconsistency is real (mid-array nulls already survive keyed reconcile per the 2.0.0-beta.14: Keyed reconcile crashes on nullable or primitive array entries #2772 tests), but scanning next for any keyed item makes the keyed/positional decision O(n) for the common reconcile(primitiveArray) case where it's O(1) today, and it changes behavior for arrays mixing keyless objects among keyed ones (their undefined keys would collide in the index map). The failure mode it fixes is also mild — positional fallback still reconciles correct values, it only loses referential identity. Not worth the cost for [null, ...items]-shaped data; happy to revisit if it bites someone in practice.

Closing since everything here is either landed or intentionally passed on. Appreciate the hunting — 3 of 4 were genuine bugs with clean fixes.

@ryansolid ryansolid closed this Jul 10, 2026
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