enhan: implement default value handling for number fields and add utility functions - #5246
Open
micanipho wants to merge 3 commits into
Open
enhan: implement default value handling for number fields and add utility functions#5246micanipho wants to merge 3 commits into
micanipho wants to merge 3 commits into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Contributor
WalkthroughChangesThe form store now resolves configured defaults, converts values using metadata, and applies them during model initialization and data loading. Number fields use Number field defaults
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant useShaForm
participant ShaFormInstance
participant getFormDefaultValues
participant FormModel
useShaForm->>ShaFormInstance: Provide toolbox components and application context
ShaFormInstance->>getFormDefaultValues: Resolve configured component defaults
getFormDefaultValues-->>ShaFormInstance: Return evaluated default values
ShaFormInstance->>FormModel: Apply defaults to missing model properties
FormModel-->>ShaFormInstance: Return initialized form data
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shesha-reactjs/src/designer-components/numberField/numberField.tsx`:
- Around line 23-29: Define a shared number-field value type covering number,
string, null, and undefined, then use it for INumberFieldEditorProps.value and
the onChange callback parameter instead of any. Update the related control.tsx
value typing to use the same type so stringMode supports high-precision string
values consistently.
In `@shesha-reactjs/src/designer-components/numberField/utils.ts`:
- Around line 33-39: Update the default-value parsing logic around
resolveDefaultValue to reject partially numeric strings such as “12abc” and
non-finite values such as Infinity. Validate the entire evaluated string as a
numeric literal rather than relying on parseFloat prefixes; for
non-high-precision fields, return undefined unless the parsed number is finite,
while preserving raw valid strings for high-precision stringMode.
- Around line 20-22: Update resolveDefaultValue and the related value input to
use unknown instead of any, then add appropriate validation or narrowing before
evaluation and coercion so existing behavior remains safe. Also tighten any
IDefaultValueEvaluationContext properties that currently use any where their
form-mode/data shapes are known.
- Around line 10-24: The number field should no longer resolve or apply
DefaultValue directly. Remove the resolveDefaultValue utility and its usage in
numberField.tsx, including the initialValue/onChange path, and rely on data
produced by the field’s onAfterDataLoad scripts; update both affected files
accordingly.
- Around line 3-7: Remove globalState from IDefaultValueEvaluationContext in
shesha-reactjs/src/designer-components/numberField/utils.ts and update
resolveDefaultValue callers accordingly. In
shesha-reactjs/src/designer-components/numberField/numberField.tsx, remove the
useGlobalState dependency and stop passing globalState into resolveDefaultValue;
use the typed DataContext values instead while preserving the existing
default-value evaluation behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 84fe47d2-9437-45c5-8fab-a2a7bab9d552
📒 Files selected for processing (3)
shesha-reactjs/src/designer-components/numberField/control.tsxshesha-reactjs/src/designer-components/numberField/numberField.tsxshesha-reactjs/src/designer-components/numberField/utils.ts
micanipho
requested review from
IvanIlyichev and
James-Baloyi
and removed request for
James-Baloyi
August 5, 2026 09:40
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shesha-reactjs/src/providers/form/store/defaultValues.ts`:
- Around line 39-40: Update isValueSpecified and the applyDefaultValues update
path around lines 188–193 so explicit model values, including null and empty
strings, are preserved. Use a predicate for model presence that treats only
undefined as missing when deciding whether to apply defaults, while retaining
any distinct validation needed for configured default values.
- Around line 93-103: Update the numeric conversion logic in the data-type
handling branch around NUMERIC_LITERAL_REGEX so accepted finite literals such as
.5, 1.0, and 1e3 convert to numbers regardless of lexical formatting. Return the
original value only for non-finite results or unsafe integer results, while
preserving valid safe integers and finite non-integer values.
- Around line 10-20: Replace any-based default-value typing with unknown across
IFormDefaultValue.value, DefaultValueEvaluator, isValueSpecified,
convertToDataType, resolveDefaultValue, and applyDefaultValues. Add explicit
type guards at conversion and model-write boundaries so only validated values
are passed onward, while preserving existing default-value resolution behavior.
In `@shesha-reactjs/src/providers/form/store/shaFormInstance.tsx`:
- Around line 344-354: Remove the default-value resolution and application from
`#initModel`, including getFormDefaultValues, this.defaultValues, and
applyDefaultValues usage there. Configure form defaults through the existing
onAfterDataLoad script flow instead, ensuring scripts execute in dependency
order and write directly to data or context without SetFieldValue.
- Line 30: Update the ApplicationContextGetter type alias to return
IApplicationContext instead of any, ensuring consumers such as
`#evaluateDefaultValue` remain type-checked.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0473e285-9c8e-4d68-99d5-9eab202e0288
📒 Files selected for processing (4)
shesha-reactjs/src/designer-components/numberField/control.tsxshesha-reactjs/src/designer-components/numberField/numberField.tsxshesha-reactjs/src/providers/form/store/defaultValues.tsshesha-reactjs/src/providers/form/store/shaFormInstance.tsx
Comment on lines
+10
to
+20
| export interface IFormDefaultValue { | ||
| /** Full name (path) of the property the value is bound to, e.g. `qty` or `address.city` */ | ||
| propertyName: string; | ||
| value: any; | ||
| } | ||
|
|
||
| /** | ||
| * Resolves the actual value of the `defaultValue` setting of a component. The setting may be specified as an | ||
| * expression (JS setting or a legacy mustache template) and has to be evaluated in the context of the form. | ||
| */ | ||
| export type DefaultValueEvaluator = (component: IConfigurableFormComponent) => any; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target file and nearby use sites without running repository code.
target="shesha-reactjs/src/providers/form/store/defaultValues.ts"
printf 'Target file exists: '
if [ -f "$target" ]; then echo yes; else echo no; fi
printf '\nFile outline:\n'
ast-grep outline "$target" || true
printf '\nRelevant sections:\n'
cat -n "$target" | sed -n '1,240p'
printf '\nSearch for API names in TypeScript files:\n'
rg -n "IFormDefaultValue|DefaultValueEvaluator|applyDefaultValues|\\.applyDefaultValues|applyDefaultValues\\(" -g '*.ts' -g '*.tsx' .Repository: shesha-io/shesha-framework
Length of output: 11756
Replace the untyped default-value contract.
IFormDefaultValue.value, DefaultValueEvaluator, isValueSpecified, convertToDataType, resolveDefaultValue, and applyDefaultValues propagate any through default-value resolution and model writes. Use unknown for unknown payload values and add explicit type guards before conversion or writes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shesha-reactjs/src/providers/form/store/defaultValues.ts` around lines 10 -
20, Replace any-based default-value typing with unknown across
IFormDefaultValue.value, DefaultValueEvaluator, isValueSpecified,
convertToDataType, resolveDefaultValue, and applyDefaultValues. Add explicit
type guards at conversion and model-write boundaries so only validated values
are passed onward, while preserving existing default-value resolution behavior.
Source: Coding guidelines
Comment on lines
+39
to
+40
| /** Returns true when the value should be treated as specified, i.e. it should not be replaced by a default value. */ | ||
| const isValueSpecified = (value: any): boolean => value !== undefined && value !== null && value !== ''; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not replace explicit null model values.
Line 40 treats null and '' as unspecified. Line 189 then applies the configured default over an explicit cleared value. This contradicts the documented rule that existing model values win.
Use separate predicates for configured defaults and model values. Treat only an absent property (undefined) as missing when applyDefaultValues updates the model.
Proposed fix
-const isValueSpecified = (value: any): boolean => value !== undefined && value !== null && value !== '';
+const hasConfiguredDefault = (value: unknown): boolean => value !== undefined && value !== null && value !== '';
+const hasModelValue = (value: unknown): boolean => value !== undefined;
...
- if (isValueSpecified(getValueByPropertyName(result, propertyName)))
+ if (hasModelValue(getValueByPropertyName(result, propertyName)))
return;Also applies to: 188-193
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shesha-reactjs/src/providers/form/store/defaultValues.ts` around lines 39 -
40, Update isValueSpecified and the applyDefaultValues update path around lines
188–193 so explicit model values, including null and empty strings, are
preserved. Use a predicate for model presence that treats only undefined as
missing when deciding whether to apply defaults, while retaining any distinct
validation needed for configured default values.
Comment on lines
+93
to
+103
| if (dataType === DataTypes.number) { | ||
| if (!NUMERIC_LITERAL_REGEX.test(trimmed)) | ||
| return value; | ||
|
|
||
| const parsed = Number(trimmed); | ||
|
|
||
| // keep the literal when the conversion is not lossless, high precision values may legitimately exceed | ||
| // `Number.MAX_SAFE_INTEGER` | ||
| const isLossless = Number.isFinite(parsed) && (Number.isSafeInteger(parsed) || String(parsed) === trimmed); | ||
|
|
||
| return isLossless ? parsed : value; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Convert accepted exact numeric literals.
The regex accepts .5, 1.0, and 1e3, but Line 101 compares the parsed number with the original lexical form. These values remain strings despite being valid numeric defaults. A number-model property can then receive a string.
Check numeric safety without requiring the canonical String(parsed) form to equal the source literal. Preserve only non-finite values and unsafe integer values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shesha-reactjs/src/providers/form/store/defaultValues.ts` around lines 93 -
103, Update the numeric conversion logic in the data-type handling branch around
NUMERIC_LITERAL_REGEX so accepted finite literals such as .5, 1.0, and 1e3
convert to numbers regardless of lexical formatting. Return the original value
only for non-finite results or unsafe integer results, while preserving valid
safe integers and finite non-integer values.
| import { useFormDesignerComponents } from "../hooks"; | ||
|
|
||
| type ForceUpdateTrigger = () => void; | ||
| type ApplicationContextGetter = () => any; |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'shaFormInstance\.tsx$' . || true
echo "== file outline/size =="
wc -l shesha-reactjs/src/providers/form/store/shaFormInstance.tsx
ast-grep outline shesha-reactjs/src/providers/form/store/shaFormInstance.tsx --match ApplicationContextGetter --view expanded || true
echo "== relevant lines =="
sed -n '1,70p' shesha-reactjs/src/providers/form/store/shaFormInstance.tsx | cat -n
sed -n '320,370p' shesha-reactjs/src/providers/form/store/shaFormInstance.tsx | cat -n
echo "== imports/usages of IApplicationContext and constants =="
rg -n "IApplicationContext|ApplicationContextGetter|allConstants|`#evaluateDefaultValue`" shesha-reactjs/src/providers/form/store/shaFormInstance.tsxRepository: shesha-io/shesha-framework
Length of output: 8653
Remove any from ApplicationContextGetter.
This getter returns the form’s application context object. Use () => IApplicationContext instead of () => any so #evaluateDefaultValue remains type-checked.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shesha-reactjs/src/providers/form/store/shaFormInstance.tsx` at line 30,
Update the ApplicationContextGetter type alias to return IApplicationContext
instead of any, ensuring consumers such as `#evaluateDefaultValue` remain
type-checked.
Source: Coding guidelines
Comment on lines
+344
to
+354
| #initModel = <T,>(values: T): T => { | ||
| this.defaultValues = getFormDefaultValues({ | ||
| flatStructure: this.flatStructure, | ||
| toolboxComponents: this.toolboxComponents, | ||
| evaluator: this.#evaluateDefaultValue, | ||
| metadata: this.modelMetadata, | ||
| }); | ||
|
|
||
| this.log('LOG: default values', this.defaultValues); | ||
|
|
||
| return applyDefaultValues(values, this.defaultValues); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Move configured defaults to onAfterDataLoad.
#initModel resolves and writes component.defaultValue during model initialization. This bypasses the required dependency-ordered onAfterDataLoad scripts. A default expression that depends on another default can evaluate before its dependency is initialized.
Remove the DefaultValue-based initialization flow. Set form defaults from onAfterDataLoad scripts, where scripts can write directly to data and context.
As per coding guidelines, “Form default values must be set through onAfterDataLoad scripts rather than the DefaultValue property; scripts execute in dependency order and may write directly to data and context without SetFieldValue.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@shesha-reactjs/src/providers/form/store/shaFormInstance.tsx` around lines 344
- 354, Remove the default-value resolution and application from `#initModel`,
including getFormDefaultValues, this.defaultValues, and applyDefaultValues usage
there. Configure form defaults through the existing onAfterDataLoad script flow
instead, ensuring scripts execute in dependency order and write directly to data
or context without SetFieldValue.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors how default values are handled for number fields in the form designer, ensuring that configured defaults are properly written into the form data and submitted, rather than only appearing as uncontrolled UI state. The changes introduce a new utility for evaluating and applying default values, and update the component structure to use this logic consistently.
Default value handling improvements:
resolveDefaultValueutility inutils.tsto safely evaluate and coerce the configured default value (including expressions) for number fields, ensuring the correct type is submitted with the form.NumberFieldEditorcomponent innumberField.tsxthat ensures the default value is written into the form data when appropriate, rather than relying on uncontrolled input state. This uses a ref to avoid re-applying the default after user input.resolveDefaultValueandNumberFieldEditor, and to only apply the default value when the form is in edit mode and data loading is complete.Code cleanup and documentation:
defaultValueprop fromNumberFieldControlto prevent discrepancies between UI state and submitted data, with explanatory comments.Summary by CodeRabbit
Issue: #5212