Conversation
📝 WalkthroughWalkthroughThree files in the form handling layer were updated to adjust dirty-field detection, parameter value iteration, and form reset timing. The changes refine how React Hook Form state is tracked and applied during form operations. Changes
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/parameters/common/utils.ts (1)
101-106:⚠️ Potential issue | 🟠 MajorPotential TypeError when key exists in
_specificParametersValuesbut not informData[SPECIFIC_PARAMETERS].If the parameter description and form state diverge (e.g., provider schema adds new fields asynchronously), accessing
formData[SPECIFIC_PARAMETERS][key]may returnundefined, causing.toString()to throw. Add a guard to handle missing keys.🐛 Proposed fix to guard against missing keys
return Object.keys(_specificParametersValues).reduce((acc: SpecificParametersValues, key: string) => { + const formValue = formData[SPECIFIC_PARAMETERS]?.[key]; + if (formValue === undefined) { + return acc; + } - if (_specificParametersValues[key].toString() !== formData[SPECIFIC_PARAMETERS][key].toString()) { - acc[key] = formData[SPECIFIC_PARAMETERS][key].toString(); + if (_specificParametersValues[key].toString() !== formValue.toString()) { + acc[key] = formValue.toString(); } return acc; }, {});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/parameters/common/utils.ts` around lines 101 - 106, The reduce block using _specificParametersValues and formData[SPECIFIC_PARAMETERS] can throw when formData[SPECIFIC_PARAMETERS][key] is undefined; update the reducer in utils.ts (the block referencing _specificParametersValues, SPECIFIC_PARAMETERS, formData, and SpecificParametersValues) to first guard that formData[SPECIFIC_PARAMETERS] and formData[SPECIFIC_PARAMETERS][key] are not null/undefined before calling toString(); if the form value is missing, skip adding that key to acc (or use String(...) with a safe default) so no .toString() is invoked on undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/parameters/loadflow/use-load-flow-parameters-form.ts`:
- Around line 292-298: The two separate effects (the reset effect that calls
resetForm(params) and sets previousWatchProviderRef.current, and the
provider-switch effect that uses watchProvider) can run in either order and
cause a race; fix by either merging them into a single useEffect that handles
both the resetForm(params) call and the provider-switch logic (using the same
dependency array: paramsLoaded, params, specificParamsDescription,
watchProvider) so state updates happen deterministically, or add a coordinating
ref/flag (e.g., resettingRef) that the provider-switch effect checks and sets:
have the reset effect set resettingRef.current = true before running resetForm
and false after, and have the provider-switch effect return early when
resettingRef.current is true; update references to
previousWatchProviderRef.current, resetForm, params and watchProvider
accordingly.
---
Outside diff comments:
In `@src/components/parameters/common/utils.ts`:
- Around line 101-106: The reduce block using _specificParametersValues and
formData[SPECIFIC_PARAMETERS] can throw when formData[SPECIFIC_PARAMETERS][key]
is undefined; update the reducer in utils.ts (the block referencing
_specificParametersValues, SPECIFIC_PARAMETERS, formData, and
SpecificParametersValues) to first guard that formData[SPECIFIC_PARAMETERS] and
formData[SPECIFIC_PARAMETERS][key] are not null/undefined before calling
toString(); if the form value is missing, skip adding that key to acc (or use
String(...) with a safe default) so no .toString() is invoked on undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1af48898-9ecc-4b76-a4c3-3ca7064ae39c
📒 Files selected for processing (3)
src/components/inputs/reactHookForm/utils/SubmitButton.tsxsrc/components/parameters/common/utils.tssrc/components/parameters/loadflow/use-load-flow-parameters-form.ts
| useEffect(() => { | ||
| if (!params) { | ||
| return; | ||
| } | ||
| resetForm(params); | ||
| previousWatchProviderRef.current = params.provider; // Do no reset values when switch provider from a callBack | ||
| }, [paramsLoaded, params, specificParamsDescription]); |
There was a problem hiding this comment.
Effect ordering may cause race condition with provider-switch effect.
Both effects run on the same render cycle when params changes. Since React doesn't guarantee execution order between effects, the provider-switch effect (lines 264-290) could run before this reset effect, causing previousWatchProviderRef.current to be set to watchProvider before being corrected here. Consider combining these into a single effect or using a flag to coordinate.
Run the following script to check if there are integration tests covering the reset scenario:
#!/bin/bash
# Search for tests covering loadflow parameter reset or provider switching
rg -n -C3 'resetForm|previousWatchProvider|reset.*param' --glob '*test*' --glob '*spec*'Additionally, verify eslint-plugin-react-hooks version:
#!/bin/bash
# Check eslint-plugin-react-hooks version for useEffectEvent support
cat package.json | jq '.dependencies["eslint-plugin-react-hooks"] // .devDependencies["eslint-plugin-react-hooks"]'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/parameters/loadflow/use-load-flow-parameters-form.ts` around
lines 292 - 298, The two separate effects (the reset effect that calls
resetForm(params) and sets previousWatchProviderRef.current, and the
provider-switch effect that uses watchProvider) can run in either order and
cause a race; fix by either merging them into a single useEffect that handles
both the resetForm(params) call and the provider-switch logic (using the same
dependency array: paramsLoaded, params, specificParamsDescription,
watchProvider) so state updates happen deterministically, or add a coordinating
ref/flag (e.g., resettingRef) that the provider-switch effect checks and sets:
have the reset effect set resettingRef.current = true before running resetForm
and false after, and have the provider-switch effect return early when
resettingRef.current is true; update references to
previousWatchProviderRef.current, resetForm, params and watchProvider
accordingly.
2 participants
PR Summary