Skip to content

Commit

Permalink
fix: object merging with mutation in VFolderTable, hooks, and Session…
Browse files Browse the repository at this point in the history
…LauncherPage (#2699)

### TL;DR

Modified `_.merge()` calls to include an empty object as the first argument.

This PR fixed a bug where the user could not turn on the "Use automatic OpenMP optimization" option after turning it off in the Neo Session Launcher.

![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/XqC2uNFuj0wg8I60sMUh/935c7236-ab00-49a2-a83a-469048b0c31d.png)

### What changed?

Updated three instances of `_.merge()` function calls in different files:
1. In `VFolderTable.tsx`, added an empty object as the first argument to `mergedVFolderPermissions`.
2. In `backendai.tsx`, added an empty object as the first argument when merging `deviceMetadata` and `resourceSlots`.
3. In `SessionLauncherPage.tsx`, added an empty object as the first argument when merging `INITIAL_FORM_VALUES` and `formValuesFromQueryParams`.

### Why make this change?

The addition of an empty object as the first argument in `_.merge()` calls ensures that the original objects are not modified during the merge operation. This prevents potential side effects and maintains the immutability of the source objects, leading to more predictable and safer code behavior.
  • Loading branch information
yomybaby committed Sep 10, 2024
1 parent 7b8aaac commit 9d0dd72
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 2 deletions.
1 change: 1 addition & 0 deletions react/src/components/VFolderTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ const VFolderTable: React.FC<VFolderTableProps> = ({
);

const mergedVFolderPermissions = _.merge(
{}, // start with empty object
allowedVFolderHostsByDomain,
allowedVFolderHostsByGroup,
allowedVFolderHostsByKeypairResourcePolicy,
Expand Down
2 changes: 1 addition & 1 deletion react/src/hooks/backendai.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export const useResourceSlotsDetails = (resourceGroupName?: string) => {
});

return [
_.merge(deviceMetadata, resourceSlots),
_.merge({}, deviceMetadata, resourceSlots),
{
refresh: () => checkUpdate(),
},
Expand Down
2 changes: 1 addition & 1 deletion react/src/pages/SessionLauncherPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ const SessionLauncherPage = () => {
}, []);

const mergedInitialValues = useMemo(() => {
return _.merge(INITIAL_FORM_VALUES, formValuesFromQueryParams);
return _.merge({}, INITIAL_FORM_VALUES, formValuesFromQueryParams);
}, [INITIAL_FORM_VALUES, formValuesFromQueryParams]);

// ScrollTo top when step is changed
Expand Down

0 comments on commit 9d0dd72

Please sign in to comment.