packages/bruno-app/src/components/EnvironmentVariablesTable/index.js (4)

259-298: Stale closure risk with formik.values in useCallback dependency.

Using formik.values directly in the dependency array means the callback recreates on every value change, but there's still a subtle timing issue where formik.values inside the callback could be stale if the callback is invoked during a render cycle.

Consider using formik.setValues with a functional updater or accessing values via ref for more predictable behavior.

Alternative approach using functional update

  const handleRemoveVar = useCallback(
    (id) => {
-     const currentValues = formik.values;
-
-     if (!currentValues || currentValues.length === 0) {
-       return;
-     }
-
-     const lastRow = currentValues[currentValues.length - 1];
+     formik.setValues((currentValues) => {
+       if (!currentValues || currentValues.length === 0) {
+         return currentValues;
+       }
+
+       const lastRow = currentValues[currentValues.length - 1];
        // ... rest of logic, returning newValues
+     });
    },
-   [formik.values]
+   [formik]
  );

---

179-200: Missing formik in effect dependencies.

The effect calls formik.setValues but doesn't include formik in the dependency array. While formik is typically stable across renders, this could cause linter warnings.

- }, [environment.uid, hasDraftForThisEnv, draft?.variables]);
+ }, [environment.uid, hasDraftForThisEnv, draft?.variables, formik]);

---

236-257: Consider extracting ErrorMessage outside the component.

Defining ErrorMessage inside the component means it's recreated on every render. While functional, extracting it or memoizing it would improve performance slightly for large tables.

---

110-114: Minor: prefer const for _collection.

The variable is reassigned via property assignment (_collection.globalEnvironmentVariables = ...), but the binding itself doesn't change. Use const for consistency.

- let _collection = collection ? cloneDeep(collection) : {};
+ const _collection = collection ? cloneDeep(collection) : {};

packages/bruno-app/src/components/EditableTable/index.js (1)

28-36: Sync column widths when columns changes.
State is only initialized once; if columns are added/removed or re-keyed later, widths can drift or become stale. Consider syncing on columns updates while preserving existing widths.

♻️ Suggested sync on columns change

  const [columnWidths, setColumnWidths] = useState(() => {
    const initialWidths = {};
    columns.forEach((col) => {
      initialWidths[col.key] = col.width || 'auto';
    });
    return initialWidths;
  });

+  useEffect(() => {
+    setColumnWidths((prev) => {
+      const next = {};
+      columns.forEach((col) => {
+        next[col.key] = prev[col.key] ?? col.width ?? 'auto';
+      });
+      return next;
+    });
+  }, [columns]);

packages/bruno-app/src/components/EditableTable/index.js (1)

120-122: Add JSDoc for the resize helper.

This helper is a new abstraction and would benefit from a brief JSDoc.

As per coding guidelines: Add JSDoc comments to abstractions for additional details.

📝 Example JSDoc

+  /**
+   * Resolves a column width from resized state or column config.
+   * `@param` {{ key: string, width?: string }} column
+   * `@returns` {string}
+   */
   const getColumnWidth = useCallback((column) => {
     return columnWidths[column.key] || column.width || 'auto';
   }, [columnWidths]);

packages/bruno-app/src/components/EnvironmentVariablesTable/index.js (2)

222-243: Inner component ErrorMessage recreated on every render.

Defining a component inside the render body creates a new function reference each render, causing React to unmount/remount the component unnecessarily. Consider memoizing with useCallback or extracting it outside and passing formik as a prop.

♻️ Proposed fix using useCallback

- const ErrorMessage = ({ name, index }) => {
-   const meta = formik.getFieldMeta(name);
-   const id = `error-${name}-${index}`;
-
-   const isLastRow = index === formik.values.length - 1;
-   const variable = formik.values[index];
-   const isEmptyRow = !variable?.name || variable.name.trim() === '';
-
-   if (isLastRow && isEmptyRow) {
-     return null;
-   }
-
-   if (!meta.error || !meta.touched) {
-     return null;
-   }
-   return (
-     <span>
-       <IconAlertCircle id={id} className="text-red-600 cursor-pointer" size={20} />
-       <Tooltip className="tooltip-mod" anchorId={id} html={meta.error || ''} />
-     </span>
-   );
- };
+ const renderErrorMessage = useCallback((name, index) => {
+   const meta = formik.getFieldMeta(name);
+   const id = `error-${name}-${index}`;
+
+   const isLastRow = index === formik.values.length - 1;
+   const variable = formik.values[index];
+   const isEmptyRow = !variable?.name || variable.name.trim() === '';
+
+   if ((isLastRow && isEmptyRow) || !meta.error || !meta.touched) {
+     return null;
+   }
+   return (
+     <span>
+       <IconAlertCircle id={id} className="text-red-600 cursor-pointer" size={20} />
+       <Tooltip className="tooltip-mod" anchorId={id} html={meta.error || ''} />
+     </span>
+   );
+ }, [formik]);

Then in JSX: {renderErrorMessage(${index}.name, index)}

---

299-301: setTimeout(..., 0) workaround to defer state update.

Using setTimeout with 0ms to avoid updating during render works but is fragile. Consider using a ref to track pending additions and applying them in a useEffect:

♻️ Alternative approach

+ const pendingNewRowRef = useRef(false);
+
+ useEffect(() => {
+   if (pendingNewRowRef.current) {
+     pendingNewRowRef.current = false;
+     const newVariable = {
+       uid: uuid(),
+       name: '',
+       value: '',
+       type: 'text',
+       secret: false,
+       enabled: true
+     };
+     formik.setFieldValue(formik.values.length, newVariable, false);
+   }
+ }, [formik.values.length]);

  const handleNameChange = (index, e) => {
    formik.handleChange(e);
    const isLastRow = index === formik.values.length - 1;

    if (isLastRow) {
-     const newVariable = {
-       uid: uuid(),
-       name: '',
-       value: '',
-       type: 'text',
-       secret: false,
-       enabled: true
-     };
-     setTimeout(() => {
-       formik.setFieldValue(formik.values.length, newVariable, false);
-     }, 0);
+     pendingNewRowRef.current = true;
    }
  };