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

test: add cyclical class test #794

Closed
wants to merge 1 commit into from
Closed

Conversation

Gregoor
Copy link

@Gregoor Gregoor commented Oct 3, 2023

Hi there,

thanks for the great library!

I've got a case of mildly cursed cyclical classes, that I don't think I can data model my way around. valtio seems like a great solution to make it work, but I seem to have run into a proxy-ing bug which I've been able to repro here (the test case succeeds without the proxy()-wrap).

Now the docs do say

Not everything can be proxied. Generally, you are safe if it is serializable. Classes can also be proxied. But avoid special objects.

And it's not serializable and it's definitely special (the bad kind of special), but as I said, it also seems necessary for my use case.

So here's my question: would fixing this be possible and in-scope? If so I'd also be happy to see if I can figure it out on my own, though guidance would certainly help.

Cheers!

@vercel
Copy link

vercel bot commented Oct 3, 2023

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 Oct 3, 2023 8:31pm

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 3, 2023

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 46c0879:

Sandbox Source
React Configuration
React TypeScript Configuration
React Browserify Configuration
React Snowpack Configuration
React Parcel Configuration

@dai-shi
Copy link
Member

dai-shi commented Oct 4, 2023

Thanks for opening up a discussion.

would fixing this be possible and in-scope?

One thing we can't change is we kind of support classes by not touching prototype chain.
So, leaving prototype untouched is the design decision.
tbh, I'm not sure if it's fixable under this constraint.

If so I'd also be happy to see if I can figure it out on my own, though guidance would certainly help.

I'd encourage you to dig a little deeper. I don't yet know why it doesn't work. We do support cyclical objects, which is a unique feature to my knowledge.

@Gregoor
Copy link
Author

Gregoor commented Oct 9, 2023

I did not manage to figure it out and in the scramble I managed to unscramble my data model. So I'll close this PR. Thanks for the words and the lib! Also for Jotai, which I'm going back to now, since it indeed was the only solid lib I found that does cyclical objects.

@Gregoor Gregoor closed this Oct 9, 2023
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.

2 participants