From f8b2ee8c0d03d15b257832f88650d7fbeb24ec93 Mon Sep 17 00:00:00 2001 From: LP B Date: Fri, 10 Jan 2025 18:20:44 +0100 Subject: [PATCH] fix(app/edge-stack): local filesystem path is not retained (#292) --- app/react/hooks/useDebounce.ts | 47 ++++++- .../RelativePathFieldset.tsx | 122 +++++++++--------- .../RelativePathFieldset/useEnableFsPath.ts | 22 ---- 3 files changed, 105 insertions(+), 86 deletions(-) delete mode 100644 app/react/portainer/gitops/RelativePathFieldset/useEnableFsPath.ts diff --git a/app/react/hooks/useDebounce.ts b/app/react/hooks/useDebounce.ts index 3e413417e..271b0158c 100644 --- a/app/react/hooks/useDebounce.ts +++ b/app/react/hooks/useDebounce.ts @@ -1,17 +1,56 @@ -import _ from 'lodash'; +import { debounce } from 'lodash'; import { useState, useRef, useCallback, useEffect } from 'react'; +// `useRef` to keep the debouncer function (result of the _.debounce call) between rerenders. +// +// debouncer func is (value, onChange) => { onChange(value) }; +// +// Previously written and used as +// const onChangeDebouncer = useRef(debounce(onChange, 300)); +// onChangeDebouncer.current(value) +// +// The issue with the previous syntax is that it was holding the initial state of the `onChange` function passed to `useDebounce()`. +// When the `onChange` function was using a dynamic context (vars in parent scope/not in its parameters) +// then invoking the debouncer was producing a result of `onChange` computed uppon the initial state of the function, not the current state. +// +// Example of the issue +// +// function Component({ value }: { value: string; }) { +// +// function onChange(v: string) { +// // This will always print the first value of the "value" prop + the updated value of "v" +// // when called from "handleChange". +// // This is an issue when the `onChange` is a prop of the component and the real function performs state mutations upflow based on +// // values that are in the parent component, as `setDebouncedValue` will only use the initial instance of the `onChange` prop, thus +// // the initial state of the parent component. +// console.log(value, v) +// } +// +// const [debouncedValue, setDebouncedValue] = useDebounce(value, onChange); +// +// function handleChange(newValue: string) { +// setDebouncedValue(newValue); +// } +// +// return ( handleChange(e.target.value)} />) +// } export function useDebounce(value: string, onChange: (value: string) => void) { const [debouncedValue, setDebouncedValue] = useState(value); - const onChangeDebounces = useRef(_.debounce(onChange, 300)); + // Do not change. See notes above + const onChangeDebouncer = useRef( + debounce( + (value: string, onChangeFunc: (v: string) => void) => onChangeFunc(value), + 300 + ) + ); const handleChange = useCallback( (value: string) => { setDebouncedValue(value); - onChangeDebounces.current(value); + onChangeDebouncer.current(value, onChange); }, - [onChangeDebounces, setDebouncedValue] + [onChangeDebouncer, setDebouncedValue, onChange] ); useEffect(() => { diff --git a/app/react/portainer/gitops/RelativePathFieldset/RelativePathFieldset.tsx b/app/react/portainer/gitops/RelativePathFieldset/RelativePathFieldset.tsx index 6c643a1b4..4b4dac139 100644 --- a/app/react/portainer/gitops/RelativePathFieldset/RelativePathFieldset.tsx +++ b/app/react/portainer/gitops/RelativePathFieldset/RelativePathFieldset.tsx @@ -1,9 +1,9 @@ +import { useState } from 'react'; import { FormikErrors } from 'formik'; import { GitFormModel } from '@/react/portainer/gitops/types'; import { PathSelector } from '@/react/portainer/gitops/ComposePathField/PathSelector'; import { dummyGitForm } from '@/react/portainer/gitops/RelativePathFieldset/utils'; -import { useEnableFsPath } from '@/react/portainer/gitops/RelativePathFieldset/useEnableFsPath'; import { SwitchField } from '@@/form-components/SwitchField'; import { TextTip } from '@@/Tip/TextTip'; @@ -30,17 +30,20 @@ export function RelativePathFieldset({ hideEdgeConfigs, errors, }: Props) { - const { enableFsPath0, enableFsPath1, toggleFsPath } = useEnableFsPath(value); + const [relativePathManuallyEnabled, setRelativePathManuallyEnabled] = + useState(value.SupportRelativePath); + + const [relativePathForcedEnabled, setRelativePathForcedEnabled] = useState( + value.SupportPerDeviceConfigs + ); const gitoptsEdgeConfigDocUrl = useDocsUrl( '/user/edge/stacks/add#gitops-edge-configurations' ); - const pathTip0 = + const pathTipSwarm = 'For relative path volumes use with Docker Swarm, you must have a network filesystem which all of your nodes can access.'; - const pathTip1 = - 'Relative path is active. When you set the ‘local filesystem path’, it will also be utilzed for GitOps Edge configuration.'; - const pathTip2 = + const pathTipGitopsActive = 'GitOps Edge configurations is active. When you set the ‘local filesystem path’, it will also be utilized for relative paths.'; return ( @@ -53,10 +56,10 @@ export function RelativePathFieldset({ label="Enable relative path volumes" labelClass="col-sm-3 col-lg-2" tooltip="Enabling this means you can specify relative path volumes in your Compose files, with Portainer pulling the content from your git repository to the environment the stack is deployed to." - disabled={isEditing} + disabled={isEditing || relativePathForcedEnabled} checked={value.SupportRelativePath} onChange={(value) => { - toggleFsPath(0, value); + setRelativePathManuallyEnabled(value); handleChange({ SupportRelativePath: value }); }} /> @@ -68,31 +71,33 @@ export function RelativePathFieldset({
- {enableFsPath1 ? pathTip2 : pathTip0} + {relativePathForcedEnabled ? pathTipGitopsActive : pathTipSwarm}
-
-
- - - handleChange({ FilesystemPath: e.target.value }) - } - /> - + {(!relativePathForcedEnabled || hideEdgeConfigs) && ( +
+
+ + + handleChange({ FilesystemPath: e.target.value }) + } + /> + +
-
+ )} )} @@ -117,9 +122,12 @@ export function RelativePathFieldset({ tooltip="By enabling the GitOps Edge Configurations feature, you gain the ability to define relative path volumes in your configuration files. Portainer will then automatically fetch the content from your git repository by matching the folder name or file name with the Portainer Edge ID, and apply it to the environment where the stack is deployed" disabled={isEditing} checked={!!value.SupportPerDeviceConfigs} - onChange={(value) => { - toggleFsPath(1, value); - handleChange({ SupportPerDeviceConfigs: value }); + onChange={(v) => { + setRelativePathForcedEnabled(v); + handleChange({ + SupportPerDeviceConfigs: v, + SupportRelativePath: v ? true : relativePathManuallyEnabled, + }); }} />
@@ -127,38 +135,32 @@ export function RelativePathFieldset({ {value.SupportPerDeviceConfigs && ( <> - {!isEditing && ( -
-
- - {enableFsPath0 ? pathTip1 : pathTip0} - -
+
+
+ {pathTipSwarm}
- )} +
- {!isEditing && ( -
-
- - - handleChange({ FilesystemPath: e.target.value }) - } - /> - -
+
+
+ + + handleChange({ FilesystemPath: e.target.value }) + } + /> +
- )} +
diff --git a/app/react/portainer/gitops/RelativePathFieldset/useEnableFsPath.ts b/app/react/portainer/gitops/RelativePathFieldset/useEnableFsPath.ts deleted file mode 100644 index e2f5b2664..000000000 --- a/app/react/portainer/gitops/RelativePathFieldset/useEnableFsPath.ts +++ /dev/null @@ -1,22 +0,0 @@ -import { useState } from 'react'; - -import { RelativePathModel } from './types'; - -export function useEnableFsPath(initialValue: RelativePathModel) { - const [state, setState] = useState(() => - initialValue.SupportPerDeviceConfigs ? [1] : [] - ); - - const enableFsPath0 = state.length && state[0] === 0; - const enableFsPath1 = state.length && state[0] === 1; - - function toggleFsPath(idx: number, enable: boolean) { - if (enable) { - setState([...state, idx]); - } else { - setState(state.filter((e) => e !== idx)); - } - } - - return { enableFsPath0, enableFsPath1, toggleFsPath }; -}