feat(vanilla): unstable_buildProxyFunction instead of unstable_getHandler #528

dai-shi · 2022-08-18T09:08:30Z

close #522

This is a breaking change on unstable features.

Some use cases

Use deepEqual instead of Object.is

import { unstable_buildProxyFunction } from 'valtio'
export { useSnapshot } from 'valtio'
export const [proxy] = unstable_buildProxyFunction(deepEqual)

Attach `get` trap to proxy handler

import { unstable_buildProxyFunction } from 'valtio'
export { useSnapshot } from 'valtio'
export const [proxy] = unstable_buildProxyFunction(
  undefined,
  (target, handler) => {
    handler.get = ...
    return new Proxy(target, handler)
  }
)

Avoid some objects to proxy instead of using `ref` for all

import { unstable_buildProxyFunction } from 'valtio'
export { useSnapshot } from 'valtio'
const canProxyOrig = unstable_buildProxyFunction()[7]
export const [proxy] = unstable_buildProxyFunction(
  undefined,
  undefined,
  (x) => {
    if (...) {
      return false
    }
    return canProxyOrig(x)
  }
)

vercel · 2022-08-18T09:08:32Z

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

Name	Status	Preview	Updated
valtio	✅ Ready (Inspect)	Visit Preview	Aug 31, 2022 at 3:00AM (UTC)

codesandbox-ci · 2022-08-18T09:08:56Z

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 3dd8dbf:

Sandbox	Source
React	Configuration
React Typescript	Configuration
React Browserify	Configuration
React Snowpack	Configuration
React Parcel	Configuration
youthful-rosalind-pq26bq	Issue #522

github-actions · 2022-08-18T09:10:29Z

Size Change: +642 B (+1%)

Total Size: 49.3 kB

Filename	Size	Change
`dist/esm/index.js`	731 B	-3 B (0%)
`dist/esm/vanilla.js`	1.98 kB	+86 B (+5%)	🔍
`dist/index.js`	883 B	+3 B (0%)
`dist/system/index.production.js`	512 B	+4 B (+1%)
`dist/system/vanilla.development.js`	2.09 kB	+81 B (+4%)
`dist/system/vanilla.production.js`	1.29 kB	+40 B (+3%)
`dist/umd/index.development.js`	1.01 kB	+3 B (0%)
`dist/umd/index.production.js`	671 B	+1 B (0%)
`dist/umd/vanilla.development.js`	2.29 kB	+184 B (+9%)	🔍
`dist/umd/vanilla.production.js`	1.39 kB	+67 B (+5%)	🔍
`dist/vanilla.js`	2.19 kB	+176 B (+9%)	🔍

ℹ️ View Unchanged

Filename	Size
`dist/esm/macro.js`	617 B
`dist/esm/macro/vite.js`	732 B
`dist/esm/utils.js`	4.01 kB
`dist/macro.js`	910 B
`dist/macro/vite.js`	1.04 kB
`dist/system/index.development.js`	894 B
`dist/system/macro.development.js`	725 B
`dist/system/macro.production.js`	555 B
`dist/system/macro/vite.development.js`	839 B
`dist/system/macro/vite.production.js`	660 B
`dist/system/utils.development.js`	4.21 kB
`dist/system/utils.production.js`	2.83 kB
`dist/umd/macro.development.js`	1.06 kB
`dist/umd/macro.production.js`	766 B
`dist/umd/macro/vite.development.js`	1.21 kB
`dist/umd/macro/vite.production.js`	893 B
`dist/umd/utils.development.js`	4.73 kB
`dist/umd/utils.production.js`	3.02 kB
`dist/utils.js`	4.58 kB

_{compressed-size-action}

BrianHung · 2022-08-29T23:56:17Z

src/vanilla.ts

+  const customObjectIs = customizeObjectIs(Object.is)
+  const customCanProxy = customizeCanProxy(canProxy)
+  const customCreateSnapshot = customizeCreateSnapshot(createSnapshot)
+  const customCreateProxy = customizeCreateProxy(createProxy)


Is the reason to pass the default functions as arguments into the custom functions? Think this would be cleaner:

objectIs = options?.objectIs ?? Object.is canProxy = options?.canProxy ?? canProxy // ...

My imaginary use case is something like:

const customizeCanProxy = (origCanProxy) => { const canProxy = (x) => { if (...) return true/false return origCanProxy(x) } return canProxy }

Similar to the comment I left below, but I think most developers will either use all-or-none of the default behavior, instead of a mix and match. So exporting and exposing defaults would be better than passing defaults as arguments into a higher order function that the developer returns.

BrianHung · 2022-08-29T23:57:14Z

src/vanilla.ts

 const VERSION = __DEV__ ? Symbol('VERSION') : Symbol()
 const LISTENERS = __DEV__ ? Symbol('LISTENERS') : Symbol()
 const SNAPSHOT = __DEV__ ? Symbol('SNAPSHOT') : Symbol()
-const HANDLER = __DEV__ ? Symbol('HANDLER') : Symbol()
 const PROMISE_RESULT = __DEV__ ? Symbol('PROMISE_RESULT') : Symbol()
 const PROMISE_ERROR = __DEV__ ? Symbol('PROMISE_ERROR') : Symbol()


Is it possible to export these Symbols? Easier to define custom behavior when you have access all internals, instead of only having access to default methods.

For example, checking if an object has been proxied by valtio value[LISTENERS].

For now, I still want to hide them. I might consider combining into one symbol later (then it can ease customizing.) I first tried in this PR, but it made things complicated.

Here’s an example of using one symbol only: https://github.com/immerjs/immer/blob/e28b53f2d12696954b1f0125494498e34d77a3df/src/core/proxy.ts#L55

It uses one symbol to access an object which in valtio’s case can contain the listeners and version.

Yep. I wish to combine SNAPSHOT too. Removing symbols completely and using WeakMaps might be possible, but maybe performance-wise a little worth. Anyway, let's tackle it later. I would like to know what use cases this don't cover (and then the solution is either expose more or fork.)

BrianHung · 2022-08-30T00:18:57Z

src/vanilla.ts

+    proxyCache.set(initialObject, proxyObject)
+    Reflect.ownKeys(initialObject).forEach((key) => {
+      const desc = Object.getOwnPropertyDescriptor(
+        initialObject,
+        key
+      ) as PropertyDescriptor
+      if (desc.get || desc.set) {
+        Object.defineProperty(baseObject, key, desc)
      } else {
-        nextValue = value
+        proxyObject[key as keyof T] = initialObject[key as keyof T]
      }
-      Reflect.set(target, prop, nextValue, receiver)
-      notifyUpdate(['set', [prop], value, prevValue])
-      return true
-    },
+    })


Could be simplified by copying of keys in Reflect.ownKeys(initialObject) into the default createProxy method, which also allows developer to decide which keys from initialObject to keep.

const proxyObject = customCreateProxy(initialObject, handler); proxyCache.set(initialObject, proxyObject); return proxyObject; const createProxy = <T extends object>( initialObject: T, handler: ProxyHandler<T> ): readonly [T, T] => { const baseObject = Array.isArray(initialObject) ? [] : Object.create(Object.getPrototypeOf(initialObject)) const proxyObject = new Proxy(baseObject, handler) Reflect.ownKeys(initialObject).forEach((key) => { const desc = Object.getOwnPropertyDescriptor( initialObject, key ) as PropertyDescriptor if (desc.get || desc.set) { Object.defineProperty(baseObject, key, desc) } else { proxyObject[key as keyof T] = initialObject[key as keyof T] } }) return proxyObject }

I'm not sure if I understand your comment.

I’m suggesting to move the for loop which copies keys from the initial object into the proxy object into the default createProxy function.

So developers can also fully customize which keys to copy if they decide not to use initialObject.

Ah, I probably see your point.
I have considered that, and for this time, I decided not to add it after the consideration.
If we got use cases require such capability, we consider it in a follow up PR.

(after all, it's the balance between customization and forking.)

Hm, maybe I can do that in this PR. Let me try.

BrianHung · 2022-08-30T00:23:15Z

src/vanilla.ts

+  customizeObjectIs: (fn: typeof Object.is) => typeof Object.is = identity,
+  customizeCanProxy: (fn: typeof canProxy) => typeof canProxy = identity,
+  customizeCreateSnapshot: (
+    fn: typeof createSnapshot
+  ) => typeof createSnapshot = identity,
+  customizeCreateProxy: (
+    fn: typeof createProxy
+  ) => typeof createProxy = identity
+) => {


Maybe change this to argument object to be less unwieldy:

const buildProxyFunction = ({ objectIs = Object.is, canProxy = canProxy, createSnapshot = createSnapshot, createProxy = createProxy, // default methods } : { objectIs: typeof Object.is, canProxy: typeof canProxy, createSnapshot: typeof createSnapshot, createProxy: typeof createProxy, // types }) => {

Then, you can't reuse the existing functions. One point of this style is to avoid exporting other functions (and symbols).

I think exporting the internals would be better, since to reuse existing functions you already need to know how they work.

Also since custom proxy is marked as unstable, developers could also understand that applies to exported internals too.

Do you mean to export many unstable functions? Anyway, if this is just a preference in styles rather than capability, I'd go with this for this time.

Yes. Yeah it’s a preference in style only.

BrianHung

For more context, here's my fork of valtio that I made to work with immer:
https://gist.github.com/BrianHung/ec6c0ce729acfa0361334c128eeef7ef

Noticeable change is that by forking valtio, I can write back to the initial object and access it as well. I found keeping two separate objects, the valtio proxy object and the initial / target object, easier to work with especially when it comes to nesting.

dai-shi · 2022-08-30T00:54:31Z

I can write back to the initial object and access it as well.

This PR should allow such use cases too.

customizeCreateProxy = (origCreateProxy) => {
  const createProxy = (initialObject, handler) => {
    return [initialObject, new Proxy(initialObject, handler)]
  }
  return createProxy
}

I didn't try it though.

dai-shi · 2022-08-30T06:38:13Z

9bc48a3 is to expose more functions. I'm not sure if it's worth the bundle size increase.
1% of users would use custom objectIs, 0.1% of users would use custom canProxy, 0.01% of users would use custom createSnapshot, and the last one is only for you (and I'm not even sure if you would use it.)
In this approach, I think three symbols are actually good. (otherwise, we would build all four functions.)

dai-shi · 2022-08-30T10:21:03Z

otherwise, we would build all four functions.

Hmm, let me try this.

dai-shi · 2022-08-31T01:46:38Z

Hmm, this probably doesn't solve your use case...

BrianHung · 2022-08-31T15:53:55Z

https://gist.github.com/BrianHung/ec6c0ce729acfa0361334c128eeef7ef

My overall use case and how it differs from valtio currently is:

custom snapshot method
custom proxy handler get, deleteProperty, set
access to LISTENERS and SNAPSHOT to build those custom methods

I can also try an having an implementation as well, by end of this week.

BrianHung · 2022-08-31T16:02:17Z

src/vanilla.ts

+  PROMISE_RESULT = __DEV__ ? Symbol('PROMISE_RESULT') : Symbol(),
+  PROMISE_ERROR = __DEV__ ? Symbol('PROMISE_ERROR') : Symbol(),


Should these be moved to shared state and be next to VERSION, LISTENERS, SNAPSHOT.

Is it also possible to pass VERSION, LISTENERS, SNAPSHOT into buildProxyFunction so that a custom createSnapshot and custom proxyFunction could access these methods, but they're not directly exported?

PROMISE_* symbols are not shared state. You can implement createSnapshot and proxyFunction without them, even just with properties. (Eventually, I will revisit this in the future, because React might be providing a new function for this. So, things will change. These are very internal.)

to pass VERSION, LISTENERS, SNAPSHOT into buildProxyFunction

These are shared state and you would never want to change. (If you want, you still can.)
Passing them to the function doesn't allow you to read them, right? Unless we go back to customizeFoo style, which is very complicated.
The trick is you would need to call buildProxyFunction function twice.
Honestly, I wish to have a better solution but didn't come up with ideas (that fulfill my requirements.)

dai-shi · 2022-08-31T22:57:27Z

custom snapshot method

Pass your own createSnapshot.

custom proxy handler get, deleteProperty, set

Pass your own newProxy.

access to LISTENERS and SNAPSHOT to build those custom methods

Run buildProxyFunction to get them.

so that a custom createSnapshot and custom proxyFunction could access these methods

I might be misunderstanding something in my approach. If you have some ideas, please feel free to share code snippets.

BrianHung · 2022-09-01T06:45:31Z

Run buildProxyFunction to get them.
The trick is you would need to call buildProxyFunction function twice.

Oh my confusion was over this since I didn't realize you could access LISTENERS and SNAPSHOTS, just with a second initialization. 😅

dai-shi · 2022-09-01T07:34:28Z

Please check out some examples in the PR description, if you haven't.

dai-shi · 2022-09-05T02:34:17Z

I agree it's confusing, but no other ideas at this moment.
It should at least cover the three use cases listed in the description.
Let's ship it. We may expect some refactors in the future anyway.

feat(vanilla): unstable_customize instead of unstable_getHandler

3655712

unstable_buildProxyFunction

95c7238

vercel bot deployed to Preview August 18, 2022 11:59 View deployment

dai-shi changed the title ~~feat(vanilla): unstable_customize instead of unstable_getHandler~~ feat(vanilla): unstable_buildProxyFunction instead of unstable_getHandler Aug 18, 2022

rename to avoid confusion

1d10eeb

dai-shi mentioned this pull request Aug 18, 2022

custom snapshot method #522

Closed

vercel bot deployed to Preview August 18, 2022 13:51 View deployment

fix unstable prefix

873a060

vercel bot deployed to Preview August 18, 2022 15:48 View deployment

Merge branch 'main' into feat/customizable-proxy

dbb8ef3

vercel bot deployed to Preview August 18, 2022 22:39 View deployment

update docs

726186c

dai-shi marked this pull request as ready for review August 29, 2022 05:05

vercel bot deployed to Preview August 29, 2022 05:06 View deployment

dai-shi mentioned this pull request Aug 29, 2022

fix(utils): fix proxyWithHistory behavibor with nested objects #531

Merged

BrianHung reviewed Aug 30, 2022

View reviewed changes

dai-shi mentioned this pull request Aug 30, 2022

valtio should avoid state change for compare equal value #511

Closed

expose full features

9bc48a3

vercel bot deployed to Preview August 30, 2022 06:29 View deployment

fix snapshotCache

b385863

vercel bot deployed to Preview August 30, 2022 10:22 View deployment

simplify exposing internal function

823c07e

dai-shi changed the title ~~feat(vanilla): unstable_buildProxyFunction instead of unstable_getHandler~~ feat(vanilla): unstable_buildFunctions instead of unstable_getHandler Aug 30, 2022

rever the wrong change in #533

82317b8

vercel bot deployed to Preview August 30, 2022 22:42 View deployment

reorder functions

4b6e00c

vercel bot deployed to Preview August 30, 2022 22:55 View deployment

expose newProxy

4c69ace

vercel bot deployed to Preview August 30, 2022 23:06 View deployment

dai-shi added 2 commits August 31, 2022 08:19

revert readme

8da7b44

go back to buildProxyFunction

293fc58

dai-shi changed the title ~~feat(vanilla): unstable_buildFunctions instead of unstable_getHandler~~ feat(vanilla): unstable_buildProxyFunction instead of unstable_getHandler Aug 30, 2022

dai-shi requested a review from BrianHung August 30, 2022 23:31

vercel bot deployed to Preview August 30, 2022 23:31 View deployment

fix docs

f8bc84a

vercel bot deployed to Preview August 30, 2022 23:45 View deployment

small renaming a param

e5eacb0

vercel bot deployed to Preview August 30, 2022 23:48 View deployment

make shared state global

e2bc55b

vercel bot deployed to Preview August 31, 2022 02:53 View deployment

directly access shared state

3dd8dbf

vercel bot deployed to Preview August 31, 2022 03:00 View deployment

BrianHung reviewed Aug 31, 2022

View reviewed changes

dai-shi merged commit dfd0c9b into main Sep 5, 2022

dai-shi deleted the feat/customizable-proxy branch September 5, 2022 02:35

dai-shi mentioned this pull request Dec 20, 2022

feat: lazy observables #604

Closed

1 task

dai-shi mentioned this pull request Aug 31, 2024

Validation #297

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(vanilla): unstable_buildProxyFunction instead of unstable_getHandler #528

feat(vanilla): unstable_buildProxyFunction instead of unstable_getHandler #528

dai-shi commented Aug 18, 2022 •

edited

Loading

vercel bot commented Aug 18, 2022 •

edited

Loading

codesandbox-ci bot commented Aug 18, 2022 •

edited

Loading

github-actions bot commented Aug 18, 2022 •

edited

Loading

BrianHung Aug 29, 2022

dai-shi Aug 30, 2022

BrianHung Aug 30, 2022

BrianHung Aug 29, 2022

dai-shi Aug 30, 2022

BrianHung Aug 30, 2022

dai-shi Aug 30, 2022

BrianHung Aug 30, 2022

dai-shi Aug 30, 2022

BrianHung Aug 30, 2022

dai-shi Aug 30, 2022

dai-shi Aug 30, 2022

BrianHung Aug 30, 2022

dai-shi Aug 30, 2022

BrianHung Aug 30, 2022

dai-shi Aug 30, 2022

BrianHung Aug 30, 2022

BrianHung left a comment •

edited

Loading

dai-shi commented Aug 30, 2022

dai-shi commented Aug 30, 2022

dai-shi commented Aug 30, 2022

dai-shi commented Aug 31, 2022

BrianHung commented Aug 31, 2022 •

edited

Loading

BrianHung Aug 31, 2022

BrianHung Aug 31, 2022

dai-shi Aug 31, 2022

dai-shi commented Aug 31, 2022

BrianHung commented Sep 1, 2022

dai-shi commented Sep 1, 2022

dai-shi commented Sep 5, 2022

		PROMISE_RESULT = __DEV__ ? Symbol('PROMISE_RESULT') : Symbol(),
		PROMISE_ERROR = __DEV__ ? Symbol('PROMISE_ERROR') : Symbol(),

feat(vanilla): unstable_buildProxyFunction instead of unstable_getHandler #528

feat(vanilla): unstable_buildProxyFunction instead of unstable_getHandler #528

Conversation

dai-shi commented Aug 18, 2022 • edited Loading

Some use cases

Use deepEqual instead of Object.is

Attach get trap to proxy handler

Avoid some objects to proxy instead of using ref for all

vercel bot commented Aug 18, 2022 • edited Loading

codesandbox-ci bot commented Aug 18, 2022 • edited Loading

github-actions bot commented Aug 18, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BrianHung left a comment • edited Loading

Choose a reason for hiding this comment

dai-shi commented Aug 30, 2022

dai-shi commented Aug 30, 2022

dai-shi commented Aug 30, 2022

dai-shi commented Aug 31, 2022

BrianHung commented Aug 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dai-shi commented Aug 31, 2022

BrianHung commented Sep 1, 2022

dai-shi commented Sep 1, 2022

dai-shi commented Sep 5, 2022

dai-shi commented Aug 18, 2022 •

edited

Loading

Attach `get` trap to proxy handler

Avoid some objects to proxy instead of using `ref` for all

vercel bot commented Aug 18, 2022 •

edited

Loading

codesandbox-ci bot commented Aug 18, 2022 •

edited

Loading

github-actions bot commented Aug 18, 2022 •

edited

Loading

BrianHung left a comment •

edited

Loading

BrianHung commented Aug 31, 2022 •

edited

Loading