fix(storage): restore makePersisted type inference for input signal - #1064
wahajahmed010 wants to merge 1 commit into
Conversation
The 4.4.0 release rewrote the public makePersisted overloads to take
<T, S extends Signal<T> | [Store<T>, SetStoreFunction<T>]>, expecting
TypeScript to infer T from S. Under stricter inference (TS 5/7) that
no longer happens reliably: T falls back to unknown, so calls like
makePersisted(createSignal('hello'), { name, storage: localStorage })
fail with TS2769 ("No overload matches this call"). Users had to add
explicit type arguments as a workaround.
Restore the 4.3.5 shape: public overloads only take <S>, with
SignalType<S> extracting T. The implementation signature keeps a
defaulted T = SignalType<S> so internal callers and the body still
compile. Add a regression test that fails on the old overloads and
passes here.
Closes solidjs-community#1000
Signed-off-by: Wahaj Ahmed <wahaj.ahmed010@gmail.com>
🦋 Changeset detectedLatest commit: cb1b61e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughmakePersisted now derives its value type from the supplied signal or store pair. Its overloads also allow options to be omitted when no storage-options type applies. A regression test checks string inference and signal updates. ChangesPersisted signal type inference
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The inference fix is mergeable after normal checks; no outstanding risk is established. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This branch has not been deployed
1 participant
Fixes #1000.
The 4.4.0 release rewrote the public
makePersistedoverloads to<T, S extends Signal<T> | [Store<T>, SetStoreFunction<T>]>, relying on TypeScript to inferTfromS. Under stricter inference (TypeScript 5/7) that no longer happens reliably, soTfalls back tounknownand the call site errors with TS2769 (No overload matches this call). Users had to fall back to explicit type arguments likemakePersisted<string, Signal<string>>(...).This change restores the 4.3.5 shape:
SignalInput/SignalType<S>helper pair is reintroduced so the public overloads only take<S>.Tis extracted viaSignalType<S>in thePersistenceOptions<T, ...>argument, which is the standard pattern for inferring the inner type of a signal/store tuple.T = SignalType<S>as a defaulted type parameter, so the body and internal call sites are unchanged.infers the signal value type from the input without explicit annotations) that fails on the previous overloads and passes here. Running it against the old source withtsc --strictreproduces the originalTS2769error verbatim.No runtime changes — pure type-level fix. The full vitest suite for
@solid-primitives/storage(21 tests across 4 files) passes.Summary by CodeRabbit
makePersisted, so signal values retain their expected types instead of being inferred asunknown.makePersistedwithout storage options when no options are required.