Skip to content

fix(solid): exclude keys shared across splitProps groups on the proxy path#2812

Open
spokodev wants to merge 1 commit into
solidjs:mainfrom
spokodev:fix/splitprops-proxy-shared-keys
Open

fix(solid): exclude keys shared across splitProps groups on the proxy path#2812
spokodev wants to merge 1 commit into
solidjs:mainfrom
spokodev:fix/splitprops-proxy-shared-keys

Conversation

@spokodev

@spokodev spokodev commented Jul 1, 2026

Copy link
Copy Markdown

Problem

splitProps proxy path duplicates a key that is shared across two groups, disagreeing with its own plain-object path and with the documented contract that a property belongs to a single group.

Repro

// plain object source
const [g1, g2, rest] = splitProps({ a: 1, b: 2, c: 3 }, ["a", "b"], ["b", "c"]);
// g2.b === undefined, "b" in g2 === false, Object.keys(g2) === ["c"], { ...g2 } === { c: 3 }   (correct)

// createStore proxy source (same as component props at runtime)
const [state] = createStore({ a: 1, b: 2, c: 3 });
const [p1, p2, prest] = splitProps(state, ["a", "b"], ["b", "c"]);
// ACTUAL:   p2.b === 2, "b" in p2 === true, Object.keys(p2) === ["b", "c"], { ...p2 } === { b: 2, c: 3 }
// EXPECTED: p2.b === undefined, "b" in p2 === false, Object.keys(p2) === ["c"], { ...p2 } === { c: 3 }

The key b is consumed by the first group but still leaks into the second group through get, has, enumeration, and spread.

Root cause

splitProps has two implementations. The plain-object path walks each property once and assigns it to the first group that lists it (first-match break), so keys are exclusive. The proxy path instead builds one Proxy per group whose get/has/keys each test k.includes(property) against that group's own key list, with no cross-group exclusion. A key present in two groups is therefore reported by both proxied groups. Component props and stores are proxies at runtime, so the proxy path is the one that actually runs for components.

Fix

In the proxy branch, track keys claimed by earlier groups and let each group own only its not-yet-claimed keys. get, has, and keys all read from that owned set, bringing the proxy path to parity with the plain-object first-match semantics. The rest proxy is unchanged, since it already excludes keys.flat().

Authority

Solid docs state splitProps partitions props and each property belongs to one group, and the plain-object implementation in the same function already enforces this with a first-match break.

Tests

Added a case to packages/solid/test/component.spec.ts asserting first-group-only ownership for both a plain object and a createStore proxy across get, has, keys, and spread. It fails on the current proxy path (expected 2 to be undefined) and passes with the fix.

packages/solid suite: 482 passing plus the new test. The only failure is the unrelated pre-existing web/test/transition.spec.tsx "Transition memo stale read (#2046)" flake, which is nondeterministic and passes when run in isolation on the clean tree.

… path

The proxy branch of splitProps built one Proxy per group whose
get/has/keys each tested membership against that group's key list
independently, with no cross-group exclusion. A key listed in two
groups was therefore exposed by both proxied groups, while the
plain-object path assigns each key to the first group that lists it
(first-match break). Component props and stores are proxies at runtime,
so the proxy path is the live one.

Track keys claimed by earlier groups and let each group own only its
not-yet-claimed keys, so get, has, and keys all honor the exclusion and
match the plain-object semantics.
@changeset-bot

changeset-bot Bot commented Jul 1, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: f97e08f

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

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.

1 participant