#8636: extract getFullVariableName

src/components/fields/schemaFields/widgets/varPopup/VarPopup.tsx

@@ -17,10 +17,12 @@
 
 import React, { useCallback, useEffect } from "react";
 import { type FieldInputMode } from "@/components/fields/schemaFields/fieldInputMode";
-import { replaceLikelyVariable } from "./likelyVariableUtils";
+import {
+  getFullVariableName,
+  replaceLikelyVariable,
+} from "./likelyVariableUtils";
 import VarMenu from "./VarMenu";
 import fitTextarea from "fit-textarea";
-import { getPathFromArray } from "@/runtime/pathHelpers";
 import useAttachPopup from "@/components/fields/schemaFields/widgets/varPopup/useAttachPopup";
 import reportEvent from "@/telemetry/reportEvent";
 import { Events } from "@/telemetry/events";
@@ -71,8 +73,10 @@ const VarPopup: React.FunctionComponent<VarPopupProps> = ({
     (selectedPath: string[]) => {
       reportEvent(Events.VAR_POPOVER_SELECT);
 
-      // FIXME: if the user is editing with optional chaining, those will be removed
-      const fullVariableName = getPathFromArray(selectedPath);
+      const fullVariableName = getFullVariableName(
+        likelyVariable,
+        selectedPath,
+      );
 
       switch (inputMode) {
         case "var": {
@@ -118,7 +122,7 @@ const VarPopup: React.FunctionComponent<VarPopupProps> = ({
 
       hideMenu();
     },
-    [hideMenu, inputElementRef, inputMode, setValue, value],
+    [hideMenu, inputElementRef, inputMode, setValue, value, likelyVariable],
   );
 
   if (!isMenuShowing) {

src/components/fields/schemaFields/widgets/varPopup/likelyVariableUtils.test.ts

@@ -16,6 +16,7 @@
  */
 
 import {
+  getFullVariableName,
   getLikelyVariableAtPosition,
   replaceLikelyVariable,
 } from "./likelyVariableUtils";
@@ -314,3 +315,18 @@ describe("replaceLikelyVariable", () => {
     expect(newCursorPosition).toEqual(endOfVariableIndex);
   });
 });
+
+describe("getFullVariableName", () => {
+  it("preserves optional chaining", () => {
+    expect(getFullVariableName("@foo", ["@foo", "bar"])).toBe("@foo.bar");
+    expect(getFullVariableName("@foo?", ["@foo", "bar"])).toBe("@foo?.bar");
+    expect(getFullVariableName("@foo?.bar?", ["@foo", "bar", "baz"])).toBe(
+      "@foo?.bar?.baz",
+    );
+  });
+
+  it("handles array", () => {
+    // TODO: handle optional chaining with arrays
+    // expect(getFullVariableName("@foo.bar[42]?", ["@foo", "bar", "42", "qux"])).toBe("@foo.bar[42]?.qux");
+  });
+});

src/components/fields/schemaFields/widgets/varPopup/likelyVariableUtils.ts

@@ -15,6 +15,9 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+import { toPath } from "lodash";
+import { getPathFromArray } from "@/runtime/pathHelpers";
+
 const varRegex = /(?<varName>@(\.|\w|(\[\d+])|(\[("|')[\s\w]+("|')]))*)/g;
 
 type LikelyVariable = {
@@ -189,3 +192,28 @@ export function replaceLikelyVariable(
     newCursorPosition: endOfVariableIndex,
   };
 }
+
+/**
+ * Select the full variable name based on the selected path and user's expression so far.
+ */
+export function getFullVariableName(
+  likelyVariable: string,
+  selectedPath: string[],
+): string {
+  const originalPath = toPath(likelyVariable);
+
+  return getPathFromArray(
+    selectedPath.map((part, index) => {
+      // Preserve optional chaining from what the use has typed so far
+      // eslint-disable-next-line security/detect-object-injection -- numeric index
+      if (
+        originalPath[index]?.endsWith("?") &&
+        index !== selectedPath.length - 1
+      ) {
+        return part + "?";
+      }
+
+      return part;
+    }),
+  );
+}

src/components/fields/schemaFields/widgets/varPopup/menuFilters.ts

@@ -39,9 +39,9 @@ function isTextOrNull(value: string | null): boolean {
 }
 
 /**
- * Convert a variable to a normalized path, with support for optional chaining.
+ * Convert a variable to a normalized variable path, removing optional chaining.
  */
-function toNormalizedPath(value: string): string[] {
+function toVarPath(value: string): string[] {
   return toPath(value.replaceAll("?.", "."));
 }
 
@@ -72,7 +72,7 @@ export function filterOptionsByVariable(
     return options;
   }
 
-  const [base, ...rest] = toNormalizedPath(likelyVariable);
+  const [base, ...rest] = toVarPath(likelyVariable);
   return options.filter(([source, vars]) => {
     if (rest.length === 0) {
       return Object.keys(vars).some((x) => x.startsWith(base));
@@ -128,7 +128,7 @@ export function filterVarMapByVariable(
     return varMap;
   }
 
-  return filterVarMapByPath(varMap, toNormalizedPath(likelyVariable));
+  return filterVarMapByPath(varMap, toVarPath(likelyVariable));
 }
 
 /**
@@ -146,7 +146,7 @@ export function expandCurrentVariableLevel(
 
   // If likelyVariable ends with ".", there's a part for the empty string at the end of the path. So can just use
   // as normal without logic for trailing "."
-  const parts = toNormalizedPath(likelyVariable);
+  const parts = toVarPath(likelyVariable);
   return (keyPath, data, level) => {
     // Key path from JSONTree is in reverse order
     const reverseKeyPath = [...keyPath].reverse();
@@ -243,10 +243,7 @@ export function defaultMenuOption(
     return null;
   }
 
-  if (
-    isTextOrNull(likelyVariable) ||
-    toNormalizedPath(likelyVariable).length === 0
-  ) {
+  if (isTextOrNull(likelyVariable) || toVarPath(likelyVariable).length === 0) {
     // Must always have at least one option (e.g., the `@input`)
     // Prefer the last option, because that's the latest output
 
@@ -255,7 +252,7 @@ export function defaultMenuOption(
     return [first];
   }
 
-  const parts = toNormalizedPath(likelyVariable);
+  const parts = toVarPath(likelyVariable);
 
   const [head, ...rest] = parts;