Clone object proxies in mapper/resource params #519
Conversation
bennostein
force-pushed
the
clone-proxied-params
branch
from
30496bd to
606ad7d
Compare
| return value; | ||
| if (typeof value == "object") { | ||
| if (value === null) return value; | ||
| if (isObjectProxy(value)) return value.clone() as T; |
There was a problem hiding this comment.
Why call clone here instead of in deepFreeze? Do we need a distinct clone for each time it gets checked? If it needs to be here, then check is confusingly misnamed.
There was a problem hiding this comment.
My understanding (@skiplabsdaniel is the authority) is that this is a distinct issue from deep-freezing: these proxy objects are frozen, but when they're used as parameters to mappers and resources, their skip-heap pointers can outlive the lifetime of the objects they point to. So it's not always necessary to clone when freezing: only at this point when the proxy is used as a parameter.
Agreed on the naming point, will change.
There was a problem hiding this comment.
Ok, but I don't understand. :-) Is it a pointer stored in the skip heap to a proxy object in the js heap that is the problem? Is the proxy in the js heap really just supposed to be a wrapper for a skip object? So would it make sense to have the skip heap hold a pointer the the proxied skip object instead?
It isn't clear how calling clone here will make a difference to the relative lifetimes. I worry that this is a bug waiting to happen since the clone call looks like a nop and someone could easily remove it later.
bennostein
force-pushed
the
clone-proxied-params
branch
from
606ad7d to
6f74207
Compare
(stacked on #519) Adds a runnable version of the getting started example and applies a handful of unrelated bug fixes and cleanups. Run as follows: ``` cd skipruntime-ts make bunbuild bun run examples/groups.ts & bun run examples/groups-server.ts & bun run examples/groups-client ```
3 participants
This was causing some issues in one of my examples, as pointers copied over through proxy objects were outliving the actual object in the skip heap. Thanks Daniel for the help debugging!