Skip to content

fix(solid): preserve server projection errors - #3224

Merged
ryansolid merged 2 commits into
solidjs:nextfrom
GabbeV:fix/server-projection-error-state
Sep 2, 2026
Merged

fix(solid): preserve server projection errors#3224
ryansolid merged 2 commits into
solidjs:nextfrom
GabbeV:fix/server-projection-error-state

Conversation

@GabbeV

@GabbeV GabbeV commented Sep 2, 2026

Copy link
Copy Markdown

Summary

  • keep pending server projection proxies errored when their async source rejects
  • rethrow the original error from later property reads instead of exposing the seed
  • cover the behavior with a seeded Promise projection regression test

This is extracted from #3194 so the baseline bug can be reviewed independently. It intentionally keeps the existing proxy target and reflection/symbol behavior unchanged; it does not include the broader pending-proxy membrane from that proposal.

Test plan

  • pnpm exec vitest run test/server/ssr-async.spec.ts (158 tests)
  • pnpm types
  • pnpm test-types

@changeset-bot

changeset-bot Bot commented Sep 2, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 1e6b7e5

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

This PR includes changesets to release 11 packages
Name Type
solid-js Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
test-integration Patch
@solidjs/universal Patch
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/compiler Patch
@solidjs/diagnostics Patch
@solidjs/signals 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

@ryansolid ryansolid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extracted fix is independent from #3194 and the implementation appears sound, but it changes four settlement paths while only Promise rejection is covered. Please add focused coverage for at least (1) an iterable rejecting before its first yield and (2) seedLoadingValue rejecting, including repeated/different ordinary property reads rethrowing the original error.

Please also narrow the changeset wording: ordinary string-key reads stop exposing the seed, while reflection/symbol behavior intentionally remains unchanged.

No API concern. The measured impact is server-only at about +38 bytes gzip; please keep the broader pending-proxy membrane out of this PR.

@ryansolid

Copy link
Copy Markdown
Member

This is a worthwhile memo/projection parity fix and the narrow implementation looks sound. I'm going to add the iterable and seedLoadingValue rejection regressions and tighten the changeset wording directly, then we'll move this toward merge. No action needed from you on the earlier coverage request.

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

@ryansolid ryansolid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the missing iterable and seedLoadingValue rejection regressions and narrowed the changeset wording. The SSR async suite, workspace build, declarations/type tests, formatting, and all size scenarios pass. The implementation remains narrow and server-only.

@ryansolid
ryansolid merged commit e59a8a9 into solidjs:next Sep 2, 2026
2 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