fix(signals): require complete store seeds - #3258
Conversation
🦋 Changeset detectedLatest commit: 5ceea76 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
Coverage Report for CI Build 33782694918Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage remained the same at 71.814%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
…stores (#3258) Derived createStore/createProjection/createOptimisticStore accept Partial<T> seeds again. #3258 merged without maintainer review; on review the ruling is that the requirement's ergonomic cost exceeds what it fixes: any async object store reconciling on a non-'id' key needs the options slot and therefore the seed slot, and there the seed is pure ceremony — reads pend until the first resolution, so a fabricated complete T is never observable (verified: an uninitialized derived read throws NotReadyError; the seed never leaks). The type-honesty gain is real only for sync draft-reading callbacks and is better served by the #3194 seedless-callback direction. #3260's overload alignment (slot order, shallow in options, Refreshable derived returns, NoFn on plain roots) stays. Since #3258 never shipped, its pending changeset is dropped; the API is unchanged from 2.0.0-rc.6. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Heads up: this has been reverted on Process first: this PR was merged by Ryan's agent without his review — a process error on the agent's side, not a maintainer approval. When Ryan reviewed the merged state, he ruled against the design. We're flagging that plainly so the reversal reads as what it is: the design question was never actually decided until now, and contributors aiming at the project's rulings shouldn't read this as instability. The ruling on the substance: the completeness requirement costs more than it fixes. The case that decides it: any async object store that reconciles on a non- createStore(async () => fetchUser(), /* complete User required here */, { key: "email" });and in exactly that case the seed is pure ceremony: reads of an async derived store pend with The concern itself is legitimate, and we think the right home for it is your #3194 direction: a callback that takes no draft parameter needs no seed at all, which fixes the awkward case and keeps the honesty for draft-taking callbacks. That conversation is still live. #3260 is untouched by the revert — the overload alignment (slot order, |
3 participants
Summary
Split from #3194 to isolate the seed type-safety change.
Derived store APIs currently accept a
Partial<T>seed while exposing both the returned store and the projection draft as a completeT. This allows required properties to be absent at runtime even though reads and mutations are typed as if they are present.This changes the seed to a complete
TforcreateStore(fn, seed),createProjection, andcreateOptimisticStore(fn, seed)across the signals, client, and server declarations. It also applies the existingNoFn<T>exclusion consistently, since a callable root cannot be distinguished from the function form by the runtime dispatch.Existing tests that intentionally initialize from
{}and establish the complete value through mutation now use{} as any. This preserves that pattern as an explicit unsafe escape hatch rather than claiming it is type-safe.No runtime behavior is intended to change.
How did you test this change?
pnpm typesinpackages/signals.pnpm exec tsc -p tsconfig.build.json --noEmitinpackages/solid.store.type-tests.tsindependently with strict TypeScript settings.