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: ref api #62

Merged
merged 4 commits into from
Jan 10, 2021
Merged

feat: ref api #62

merged 4 commits into from
Jan 10, 2021

Conversation

dai-shi
Copy link
Member

@dai-shi dai-shi commented Jan 4, 2021

close #61

Summary

ref will mark an object so that it won't be wrapped by proxies when used as a (nested) property.

Usage

import { proxy, ref } from 'valtio'

const state = proxy({
  obj: {},
  body: ref(document.body), // this is not tracked
})

Caveat 1

This ref is for tracking mutations only.

For render optimization (= useProxy(state) use case), objects could wrapped by proxies if they are plain objects, but that would only happen on demand, so it shouldn't be a problem. It doesn't influence the state object either.

Fixed in #65

Caveat 2

const state = proxy(ref(obj)) // this is not supported, it's nonsense

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 4, 2021

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.

Latest deployment of this branch, based on commit fc5cd6a:

Sandbox Source
React Configuration
React Typescript Configuration

@dai-shi
Copy link
Member Author

dai-shi commented Jan 4, 2021

@drcmda Would you be able to try this build and see if it works as expected?

@drcmda
Copy link
Member

drcmda commented Jan 6, 2021

@dai-shi works! :-D blocks inner mutation, but ref equality still holds.

@dai-shi
Copy link
Member Author

dai-shi commented Jan 7, 2021

@drcmda
Great to hear that. Would you help me adding this feature in readme.md? Any suggestion?

@dai-shi dai-shi merged commit 4ebdb2b into master Jan 10, 2021
@dai-shi dai-shi deleted the fix/issue-61 branch January 10, 2021 02:41
@dai-shi
Copy link
Member Author

dai-shi commented Jan 10, 2021

@StarpTech
Copy link

StarpTech commented Jul 18, 2021

This feature is very helpful. Without bypassing the EventEmitter the code won't work when subscribing to an event.

import { proxy, ref } from 'valtio';

export const state = proxy({
  isConnecting: false,
  signals: ref(new EventEmitter()),
  connection: null
});

The following code will results in:

Uncaught TypeError: 'set' on proxy: trap returned falsish for property 'Notification'
import { proxy, ref } from 'valtio';

export const state = proxy({
  isConnecting: false,
  signals: new EventEmitter(),
  connection: null
});

// anywhere
const { connection, signals } = useSnapshot(state);
useEffect(() => {
 signals.on('Notification', fn);
 return () => {
   return signals.off('Notification', fn);
 };
}, [signals)

@dai-shi is it intentional or a bug?

@dai-shi
Copy link
Member Author

dai-shi commented Jul 18, 2021

@StarpTech You might want to try using state.signals instead inside useEffect. If it doesn't work, ref() would be unavoidable.

@StarpTech
Copy link

StarpTech commented Jul 18, 2021

It doesn't work. I'm wondering why it happened at all. It feels like a bug. See GoogleChrome/proxy-polyfill#20 (comment)

@StarpTech
Copy link

StarpTech commented Jul 18, 2021

Removing ref() will result in an infinite loop (frozen page).

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.

💊 [feature request]: valtio should support non-serializable data, too
3 participants