Skip to content

Commit 7cf1b15

Browse files
authored
Remove "isLoaded" flag from deployment form (#5605)
### Description of the change This PR solves a bug found during a manual test prior to the release: when selecting a different version, the `isLoaded` flag was preventing a proper execution of the useEffect flow. The flag has been replaced with `isLoading`, which just avoid re-renders when an expensive operation is running (like parsing the schema). ### Benefits Users upgrading package will see the deployed values (correct) instead of the pkg defaults (incorrect). ### Possible drawbacks In those cases, the component will re-render twice, but that's not a big deal (no expensive backend calls, the table is now performing well, etc). ### Applicable issues - fixes #5603 ### Additional information ![deployedValuesFix](https://user-images.githubusercontent.com/11535726/199706432-720c65c0-8351-438d-86d0-bbf3192a1955.gif) Signed-off-by: Antonio Gamez Diaz <[email protected]>
1 parent 64f5923 commit 7cf1b15

File tree

1 file changed

+15
-7
lines changed

1 file changed

+15
-7
lines changed

dashboard/src/components/DeploymentForm/DeploymentFormBody/DeploymentFormBody.tsx

+15-7
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ function DeploymentFormBody({
8383
);
8484

8585
const [restoreModalIsOpen, setRestoreModalOpen] = useState(false);
86-
const [isLoaded, setIsLoaded] = useState(false);
8786
const [isLoading, setIsLoading] = useState(true);
8887
const [unsavedChangesMap] = useState(new Map<string, any>());
8988
const [shouldSubmitForm, setShouldSubmitForm] = useState(false);
@@ -92,9 +91,9 @@ function DeploymentFormBody({
9291
// we need to force a new extraction of the params from the schema
9392
useEffect(() => {
9493
if (
95-
!isLoaded &&
96-
shouldRenderBasicForm(schemaFromTheParentContainerParsed) &&
97-
!isEmpty(valuesFromTheParentContainerNodes)
94+
!isLoading &&
95+
!isEmpty(valuesFromTheParentContainerNodes) &&
96+
shouldRenderBasicForm(schemaFromTheParentContainerParsed)
9897
) {
9998
const initialParamsFromContainer = retrieveBasicFormParams(
10099
valuesFromTheParentContainerNodes,
@@ -104,12 +103,11 @@ function DeploymentFormBody({
104103
valuesFromTheDeployedPackageNodes,
105104
);
106105
setParamsFromComponentState(initialParamsFromContainer);
107-
setIsLoaded(true);
108106
setIsLoading(false);
109107
}
110108
}, [
111109
deploymentEvent,
112-
isLoaded,
110+
isLoading,
113111
schemaFromTheParentContainerParsed,
114112
valuesFromTheAvailablePackageNodes,
115113
valuesFromTheDeployedPackageNodes,
@@ -119,21 +117,27 @@ function DeploymentFormBody({
119117
// parse and store in the component state the values from the available package
120118
useEffect(() => {
121119
if (valuesFromTheAvailablePackage) {
120+
setIsLoading(true);
122121
setValuesFromTheAvailablePackageNodes(parseToYamlNode(valuesFromTheAvailablePackage));
122+
setIsLoading(false);
123123
}
124124
}, [valuesFromTheAvailablePackage]);
125125

126126
// parse and store in the component state the current values (which come from the parent container)
127127
useEffect(() => {
128128
if (valuesFromTheParentContainer) {
129+
setIsLoading(true);
129130
setValuesFromTheParentContainerNodes(parseToYamlNode(valuesFromTheParentContainer));
131+
setIsLoading(false);
130132
}
131133
}, [valuesFromTheParentContainer]);
132134

133135
// parse and store in the component state the values from the deployed package
134136
useEffect(() => {
135137
if (valuesFromTheDeployedPackage) {
138+
setIsLoading(true);
136139
setValuesFromTheDeployedPackageNodes(parseToYamlNode(valuesFromTheDeployedPackage));
140+
setIsLoading(false);
137141
}
138142
}, [valuesFromTheDeployedPackage]);
139143

@@ -153,7 +157,6 @@ function DeploymentFormBody({
153157
const schemaObject = schemaToObject(schemaFromTheParentContainerString);
154158
setSchemaFromTheParentContainerParsed(schemaObject);
155159
setIsLoading(false);
156-
setIsLoaded(false);
157160
}, [schemaFromTheParentContainerString]);
158161

159162
// when the shouldSubmitForm flag is enabled, the form will be submitted, but using a native
@@ -208,6 +211,7 @@ function DeploymentFormBody({
208211
// re-build the table based on the new YAML
209212
const refreshBasicParameters = () => {
210213
if (shouldRenderBasicForm(schemaFromTheParentContainerParsed)) {
214+
setIsLoading(true);
211215
setParamsFromComponentState(
212216
retrieveBasicFormParams(
213217
valuesFromTheParentContainerNodes,
@@ -217,6 +221,7 @@ function DeploymentFormBody({
217221
valuesFromTheDeployedPackageNodes,
218222
),
219223
);
224+
setIsLoading(false);
220225
}
221226
};
222227

@@ -245,6 +250,7 @@ function DeploymentFormBody({
245250
};
246251

247252
const restoreDefaultValues = () => {
253+
setIsLoading(true);
248254
setValuesFromTheParentContainer(valuesFromTheAvailablePackage || "");
249255
setSchemaFromTheParentContainerParsed(schemaFromTheAvailablePackage as JSONSchemaType<any>);
250256
setSchemaFromTheParentContainerString(schemaToString(schemaFromTheAvailablePackage));
@@ -260,6 +266,7 @@ function DeploymentFormBody({
260266
);
261267
}
262268
setRestoreModalOpen(false);
269+
setIsLoading(false);
263270
};
264271

265272
// early return if error
@@ -273,6 +280,7 @@ function DeploymentFormBody({
273280

274281
// early return if loading
275282
if (
283+
isLoading ||
276284
packagesIsFetching ||
277285
!availablePackageDetail ||
278286
(!versions.length &&

0 commit comments

Comments
 (0)