oby: Infinite recursion with arrays
I’ve been running into a lot of difficult-to-debug issues around infinite loops involving arrays and circular references. Here’s a sandbox with a sample of what I’m talking about:
https://codesandbox.io/s/voby-demo-store-counter-forked-r225g4?file=/src/index.tsx
(click the +
a couple times and the app will hang)
This has been a bit difficult in my app because Solid handles these circular references in arrays just fine and I have data of a similar shape cropping up in numerous places in my app. So this looks to me like a bug, but what do you think? Are these cases something that voby/oby should handle? Or does there just need to be a big warning sign in the readme?
About this issue
- Original URL
- State: closed
- Created 9 months ago
- Comments: 19 (19 by maintainers)
I am currently still using it. The batching/tick behavior is pretty essential to the current iteration of my app. Though I’ve recently had some insights that might let me take over the tick behavior that would let me use alternate renderers. But I’m not in a rush to switch.
If I throw inside the effect I see the error in the console. Maybe you had an error boundary or something?
What you’ve outlined seems pretty reasonable. I think a big warning in the readme/docs would be very handy. This issue has cost me a lot of time because the nature of the bug is so weird. It’s seemingly recursive but it never blows the stack. And I think there might be something in the oby code swallowing errors because when I was playing around with it locally, I threw errors from the oby code when the it went through too many iterations of
get
from the store and the recursion would stop but I’d never see the errors in the console. So that might also be something to hunt down now that there is a smallish repro. I may have had something else in the app catching those errors. I’m not sure.I totally get that. Solid’s stores have bitten me repeatedly. Especially as they’ve changed several times.
That makes sense.
I was only thinking about this for when an array gets set on a store field. Any other unwrapping would be constant-time. But I agree that it is surprising and could lead users to think that the unwrapping is going to be deep when it isn’t. At least with no unwrapping of array items (status quo), it’s very clear that there’s no unwrapping beyond what you see.
Thanks for all your time on this. Oby’s batching is chef’s kiss by the way.
batch
andtick
are a great combo.Regarding the 1 level unwrapping: depending on how you look at it it feels like a worst of both worlds, or a best of both worlds, compromise. From my point of view unproxying at 1 level is both potentially way slower than the constant time unproxying, and also doesn’t solve the problem in general. From another point of view I guess it could make more code work out of the box with no user-level changes. I don’t really like it as a solution.
I guess that’s a little surprising, and there should be warning signs, but I guess they should be on MDN or something? Like a proxied object is not triple-equal to the unproxied object. structuredClone throws for proxies. postMessage also throws. It’s not Oby’s place to document this stuff probably, though some warnings can be added to the readme if they are useful of course.
Yes it already unproxies set values, but not deeply. In your code the proxy is found “deep” inside the array, so that doesn’t get unproxied.
Yeah that’s another problem with deep unwrapping, especially if done by default. You deep unwrap twice and the two objects don’t match one another. That’s how Solid works by the way.
Losing reactivity how? This option should JustWork™️, it “just” requires some understanding of this problem and manual error-prone code that unwraps values being set.
I’m not sure what you mean, it seems orthogonal to the problem at hand 🤔 array methods are already proxied.