Fix live draft key-path updates

Fix #939

Author: kyoshino

src/lib/components/contents/details/fields/object/object-editor.svelte

@@ -166,8 +166,10 @@
     forEachTargetLocale(
       { valueStore: $entryDraft?.[valueStoreKey], locale, i18n },
       (_valueMap, _locale) => {
-        // Assign `null` before deleting each property, so the draft proxy can revalidate the field
-        getKeysByPrefix(_valueMap, `${keyPath}.`).forEach((_keyPath) => {
+        // Assign `null` before deleting each property, so the draft proxy can revalidate the field.
+        // The value map is the draft’s live map, which is mutated right below, so its key paths
+        // have to be read as they are right now
+        getKeysByPrefix(_valueMap, `${keyPath}.`, { live: true }).forEach((_keyPath) => {
           /** @type {EntryDraft} */ ($entryDraft)[valueStoreKey][_locale][_keyPath] = null;
           delete $entryDraft?.[valueStoreKey][_locale][_keyPath];
         });

src/lib/services/contents/draft/update/list.js

@@ -40,6 +40,9 @@ const itemListRegexCache = new Map();
 
 /**
  * Traverse the given object by decoding dot-notated key path.
+ *
+ * The object is a draft’s live content, not a snapshot of it, and the caller writes the manipulated
+ * list straight back into it, so the key paths have to be read as they are right now.
  * @internal
  * @param {Record<string, any>} obj Original object.
  * @param {FieldKeyPath} keyPath Dot-notated field name.
@@ -53,7 +56,7 @@ export const getItemList = (obj, keyPath) => {
   );
 
   return [
-    getSubtree(obj, keyPath) ?? [],
+    getSubtree(obj, keyPath, { live: true }) ?? [],
     Object.fromEntries(Object.entries(obj).filter(([k]) => !regex.test(k))),
   ];
 };
@@ -106,12 +109,13 @@ export const updateListField = ({
 
 /**
  * Read a multi-value field out of the draft as a plain list. Our internal representation of such a
- * field is a flattened object with one numbered key per item, e.g. `images.0`, `images.1`.
+ * field is a flattened object with one numbered key per item, e.g. `images.0`, `images.1`. Just
+ * like {@link getItemList}, this reads a draft’s live content, which the caller then mutates.
  * @param {Record<string, any>} values Flattened entry content.
  * @param {FieldKeyPath} keyPath Dot-notated field name.
  * @returns {any[]} Item values in list order.
  */
-const getMultiValueList = (values, keyPath) => getSubtree(values, keyPath) ?? [];
+const getMultiValueList = (values, keyPath) => getSubtree(values, keyPath, { live: true }) ?? [];
 
 /**
  * Move an item of a multi-value field, such as a File or Image field with the `multiple` option

src/lib/services/contents/draft/update/list.test.js

@@ -13,6 +13,10 @@
  * once instead, keyed by the map object itself: the draft hands out a fresh snapshot per update, so
  * an index is built at most once per keystroke and shared by every field reading from it, then
  * garbage-collected with the snapshot.
+ *
+ * An index is therefore only valid while the map it was built from is left alone. A caller reading
+ * a map it also mutates — a draft’s live content rather than a snapshot of it — has to pass the
+ * `live` option, or it would keep getting the key paths the map had the first time it was read.
  */
 
 /**
@@ -51,9 +55,16 @@ const getSortedKeys = (valueMap) => {
  * the “first” field under a key path.
  * @param {FlattenedEntryContent} valueMap Flattened entry content.
  * @param {string} prefix Key path prefix, e.g. `authors.0.`.
+ * @param {object} [options] Options.
+ * @param {boolean} [options.live] Whether the value map may be mutated after this call, in which
+ * case the key paths are scanned instead of being read from — and memoized in — the index.
  * @returns {FieldKeyPath[]} Matching key paths.
  */
-export const getKeysByPrefix = (valueMap, prefix) => {
+export const getKeysByPrefix = (valueMap, prefix, { live = false } = {}) => {
+  if (live) {
+    return Object.keys(valueMap).filter((key) => key.startsWith(prefix));
+  }
+
   const sorted = getSortedKeys(valueMap);
   let low = 0;
   let high = sorted.length;
@@ -98,9 +109,18 @@ const listItemKeyCacheMap = new WeakMap();
  * multiple Relation or Select field — so the same index serves all of them.
  * @param {FlattenedEntryContent} valueMap Flattened entry content.
  * @param {FieldKeyPath} keyPath Key path of the list field.
+ * @param {object} [options] Options.
+ * @param {boolean} [options.live] Whether the value map may be mutated after this call, in which
+ * case the key paths are scanned instead of being read from — and memoized in — the index.
  * @returns {FieldKeyPath[]} Item key paths, e.g. `['authors.0', 'authors.1']`.
  */
-export const getListItemKeys = (valueMap, keyPath) => {
+export const getListItemKeys = (valueMap, keyPath, { live = false } = {}) => {
+  if (live) {
+    return Object.keys(valueMap).filter(
+      (key) => key.match(LIST_ITEM_KEY_REGEX)?.groups?.parent === keyPath,
+    );
+  }
+
   let index = listItemKeyCacheMap.get(valueMap);
 
   if (!index) {

src/lib/services/contents/entry/key-paths.js

@@ -43,10 +43,13 @@ export const getSubtreeEntries = (keyPath, value) => ({
  * Assemble the non-primitive value stored under the given key path from its child key paths.
  * @param {FlattenedEntryContent} valueMap Flattened entry content.
  * @param {FieldKeyPath} keyPath Key path of the field.
+ * @param {object} [options] Options.
+ * @param {boolean} [options.live] Whether the value map may be mutated after this call. See
+ * {@link getKeysByPrefix}.
  * @returns {any} Assembled value, or `undefined` if the key path has no children.
  */
-export const getSubtree = (valueMap, keyPath) => {
-  const keys = getKeysByPrefix(valueMap, `${keyPath}.`);
+export const getSubtree = (valueMap, keyPath, { live = false } = {}) => {
+  const keys = getKeysByPrefix(valueMap, `${keyPath}.`, { live });
 
   if (!keys.length) {
     return undefined;

src/lib/services/contents/entry/key-paths.test.js

@@ -23,7 +23,12 @@ export const getPairs = ({ entryDraft, valueStoreKey = 'currentValues', keyPath,
   const prefix = `${keyPath}.`;
 
   return /** @type {[string, string][]} */ (
-    getKeysByPrefix(valueMap, prefix).map((key) => [key.slice(prefix.length), valueMap[key]])
+    // The value map is the draft’s live map, which {@link savePairs} mutates in place, so its key
+    // paths have to be read as they are right now
+    getKeysByPrefix(valueMap, prefix, { live: true }).map((key) => [
+      key.slice(prefix.length),
+      valueMap[key],
+    ])
   );
 };
 
@@ -74,7 +79,9 @@ export const savePairs = ({
         // stores no placeholder at its own key path: its keys are arbitrary strings, and
         // `unflatten()` would turn numeric ones into an array. `finalizeContent()` rebuilds the
         // object from the children instead
-        getKeysByPrefix(content, `${keyPath}.`).forEach((_keyPath) => {
+        // The content is the draft’s live map, which is mutated right below, so its key paths
+        // have to be read as they are right now
+        getKeysByPrefix(content, `${keyPath}.`, { live: true }).forEach((_keyPath) => {
           delete content[_keyPath];
         });