Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(utils): add subscribeKeys #887

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat(utils): add subscribeKeys #887

wants to merge 5 commits into from

Conversation

wrong7
Copy link

@wrong7 wrong7 commented Apr 22, 2024

Summary

New utility function based on the current subscribeKey that allows subscriptions to multiple properties of a proxy.

This PR also adds an optional second argument to the callback function from subscribeKey, and therefore the new subscribeKeys to provide the previous value of the changed property.

I felt this utility was needed when I had to subscribe to a proxy that changed multiple properties at once, and I needed those changes to execute a single callback.

Usage

import { subscribeKeys } from 'valtio/utils'

const state = proxy({ width: 0, height: 0 })
subscribeKeys(state, ['width', 'height'], ([width, height], [oldWidth, oldHeight]) =>
  console.log('Area has changed from', oldWidth * oldHeight, 'to', width * height),
)

Check List

  • yarn run prettier for formatting code and docs

Copy link

vercel bot commented Apr 22, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
valtio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 22, 2024 1:15pm

Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@dai-shi
Copy link
Member

dai-shi commented Apr 22, 2024

Thanks for the suggestion.
We have a subtle design constraint.
Basically, we would like to provide only primitive capabilities and have utils outside. (and Valtio Labs https://github.com/valtiojs is for it.)
Due to this, I don't feel like having both subscribeKey and subscribeKeys in valtio/utils. Our options are:

  • Add subscribeKeys in valtiojs/*
  • Add both subscribeKey and subscribeKeys in valtiojs/* and remove subscribeKey from valtio/utils in v2
  • Remove subscribeKey from valtio/utils in v2 and add subscribeKeys to valtio/utils (in v1, deprecating subscribeKey)

Another note is that we don't generally recommend using subscribeKey. We (or at least I) prefer subscribe instead.
The current subscribeKey is like a reference implementation. So, I probably like the first option and we are free to add subscribeWithPath(s) if we want.

I'm not sure if I'm happy with returning the previous value. It can be a proxy object, and mutable. I think we should snapshot it, but that doesn't seem very good design. Yeah, this make me think we should go with the second option.

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.

None yet

2 participants