From a5cf72cd5781c380cd3a913f6b7577ce7d2a1c07 Mon Sep 17 00:00:00 2001
From: Aria Minaei <aria.minaei@gmail.com>
Date: Sun, 10 Apr 2022 20:36:10 +0200
Subject: [PATCH] Add comments to `cloneDeepSerializable()`

https://github.com/theatre-js/theatre/pull/118#discussion_r846635429
---
 theatre/core/src/propTypes/index.ts            |  5 +++++
 .../StudioStore/createTransactionPrivateApi.ts | 18 +++++++++++++-----
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/theatre/core/src/propTypes/index.ts b/theatre/core/src/propTypes/index.ts
index dd59f22..c7b4389 100644
--- a/theatre/core/src/propTypes/index.ts
+++ b/theatre/core/src/propTypes/index.ts
@@ -609,6 +609,11 @@ export interface IBasePropType<
    * Each prop config has a `deserializeAndSanitize()` function that deserializes and sanitizes
    * any js value into one that is acceptable by this prop config, or `undefined`.
    *
+   * As a rule, the value returned by this function should not hold any reference to `json` or any
+   * other value referenced by the descendent props of `json`. This is to ensure that json values
+   * controlled by the user can never change the values in the store. See `deserializeAndSanitize()` in
+   * `t.compound()` or `t.rgba()` as examples.
+   *
    * The `DeserializeType` is usually equal to `ValueType`. That is the case with
    * all simple prop configs, such as `number`, `string`, or `rgba`. However, composite
    * configs such as `compound` or `enum` may deserialize+sanitize into a partial value. For example,
diff --git a/theatre/studio/src/StudioStore/createTransactionPrivateApi.ts b/theatre/studio/src/StudioStore/createTransactionPrivateApi.ts
index 1f4ecd5..6df778a 100644
--- a/theatre/studio/src/StudioStore/createTransactionPrivateApi.ts
+++ b/theatre/studio/src/StudioStore/createTransactionPrivateApi.ts
@@ -17,7 +17,15 @@ import {getPropConfigByPath} from '@theatre/shared/propTypes/utils'
 import {isPlainObject} from 'lodash-es'
 import userReadableTypeOfValue from '@theatre/shared/utils/userReadableTypeOfValue'
 
-function cloneDeepSerializable<T>(v: T): T | undefined {
+/**
+ * Deep-clones a plain JS object or a `string | number | boolean`. In case of a plain
+ * object, all its sub-props that aren't `string | number | boolean` get pruned. Also,
+ * all empty objects (i.e. `{}`) get pruned.
+ *
+ * This is only used by {@link ITransactionPrivateApi.set} and it follows the global rule
+ * that values pointed to by `object.props[...]` are never `null | undefined` or an empty object.
+ */
+function cloneDeepSerializableAndPrune<T>(v: T): T | undefined {
   if (
     typeof v === 'boolean' ||
     typeof v === 'string' ||
@@ -28,8 +36,8 @@ function cloneDeepSerializable<T>(v: T): T | undefined {
     const cloned: $IntentionalAny = {}
     let clonedAtLeastOneProp = false
     for (const [key, val] of Object.entries(v)) {
-      const clonedVal = cloneDeepSerializable(val)
-      if (typeof clonedVal !== 'undefined') {
+      const clonedVal = cloneDeepSerializableAndPrune(val)
+      if (clonedVal !== undefined) {
         cloned[key] = val
         clonedAtLeastOneProp = true
       }
@@ -69,7 +77,7 @@ export default function createTransactionPrivateApi(
   return {
     set: (pointer, value) => {
       ensureRunning()
-      const _value = cloneDeepSerializable(value)
+      const _value = cloneDeepSerializableAndPrune(value)
       if (typeof _value === 'undefined') return
 
       const {root, path} = getPointerParts(pointer as Pointer<$FixMe>)
@@ -104,7 +112,7 @@ export default function createTransactionPrivateApi(
             return
           }
 
-          const deserialized = cloneDeepSerializable(
+          const deserialized = cloneDeepSerializableAndPrune(
             propConfig.deserializeAndSanitize(value),
           )
           if (deserialized === undefined) {