From 41b3e2bf6da73d2668e7a4361a639c28cdaa3232 Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 16 May 2025 22:06:28 +0000 Subject: [PATCH 1/4] fix: sync websocket params with form params --- .../CreateWorkspacePageViewExperimental.tsx | 93 ++++++++++++++++++- 1 file changed, 89 insertions(+), 4 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index 7d22316bfe4f7..f73a7413f3afb 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -113,6 +113,23 @@ export const CreateWorkspacePageViewExperimental: FC< setSuggestedName(() => generateWorkspaceName()); }, []); + const autofillByName = Object.fromEntries( + autofillParameters.map((param) => [param.name, param]), + ); + + // only touched fields are sent to the websocket + // In the case of autofill parameters, we need to mark the parameter as touched + // so that it is sent to the websocket + const initialTouched = parameters.reduce( + (touched, parameter) => { + if (autofillByName[parameter.name] !== undefined) { + touched[parameter.name] = true; + } + return touched; + }, + {} as Record, + ); + const form: FormikContextType = useFormik({ initialValues: { @@ -123,6 +140,7 @@ export const CreateWorkspacePageViewExperimental: FC< autofillParameters, ), }, + initialTouched, validationSchema: Yup.object({ name: nameValidator("Workspace Name"), rich_parameter_values: @@ -140,10 +158,6 @@ export const CreateWorkspacePageViewExperimental: FC< }, }); - const autofillByName = Object.fromEntries( - autofillParameters.map((param) => [param.name, param]), - ); - useEffect(() => { if (error) { window.scrollTo(0, 0); @@ -250,6 +264,12 @@ export const CreateWorkspacePageViewExperimental: FC< sendDynamicParamsRequest(parameter, value); }; + useSyncFormParameters({ + parameters, + formValues: form.values.rich_parameter_values, + setFieldValue: form.setFieldValue, + }); + return ( <>
@@ -568,3 +588,68 @@ const Diagnostics: FC = ({ diagnostics }) => {
); }; + +/** + * Custom hook to synchronize parameters with form values. + */ +function useSyncFormParameters({ + parameters, + formValues, + setFieldValue, +}: { + parameters: readonly PreviewParameter[] | undefined; + formValues: readonly TypesGen.WorkspaceBuildParameter[] | undefined; + setFieldValue: ( + field: string, + value: TypesGen.WorkspaceBuildParameter[], + ) => void; +}) { + const parametersRef = useRef( + parameters, + ); + + useEffect(() => { + const shouldSync = () => { + if (!parameters) return false; + if (!formValues) return true; + + if (parametersRef.current?.length !== parameters.length) return true; + + const currentParamNames = new Set((formValues || []).map((p) => p.name)); + const newParamNames = new Set(parameters.map((p) => p.name)); + + const hasNewParams = parameters.some( + (p) => !currentParamNames.has(p.name), + ); + const hasRemovedParams = (formValues || []).some( + (p) => !newParamNames.has(p.name), + ); + + return hasNewParams || hasRemovedParams; + }; + + if (!shouldSync()) return; + if (!parameters) return; + + const currentFormParameters = formValues || []; + + const newParameterValues = parameters.map((parameter) => { + const existingParam = currentFormParameters.find( + (p) => p.name === parameter.name, + ); + + if (existingParam) { + return existingParam; + } + + return { + name: parameter.name, + value: parameter.value.valid ? parameter.value.value : "", + }; + }); + + setFieldValue("rich_parameter_values", newParameterValues); + + parametersRef.current = parameters; + }, [parameters, formValues, setFieldValue]); +} From 68cb368fb7af052cef4c4e5c19373adf5d53318c Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Fri, 16 May 2025 22:17:02 +0000 Subject: [PATCH 2/4] fix: add null check for response.parameters --- .../CreateWorkspacePage/CreateWorkspacePageExperimental.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx index e260e030a3c99..8268ded111b59 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageExperimental.tsx @@ -136,7 +136,7 @@ const CreateWorkspacePageExperimental: FC = () => { return; } - if (!initialParamsSentRef.current && response.parameters.length > 0) { + if (!initialParamsSentRef.current && response.parameters?.length > 0) { sendInitialParameters([...response.parameters]); } From 711cbdc0c2f6c2810aa4a7f76fd5b013ce97d9ee Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Sun, 18 May 2025 12:03:46 +0000 Subject: [PATCH 3/4] fix: improve useSyncFormParameters logic --- .../CreateWorkspacePageViewExperimental.tsx | 94 ++++++++----------- 1 file changed, 41 insertions(+), 53 deletions(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index f73a7413f3afb..87b909baa42f4 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -117,9 +117,8 @@ export const CreateWorkspacePageViewExperimental: FC< autofillParameters.map((param) => [param.name, param]), ); - // only touched fields are sent to the websocket - // In the case of autofill parameters, we need to mark the parameter as touched - // so that it is sent to the websocket + // Only touched fields are sent to the websocket + // Autofilled parameters are marked as touched since they have been modified const initialTouched = parameters.reduce( (touched, parameter) => { if (autofillByName[parameter.name] !== undefined) { @@ -130,6 +129,11 @@ export const CreateWorkspacePageViewExperimental: FC< {} as Record, ); + // The form parameters values hold the working state of the parameters that will be submitted when creating a workspace + // 1. The form parameter values are initialized from the websocket response when the form is mounted + // 2. Only touched form fields are sent to the websocket, a field is touched if edited by the user or set by autofill + // 3. The websocket response may add or remove parameters, these are added or removed from the form values in the useSyncFormParameters hook + // 4. All existing form parameters are updated to match the websocket response in the useSyncFormParameters hook const form: FormikContextType = useFormik({ initialValues: { @@ -266,7 +270,7 @@ export const CreateWorkspacePageViewExperimental: FC< useSyncFormParameters({ parameters, - formValues: form.values.rich_parameter_values, + formValues: form.values.rich_parameter_values ?? [], setFieldValue: form.setFieldValue, }); @@ -589,67 +593,51 @@ const Diagnostics: FC = ({ diagnostics }) => { ); }; -/** - * Custom hook to synchronize parameters with form values. - */ -function useSyncFormParameters({ - parameters, - formValues, - setFieldValue, -}: { - parameters: readonly PreviewParameter[] | undefined; - formValues: readonly TypesGen.WorkspaceBuildParameter[] | undefined; +type UseSyncFormParametersProps = { + parameters: readonly PreviewParameter[]; + formValues: readonly TypesGen.WorkspaceBuildParameter[]; setFieldValue: ( field: string, value: TypesGen.WorkspaceBuildParameter[], ) => void; -}) { - const parametersRef = useRef( - parameters, - ); - - useEffect(() => { - const shouldSync = () => { - if (!parameters) return false; - if (!formValues) return true; - - if (parametersRef.current?.length !== parameters.length) return true; - - const currentParamNames = new Set((formValues || []).map((p) => p.name)); - const newParamNames = new Set(parameters.map((p) => p.name)); +}; - const hasNewParams = parameters.some( - (p) => !currentParamNames.has(p.name), - ); - const hasRemovedParams = (formValues || []).some( - (p) => !newParamNames.has(p.name), - ); +export function useSyncFormParameters({ + parameters, + formValues, + setFieldValue, +}: UseSyncFormParametersProps) { + // Form values only needs to be updated when parameters change + // Keep track of form values in a ref to avoid unnecessary updates to rich_parameter_values + const formValuesRef = useRef(formValues); - return hasNewParams || hasRemovedParams; - }; + useEffect(() => { + formValuesRef.current = formValues; + }, [formValues]); - if (!shouldSync()) return; + useEffect(() => { if (!parameters) return; + const currentFormValues = formValuesRef.current; - const currentFormParameters = formValues || []; - - const newParameterValues = parameters.map((parameter) => { - const existingParam = currentFormParameters.find( - (p) => p.name === parameter.name, - ); - - if (existingParam) { - return existingParam; - } - + const newParameterValues = parameters.map((param) => { return { - name: parameter.name, - value: parameter.value.valid ? parameter.value.value : "", + name: param.name, + value: param.value.valid ? param.value.value : "", }; }); - setFieldValue("rich_parameter_values", newParameterValues); + const isChanged = + currentFormValues.length !== newParameterValues.length || + newParameterValues.some( + (p) => + !currentFormValues.find( + (formValue) => + formValue.name === p.name && formValue.value === p.value, + ), + ); - parametersRef.current = parameters; - }, [parameters, formValues, setFieldValue]); + if (isChanged) { + setFieldValue("rich_parameter_values", newParameterValues); + } + }, [parameters, setFieldValue]); } From 0bab142549d8eb6299f57b64515785d30d9e468b Mon Sep 17 00:00:00 2001 From: Jaayden Halko Date: Sun, 18 May 2025 12:33:55 +0000 Subject: [PATCH 4/4] fix: remove export --- .../CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx index 87b909baa42f4..bfeecc173f3e3 100644 --- a/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx +++ b/site/src/pages/CreateWorkspacePage/CreateWorkspacePageViewExperimental.tsx @@ -602,7 +602,7 @@ type UseSyncFormParametersProps = { ) => void; }; -export function useSyncFormParameters({ +function useSyncFormParameters({ parameters, formValues, setFieldValue,