Skip to content

fix(storage): clean up sync listeners and guard deferred hydration init - #1071

Merged
davedbase merged 3 commits into
solidjs-community:nextfrom
devagrawal09:fix/storage-sync-cleanup
Oct 15, 2026
Merged

davedbase merged 3 commits into
solidjs-community:nextfrom
devagrawal09:fix/storage-sync-cleanup

Conversation

@devagrawal09

@devagrawal09 devagrawal09 commented Oct 13, 2026 •

Copy link
Copy Markdown

Problem

  1. Sync subscriptions have no explicit unsubscribe path.
  2. With hydrated: true, deferred initialization can overwrite a user write made before the timer fires.

Fix

  • Return [value, setter, init, dispose] from makePersisted, following the make* naming convention requested in review. No automatic owner cleanup.
  • dispose unsubscribes sync listeners and cancels pending initialization. Callers can opt into owner cleanup with onCleanup(dispose).
  • Built-in storage/message/WebSocket subscriptions return unsubscribe functions; multiplexSync combines them. Custom subscriptions may also return an unsubscribe function.
  • Deferred initialization checks unchanged to preserve early writes.

Test

Updated the existing regression in persisted.test.ts to check early writes, explicit timer cancellation, multiplexed listener cleanup, and that disposing an owner does not automatically clean up makePersisted.

@changeset-bot

changeset-bot Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 824470f

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

This PR includes changesets to release 1 package
Name Type
@solid-primitives/storage 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

@netlify

netlify Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for solid-primitives-v2 ready!

Name Link
🔨 Latest commit 824470f
🔍 Latest deploy log https://app.netlify.com/projects/solid-primitives-v2/deploys/6ad00fe6df033431b58fb861
😎 Deploy Preview https://deploy-preview-1071--solid-primitives-v2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e3724930-4052-4df7-9ff6-98a6d413b51d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devagrawal09
devagrawal09 force-pushed the fix/storage-sync-cleanup branch from 5e98ff0 to 2dfa8d5 Compare October 13, 2026 04:08
@atk

atk commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

This goes against our naming patterns. makePersisted should have a dispose function as fourth value in the returned tuple. This being called aautomatically would only be right for a createPersisted primitive.

@atk atk 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.

See above comment

@devagrawal09
devagrawal09 requested a review from atk October 15, 2026 00:23
@davedbase
davedbase merged commit fa670ba into solidjs-community:next Oct 15, 2026
8 of 9 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.

3 participants