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

Closed
wants to merge 5 commits into from
Closed

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.

@dai-shi
Copy link
Member

dai-shi commented Jun 5, 2024

We are about to release v2 sometime soonish.
Let's consider subscribeKeys is for valtiojs org.
Let me know if anyone is interested in working on it as well as subscribePath(s).

@dai-shi dai-shi closed this Jun 5, 2024
@ayangweb
Copy link

ayangweb commented Jun 6, 2024

We are about to release v2 sometime soonish. Let's consider subscribeKeys is for valtiojs org. Let me know if anyone is interested in working on it as well as subscribePath(s).

Hey👋, I'm interested in contributing!

@dai-shi
Copy link
Member

dai-shi commented Jun 6, 2024

Hey👋, I'm interested in contributing!

Sent an invitation!
You want to also join our Discord server(s) so that we can discuss there if you want.

@ayangweb
Copy link

ayangweb commented Jun 6, 2024

嘿👋,我有兴趣做出贡献!

已发送邀请! 您还想加入我们的 Discord 服务器,以便我们可以在那里进行讨论(如果您愿意的话)。

Okay, I've accepted the invitation and joined Discord!

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

3 participants