Skip to content

Commit 0ffaa2d

Browse files
Fix two related bugs in the BuildWorkflowParameters component:
1. Workflow parameters merging when changing workflow types When user changed from one workflow type to another (e.g., google-cloud-buildpacks to docker), the review step showed merged parameters from both workflows instead of only the currently selected workflow's parameters. 2. Form data loss when navigating back in wizard When user navigated forward to trait steps and then back to workflow configuration, all entered parameters were lost, forcing them to re-enter values.
1 parent 65898e7 commit 0ffaa2d

File tree

1 file changed

+28
-6
lines changed

1 file changed

+28
-6
lines changed

packages/app/src/scaffolder/BuildWorkflowParameters/BuildWorkflowParametersExtension.tsx

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, useState } from 'react';
1+
import { useEffect, useState, useRef } from 'react';
22
import { FieldExtensionComponentProps } from '@backstage/plugin-scaffolder-react';
33
import { Typography, Box } from '@material-ui/core';
44
import {
@@ -45,6 +45,10 @@ export const BuildWorkflowParameters = ({
4545
const [loading, setLoading] = useState(false);
4646
const [error, setError] = useState<string | null>(null);
4747

48+
// Track workflow changes to force Form remount only when workflow actually changes
49+
const [resetKey, setResetKey] = useState(0);
50+
const prevWorkflowRef = useRef<string | undefined>(undefined);
51+
4852
const discoveryApi = useApi(discoveryApiRef);
4953
const identityApi = useApi(identityApiRef);
5054

@@ -53,6 +57,16 @@ export const BuildWorkflowParameters = ({
5357
const selectedWorkflowName = formContext?.formData?.workflow_name;
5458
const organizationName = formContext?.formData?.organization_name;
5559

60+
// Increment resetKey only when workflow actually changes
61+
// This forces Form remount only on workflow change, not on every render
62+
useEffect(() => {
63+
if (prevWorkflowRef.current !== undefined &&
64+
prevWorkflowRef.current !== selectedWorkflowName) {
65+
setResetKey(prev => prev + 1);
66+
}
67+
prevWorkflowRef.current = selectedWorkflowName;
68+
}, [selectedWorkflowName]);
69+
5670
// Fetch workflow schema when workflow selection changes
5771
useEffect(() => {
5872
let ignore = false;
@@ -124,20 +138,27 @@ export const BuildWorkflowParameters = ({
124138
};
125139
}, [selectedWorkflowName, organizationName, discoveryApi, identityApi]);
126140

127-
// Sync schema to formData when workflow schema changes
141+
// Sync schema to formData when workflow changes (not just when schema loads)
128142
useEffect(() => {
129-
if (workflowSchema) {
130-
// Update formData to include the schema for validation
143+
if (workflowSchema && resetKey > 0) {
144+
// Clear old parameters only when workflow actually changes (resetKey increments)
145+
// This prevents clearing when user navigates back to this step
146+
onChange({
147+
parameters: {},
148+
schema: workflowSchema,
149+
});
150+
} else if (workflowSchema && resetKey === 0 && !formData?.schema) {
151+
// First time loading: initialize with existing or empty parameters
131152
onChange({
132153
parameters: formData?.parameters || {},
133154
schema: workflowSchema,
134155
});
135156
}
136-
// We intentionally only depend on workflowSchema to avoid infinite loop.
157+
// We intentionally only depend on workflowSchema and resetKey.
137158
// Adding onChange or formData would cause infinite re-renders:
138159
// onChange -> formData changes -> useEffect triggers -> onChange -> loop
139160
// eslint-disable-next-line react-hooks/exhaustive-deps
140-
}, [workflowSchema]);
161+
}, [workflowSchema, resetKey]);
141162

142163
// Handle form data changes from RJSF
143164
const handleFormChange = (changeEvent: any) => {
@@ -190,6 +211,7 @@ export const BuildWorkflowParameters = ({
190211
Workflow Parameters
191212
</Typography>
192213
<Form
214+
key={resetKey}
193215
schema={workflowSchema}
194216
uiSchema={uiSchema}
195217
formData={formData?.parameters || {}}

0 commit comments

Comments
 (0)