Fix #2871 toJS throws with configure({ useProxies: "ifavailable" }) #2873
Conversation
🦋 Changeset detectedLatest commit: 97523ba The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| const res = cache(__alreadySeen, source, {}) | ||
| getPlainObjectKeys(source).forEach((key: any) => { | ||
| res[key] = toJSHelper(source[key], __alreadySeen) | ||
| ;((source as any) as IIsObservableObject)[$mobx].ownKeys_().forEach((key: any) => { |
There was a problem hiding this comment.
I think the fix here is incorrect; we should use keys(source) as basis for the loop, and not use getPlainObjectKeys, to make sure that toJS will track key additions, even when it is used in ES5 mode. There are probably subtle differences between keys and getPlainObjectKeys that might need to be addressed, but we should use keys as basis, or keep it as implemented here, but make sure a read to the keys atom is registered.
There was a problem hiding this comment.
adm.ownKeys_ handles the subscription to keys atom the same way as keys(object). We simply don't have ownKeys_ exposed via object-api so I call it directly on administration.
There was a problem hiding this comment.
Just to clarify further, there is no longer any interesting logic (like keyAtom subscriptions) in object-api or in proxy handler. They simply delegate to adm and everything is done there.
There was a problem hiding this comment.
Leaving the merging up to you, in case you want to merge the open PR's in a specific order :)
Fixes: #2871 in BC manner.
We may want to reconsider which kind fo keys
toJSreturns.