Conversation
WalkthroughSwitched autocomplete loading gating to use Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shesha-reactjs/src/components/autocomplete/index.tsx (1)
84-105: Avoidsource.tableDataaccess after optional chaining (no TS narrowing) + verifyisFetchingTableDatalifecycle.
if (source?.tableData?.length > 0) { ... source.tableData ... }doesn’t reliably narrowsource/tableData(optional chaining won’t help TS here, and can still be brittle iftableDatachanges between checks).- if (source?.tableData?.length > 0) { - const foundValues = keys.map((x) => source.tableData.find((y) => keyValueFunc(outcomeValueFunc(y, allData), allData) === x)) + const tableData = source?.tableData; + if (tableData?.length > 0) { + const foundValues = keys + .map((x) => tableData.find((y) => keyValueFunc(outcomeValueFunc(y, allData), allData) === x)) .filter((v) => v != null); if (foundValues.length > 0) { selected.current = foundValues; } else { // Use original values as fallback selected.current = getNormalizedValues(props.value); } } else { // Use original values as fallback selected.current = getNormalizedValues(props.value); }Also please double-check that
source.isFetchingTableDataalways flips back tofalseon both success and failure; otherwiseloadingValuescan remain stucktrueand the control never exits the loading state.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
shesha-reactjs/src/components/autocomplete/index.tsx(3 hunks)shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx(1 hunks)shesha-reactjs/src/designer-components/slider/index.tsx(3 hunks)shesha-reactjs/src/designer-components/slider/interfaces.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)
**/*.{ts,tsx}: Eliminate theanytype; useunknowntype instead for values with unknown types, forcing explicit type checking
Prefer type guards over type casting for type checking
Avoid monolithic types; use discriminated unions with a discriminator property instead
Leverage TypeScript to its full potential as a type system, not merely as a linter
Files:
shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsxshesha-reactjs/src/components/autocomplete/index.tsxshesha-reactjs/src/designer-components/slider/interfaces.tsshesha-reactjs/src/designer-components/slider/index.tsx
🧠 Learnings (5)
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsxshesha-reactjs/src/designer-components/slider/interfaces.tsshesha-reactjs/src/designer-components/slider/index.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*{Component,Editor}.{ts,tsx} : Remove the `defaultValue` pattern entirely from controlled editor components
Applied to files:
shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
📚 Learning: 2025-11-20T14:51:46.152Z
Learnt from: AlexStepantsov
Repo: shesha-io/shesha-framework PR: 4235
File: shesha-reactjs/src/components/modelConfigurator/propertiesEditor/provider/reducer.ts:149-149
Timestamp: 2025-11-20T14:51:46.152Z
Learning: In shesha-reactjs model configurator reducer (src/components/modelConfigurator/propertiesEditor/provider/reducer.ts), when handling ArrayFormats.childObjects in the UpdateItem action, itemsType.dataFormat should be set to undefined (not ObjectFormats.object). This is the correct logic for child object array configuration.
Applied to files:
shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
📚 Learning: 2025-11-05T07:46:22.081Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/components/layerEditor/modal.tsx:55-57
Timestamp: 2025-11-05T07:46:22.081Z
Learning: In shesha-reactjs layer editor modal (src/components/layerEditor/modal.tsx), the useDeepCompareEffect that syncs items to the parent via onChange should only depend on [items], not [items, onChange]. Adding onChange to the dependency array causes the effect to fire on every parent render and breaks functionality. The guard for optional onChange is needed but the function reference should not be in the dependency array.
Applied to files:
shesha-reactjs/src/components/autocomplete/index.tsx
📚 Learning: 2025-12-11T11:57:48.398Z
Learnt from: czwe-01
Repo: shesha-io/shesha-framework PR: 4349
File: shesha-reactjs/src/designer-components/columns/styles.ts:3-14
Timestamp: 2025-12-11T11:57:48.398Z
Learning: In the shesha-framework's shesha-reactjs project, when using createStyles to define style hooks (e.g., export const useStyles = createStyles(...)), return an object from the callback with named keys (e.g., { columns }) and access them in components as const { styles } = useStyles(); then read properties via styles.columns. The wrapping behavior is provided by createStyles and should be followed consistently. This pattern applies to style modules under shesha-reactjs/src/designer-components (and similar styling files).
Applied to files:
shesha-reactjs/src/designer-components/slider/interfaces.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-attempt
🔇 Additional comments (2)
shesha-reactjs/src/components/autocomplete/index.tsx (1)
107-166: Dependency trimming on values-loading effect: verify no stale closures forkeys/filter funcs/source changes.The reduced dependency list is good for preventing effect storms, but the body reads
keys,filterKeysFunc,keyValueFunc,outcomeValueFunc,source, andloadingValueswithout listing them. Please sanity-test these scenarios to ensure the effect still refreshes correctly:
- switching
keyPropName/displayPropName/dataSourceUrlat runtime- changing
filterKeysFunc/keyValueFuncvia props- switching between URL and entity data sources while a fetch is in-flight
If any of those are supported flows, the effect may need either additional deps or local “snapshot” variables to avoid stale reads.
shesha-reactjs/src/designer-components/slider/index.tsx (1)
20-20: Confirm slider should be int-only (int32/int64) vs supporting decimals.
dataTypeSupportednow rejects non-int number formats. If sliders are expected to work fordecimal/doublefields too, this will block valid use-cases and should be widened accordingly.
| const displayValueFunc: OutcomeValueFunc = useCallback((value: any, args: any) => { | ||
| if (model.displayValueFunc) | ||
| return executeExpression(model.displayValueFunc, { ...args, item: value }, null, null); | ||
| return getValueByPropertyName(value, displayPropName) || ''; | ||
| return (typeof (value) === 'object' ? getValueByPropertyName(value, displayPropName) : value) || ''; | ||
| }, [model.displayValueFunc, displayPropName]); |
There was a problem hiding this comment.
Fix falsy primitive handling + null object check in displayValueFunc (0/false become blank today).
(… : value) || '' will turn valid values like 0 / false into '', and typeof value === 'object' also matches null.
- const displayValueFunc: OutcomeValueFunc = useCallback((value: any, args: any) => {
+ const displayValueFunc: OutcomeValueFunc = useCallback((value: unknown, args: unknown) => {
if (model.displayValueFunc)
- return executeExpression(model.displayValueFunc, { ...args, item: value }, null, null);
- return (typeof (value) === 'object' ? getValueByPropertyName(value, displayPropName) : value) || '';
+ return executeExpression(model.displayValueFunc, { ...(args as object), item: value }, null, null);
+
+ const raw =
+ value != null && typeof value === 'object'
+ ? getValueByPropertyName(value as Record<string, unknown>, displayPropName)
+ : value;
+
+ return raw == null ? '' : String(raw);
}, [model.displayValueFunc, displayPropName]);🤖 Prompt for AI Agents
In shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx around
lines 72 to 76, the current displayValueFunc treats falsy primitives like 0 and
false as empty strings and also mis-handles null as an object; update the
function so it only falls back to '' for null or undefined, explicitly check
value !== null before using typeof value === 'object', and return primitive
falsy values (0, false) unchanged; keep the model.displayValueFunc branch as-is
and only change the fallback logic to first test value === null || value ===
undefined then use getValueByPropertyName for non-null objects otherwise return
the primitive value.
| min={model.min} | ||
| max={model.max} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd shesha-reactjs && find . -name "slider" -type dRepository: shesha-io/shesha-framework
Length of output: 134
🏁 Script executed:
cat -n src/designer-components/slider/index.tsxRepository: shesha-io/shesha-framework
Length of output: 142
🏁 Script executed:
rg "antd.*Slider|from.*Slider" src/designer-components/slider/ -A 2Repository: shesha-io/shesha-framework
Length of output: 141
🏁 Script executed:
rg "ISliderComponentProps" src/designer-components/slider/ -B 2 -A 5Repository: shesha-io/shesha-framework
Length of output: 141
🏁 Script executed:
git ls-files | head -20Repository: shesha-io/shesha-framework
Length of output: 553
🏁 Script executed:
fd -name "slider" -o -name "*slider*" | grep -E "\.(ts|tsx)$"Repository: shesha-io/shesha-framework
Length of output: 300
🏁 Script executed:
find . -name "*slider*" -type f 2>/dev/null | head -20Repository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
ls -la | grep sheshaRepository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
find . -type f -name "*slider*" 2>/dev/nullRepository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
fd "slider" --type fRepository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
ls -laRepository: shesha-io/shesha-framework
Length of output: 294
🏁 Script executed:
find . -name "*.tsx" -path "*/designer-components/*" 2>/dev/null | head -10Repository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
git submodule statusRepository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
git config --file .gitmodules --name-only --get-regexp pathRepository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
git ls-files | wc -lRepository: shesha-io/shesha-framework
Length of output: 74
🏁 Script executed:
git ls-files | grep -i sliderRepository: shesha-io/shesha-framework
Length of output: 303
🏁 Script executed:
git ls-files | grep "designer-components" | head -20Repository: shesha-io/shesha-framework
Length of output: 1655
🏁 Script executed:
git ls-files | grep "shesha-reactjs" | head -20Repository: shesha-io/shesha-framework
Length of output: 792
🏁 Script executed:
git show HEAD:shesha-reactjs/src/designer-components/slider/index.tsxRepository: shesha-io/shesha-framework
Length of output: 2365
🏁 Script executed:
git show HEAD:shesha-reactjs/src/designer-components/slider/interfaces.tsRepository: shesha-io/shesha-framework
Length of output: 510
🏁 Script executed:
git show HEAD:package.json | grep -A 5 -B 5 "antd"Repository: shesha-io/shesha-framework
Length of output: 52
🏁 Script executed:
rg "parseInt.*min\|parseInt.*max" shesha-reactjs/Repository: shesha-io/shesha-framework
Length of output: 125
🏁 Script executed:
rg "Number.isFinite\|Number(" shesha-reactjs/src/designer-components/ -c | head -10Repository: shesha-io/shesha-framework
Length of output: 168
🏁 Script executed:
cat shesha-reactjs/package.json | grep antdRepository: shesha-io/shesha-framework
Length of output: 129
🏁 Script executed:
git log --oneline -1Repository: shesha-io/shesha-framework
Length of output: 207
🌐 Web query:
antd Slider component min max prop types TypeScript
💡 Result:
-
Slider prop types (TypeScript): min: number; max: number. [Ant Design API]. [1]
-
Value-related types: when range is false value/defaultValue/type is number; when range is true it is [number, number]. [1][2]
-
Defaults: min = 0, max = 100 (unless overridden). [1]
Sources:
[1] Ant Design Slider API (docs) — shows min/max types and defaults.
[2] Ant Design Slider docs — describes value as number vs [number, number].
Harden min/max migration + runtime type safety (antd Slider expects numbers).
The migrator's use of parseInt() can yield NaN for empty strings or non-numeric values (e.g., parseInt("", 10) → NaN; parseInt("invalid", 10) → NaN), and passing NaN to antd's Slider component—which expects number type for min/max—will cause inconsistent behavior.
Suggested migrator fix:
migrator: (m) =>
m.add<ISliderComponentPropsV0>(0, (prev) => ({ ...prev }))
.add<ISliderComponentProps>(1, (prev) => ({
...prev,
- min: prev?.min ? parseInt(prev.min, 10) : undefined,
- max: prev?.max ? parseInt(prev.max, 10) : undefined,
+ min: prev?.min != null && prev.min !== '' && Number.isFinite(Number(prev.min)) ? Number(prev.min) : undefined,
+ max: prev?.max != null && prev.max !== '' && Number.isFinite(Number(prev.max)) ? Number(prev.max) : undefined,
})),Optional runtime guard at lines 31-32:
- min={model.min}
- max={model.max}
+ min={Number.isFinite(model.min) ? model.min : undefined}
+ max={Number.isFinite(model.max) ? model.max : undefined}Also applies to lines 51-59.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| min={model.min} | |
| max={model.max} | |
| min={Number.isFinite(model.min) ? model.min : undefined} | |
| max={Number.isFinite(model.max) ? model.max : undefined} |
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)
shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx (1)
85-90: Consider defensive guard formodel.filterKeysFuncinside the callback.You only pass
filterKeysFuncwhenmodel.filterKeysFuncis truthy (Line 115), but adding an internal guard avoids edge cases during rapid prop changes / refactors:const filterKeysFunc: FilterSelectedFunc = useCallback((value: unknown) => { + if (!model.filterKeysFunc) return {}; const localValue = Array.isArray(value) && value?.length === 1 ? value[0] : value; return Array.isArray(localValue) ? { or: localValue.map((x) => executeExpression(model.filterKeysFunc, { value: x }, null, null)) } : executeExpression(model.filterKeysFunc, { value: localValue }, null, null); }, [model.filterKeysFunc]);
♻️ Duplicate comments (2)
shesha-reactjs/src/designer-components/slider/index.tsx (1)
53-59: Migrator still lacks NaN guards; invalid legacy data will break Slider.The migrator's use of
parseInt()without validation will convert invalid strings toNaN(e.g.,parseInt("invalid", 10)→NaN), which will then be passed to antd'sSlidercomponent at lines 31-32. This causes inconsistent behavior and potential UI breakage for users with malformed legacy data.Apply this diff to harden the migrator:
migrator: (m) => m.add<ISliderComponentPropsV0>(0, (prev) => ({ ...prev })) .add<ISliderComponentProps>(1, (prev) => ({ ...prev, - min: prev?.min && prev.min !== '' ? parseInt(prev.min, 10) : undefined, - max: prev?.max && prev.max !== '' ? parseInt(prev.max, 10) : undefined, + min: prev?.min != null && prev.min !== '' && Number.isFinite(Number(prev.min)) ? Number(prev.min) : undefined, + max: prev?.max != null && prev.max !== '' && Number.isFinite(Number(prev.max)) ? Number(prev.max) : undefined, })),Additionally, consider runtime guards at lines 31-32 as a defensive measure:
- min={model.min} - max={model.max} + min={Number.isFinite(model.min) ? model.min : undefined} + max={Number.isFinite(model.max) ? model.max : undefined}shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx (1)
78-83:displayValueFuncstill drops valid falsy values (0/false) and is typed too loosely.This is the same issue previously raised:
(...) || ''will still turn0/falseinto''. Also,displayValueFuncis declared asOutcomeValueFuncand can currently return non-strings (earlyreturn value;), which is likely not what the Autocomplete display pipeline expects.Suggested diff:
-import { FilterSelectedFunc, KayValueFunc, OutcomeValueFunc } from '@/components/autocomplete/models'; +import { DisplayValueFunc, FilterSelectedFunc, KayValueFunc, OutcomeValueFunc } from '@/components/autocomplete/models'; - const displayValueFunc: OutcomeValueFunc = useCallback((value: unknown, args: object) => { - if (!isDefined(value)) return value; + const displayValueFunc: DisplayValueFunc = useCallback((value: unknown, args: object) => { + if (!isDefined(value)) return ''; if (model.displayValueFunc) return executeExpression(model.displayValueFunc, { ...args, item: value }, null, null); - return (typeof (value) === 'object' ? getValueByPropertyName(value as Record<string, unknown>, displayPropName) : value) || ''; + const raw = + value != null && typeof value === 'object' && !Array.isArray(value) + ? getValueByPropertyName(value as Record<string, unknown>, displayPropName) + : value; + return raw == null ? '' : String(raw); }, [model.displayValueFunc, displayPropName]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
shesha-reactjs/src/components/autocomplete/models.tsx(1 hunks)shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx(2 hunks)shesha-reactjs/src/designer-components/slider/index.tsx(3 hunks)shesha-reactjs/src/designer-components/slider/interfaces.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)
**/*.{ts,tsx}: Eliminate theanytype; useunknowntype instead for values with unknown types, forcing explicit type checking
Prefer type guards over type casting for type checking
Avoid monolithic types; use discriminated unions with a discriminator property instead
Leverage TypeScript to its full potential as a type system, not merely as a linter
Files:
shesha-reactjs/src/designer-components/slider/index.tsxshesha-reactjs/src/designer-components/slider/interfaces.tsshesha-reactjs/src/components/autocomplete/models.tsxshesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
🧠 Learnings (6)
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/designer-components/slider/index.tsxshesha-reactjs/src/designer-components/slider/interfaces.tsshesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*{Component,Editor}.{ts,tsx} : Remove the `defaultValue` pattern entirely from controlled editor components
Applied to files:
shesha-reactjs/src/designer-components/slider/interfaces.ts
📚 Learning: 2025-12-11T11:57:48.398Z
Learnt from: czwe-01
Repo: shesha-io/shesha-framework PR: 4349
File: shesha-reactjs/src/designer-components/columns/styles.ts:3-14
Timestamp: 2025-12-11T11:57:48.398Z
Learning: In the shesha-framework's shesha-reactjs project, when using createStyles to define style hooks (e.g., export const useStyles = createStyles(...)), return an object from the callback with named keys (e.g., { columns }) and access them in components as const { styles } = useStyles(); then read properties via styles.columns. The wrapping behavior is provided by createStyles and should be followed consistently. This pattern applies to style modules under shesha-reactjs/src/designer-components (and similar styling files).
Applied to files:
shesha-reactjs/src/designer-components/slider/interfaces.ts
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Eliminate the `any` type; use `unknown` type instead for values with unknown types, forcing explicit type checking
Applied to files:
shesha-reactjs/src/components/autocomplete/models.tsxshesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
📚 Learning: 2025-11-20T14:51:46.152Z
Learnt from: AlexStepantsov
Repo: shesha-io/shesha-framework PR: 4235
File: shesha-reactjs/src/components/modelConfigurator/propertiesEditor/provider/reducer.ts:149-149
Timestamp: 2025-11-20T14:51:46.152Z
Learning: In shesha-reactjs model configurator reducer (src/components/modelConfigurator/propertiesEditor/provider/reducer.ts), when handling ArrayFormats.childObjects in the UpdateItem action, itemsType.dataFormat should be set to undefined (not ObjectFormats.object). This is the correct logic for child object array configuration.
Applied to files:
shesha-reactjs/src/components/autocomplete/models.tsxshesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Avoid monolithic types; use discriminated unions with a discriminator property instead
Applied to files:
shesha-reactjs/src/components/autocomplete/models.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-attempt
🔇 Additional comments (2)
shesha-reactjs/src/designer-components/slider/interfaces.ts (1)
4-15: Deprecation markers properly applied.The addition of deprecation markers at both the interface and property levels successfully addresses the previous review feedback. The clear distinction between legacy string-based (
ISliderComponentPropsV0) and current numeric (ISliderComponentProps) types will help prevent misuse of the legacy interface.shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx (1)
52-76: Harden null/object checks + remove redundant optional chaining in entityReference branches.
keyValueFunc: afterisEntityReferenceId(value)you can usevalue.id(optional chaining is redundant) and avoid treating arrays/functions as objects forgetValueByPropertyName.outcomeValueFunc:item?.toString()can be simplified toString(item)in the primitive-id fallback (you already guarded withisDefined).Suggested improvements (minimal scope):
- Remove optional chaining on
value.idat line 56 ifisEntityReferenceIdis a proper type-guard- Change
item?.toString()toString(item)at line 70- Consider adding
!Array.isArray(value)to the object-type checks at lines 57 and 75 to exclude arrays fromgetValueByPropertyName
… selected value is shown as id after saving query builder)
AlexStepantsov
force-pushed
the
alexs/fix
branch
from
6a25b6e to
0fc38d0
Compare
…property to the form
AlexStepantsov
force-pushed
the
alexs/fix
branch
from
0fc38d0 to
5b4e496
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx (1)
80-85: Return type mismatch: returningundefined/nullwhen function expectsstring.Line 81 returns
value(which would beundefinedornullwhen!isDefined(value)is true), butDisplayValueFuncsignature specifies return typestring. This creates a type inconsistency.const displayValueFunc: DisplayValueFunc = useCallback((value: unknown, args: object) => { - if (!isDefined(value)) return value; + if (!isDefined(value)) return ''; if (model.displayValueFunc) return executeExpression(model.displayValueFunc, { ...args, item: value }, null, null); return (typeof (value) === 'object' ? getValueByPropertyName(value as Record<string, unknown>, displayPropName) : value) || ''; }, [model.displayValueFunc, displayPropName]);Additionally, the
|| ''fallback at line 84 treats falsy primitives like0orfalseas empty strings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
shesha-reactjs/src/components/autocomplete/models.tsx(1 hunks)shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx(5 hunks)shesha-reactjs/src/designer-components/editableTagGroup/index.tsx(2 hunks)shesha-reactjs/src/designer-components/entityPicker/index.tsx(3 hunks)shesha-reactjs/src/designer-components/radio/radio.tsx(2 hunks)shesha-reactjs/src/interfaces/metadata.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)
**/*.{ts,tsx}: Eliminate theanytype; useunknowntype instead for values with unknown types, forcing explicit type checking
Prefer type guards over type casting for type checking
Avoid monolithic types; use discriminated unions with a discriminator property instead
Leverage TypeScript to its full potential as a type system, not merely as a linter
Files:
shesha-reactjs/src/designer-components/editableTagGroup/index.tsxshesha-reactjs/src/interfaces/metadata.tsshesha-reactjs/src/designer-components/radio/radio.tsxshesha-reactjs/src/components/autocomplete/models.tsxshesha-reactjs/src/designer-components/entityPicker/index.tsxshesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
🧠 Learnings (7)
📚 Learning: 2025-11-20T14:51:46.152Z
Learnt from: AlexStepantsov
Repo: shesha-io/shesha-framework PR: 4235
File: shesha-reactjs/src/components/modelConfigurator/propertiesEditor/provider/reducer.ts:149-149
Timestamp: 2025-11-20T14:51:46.152Z
Learning: In shesha-reactjs model configurator reducer (src/components/modelConfigurator/propertiesEditor/provider/reducer.ts), when handling ArrayFormats.childObjects in the UpdateItem action, itemsType.dataFormat should be set to undefined (not ObjectFormats.object). This is the correct logic for child object array configuration.
Applied to files:
shesha-reactjs/src/designer-components/editableTagGroup/index.tsxshesha-reactjs/src/interfaces/metadata.tsshesha-reactjs/src/designer-components/radio/radio.tsxshesha-reactjs/src/components/autocomplete/models.tsxshesha-reactjs/src/designer-components/entityPicker/index.tsxshesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
📚 Learning: 2025-11-05T07:46:22.081Z
Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/components/layerEditor/modal.tsx:55-57
Timestamp: 2025-11-05T07:46:22.081Z
Learning: In shesha-reactjs layer editor modal (src/components/layerEditor/modal.tsx), the useDeepCompareEffect that syncs items to the parent via onChange should only depend on [items], not [items, onChange]. Adding onChange to the dependency array causes the effect to fire on every parent render and breaks functionality. The guard for optional onChange is needed but the function reference should not be in the dependency array.
Applied to files:
shesha-reactjs/src/designer-components/editableTagGroup/index.tsxshesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Avoid monolithic types; use discriminated unions with a discriminator property instead
Applied to files:
shesha-reactjs/src/designer-components/editableTagGroup/index.tsxshesha-reactjs/src/components/autocomplete/models.tsxshesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
📚 Learning: 2025-06-26T20:01:48.838Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3461
File: shesha-reactjs/src/designer-components/charts/settingsFormIndividual.ts:171-192
Timestamp: 2025-06-26T20:01:48.838Z
Learning: In shesha-reactjs chart settings forms, duplicate input configurations may be intentionally included as part of UI/UX experiments and should be flagged for review during UI/UX review phases rather than immediately removed as errors.
Applied to files:
shesha-reactjs/src/designer-components/editableTagGroup/index.tsxshesha-reactjs/src/designer-components/radio/radio.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Eliminate the `any` type; use `unknown` type instead for values with unknown types, forcing explicit type checking
Applied to files:
shesha-reactjs/src/components/autocomplete/models.tsxshesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
📚 Learning: 2025-11-25T07:17:44.449Z
Learnt from: CR
Repo: shesha-io/shesha-framework PR: 0
File: CodeRabbitReview-Project-Specific-Guidelines.md:0-0
Timestamp: 2025-11-25T07:17:44.449Z
Learning: Applies to **/*.{ts,tsx} : Leverage TypeScript to its full potential as a type system, not merely as a linter
Applied to files:
shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
📚 Learning: 2025-06-12T16:55:57.638Z
Learnt from: teboho
Repo: shesha-io/shesha-framework PR: 3397
File: shesha-reactjs/src/designer-components/charts/bar.tsx:49-52
Timestamp: 2025-06-12T16:55:57.638Z
Learning: For the chart components’ migrators (e.g., BarChartComponent in shesha-reactjs/src/designer-components/charts/bar.tsx), the version 5 step intentionally spreads `{ ...prev, ...defaultConfigFiller }` so that values from `defaultConfigFiller` override any existing properties in `prev`. This reset to new defaults is by design and should not be flagged as an issue.
Applied to files:
shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-attempt
🔇 Additional comments (12)
shesha-reactjs/src/interfaces/metadata.ts (1)
2-2: LGTM: Import addition is correct.The
ArrayFormatsimport is necessary for the enhanced type guard on line 142.shesha-reactjs/src/components/autocomplete/models.tsx (1)
30-33: Good improvement usingunknowninstead ofanyfor input types.These type changes enforce explicit type checking at call sites, aligning with TypeScript best practices.
shesha-reactjs/src/designer-components/autocomplete/autocomplete.tsx (3)
37-39: Good extension for array entity reference support.The expanded
dataTypeSupportedcorrectly identifies both single entity references and array formats, enabling proper component selection in the form designer.
87-92: LGTM!The array normalization and conditional expression evaluation handle both single values and arrays appropriately.
193-197: Correct metadata extraction for entity reference arrays.Using
itemsType?.entityTypeanditemsType?.entityModuleproperly accesses the nested entity type information for array properties, fixing the drag-and-drop entity type preconfiguration issue mentioned in PR objectives.shesha-reactjs/src/designer-components/entityPicker/index.tsx (4)
6-6: LGTM!The
ArrayFormatsimport is correctly added to support the expandeddataTypeSupportedlogic.
60-62: LGTM!The expanded
dataTypeSupportedlogic correctly handles both single entity references and array-based entity references (entityReferenceandmanyToManyEntitiesformats). Theincludes()check safely handles cases wheredataFormatmight be undefined.
230-232: LGTM!The
valueFormatlogic correctly applies'entityReference'for both single and array entity reference types, maintaining consistency with themodedetermination above.
225-229: Add explicit validation to ensureitemsType.entityTypeis defined.The type guard
isEntityReferenceArrayPropertyMetadataonly checksdataTypeanddataFormat, not whetheritemsTypeoritemsType.entityTypeactually exist at runtime. At line 228, using optional chaining onpropMetadata.itemsType?.entityTypesilently produces{ name: undefined, module: ... }, which could cause downstream issues. Either strengthen the type guard to validate these properties or add explicit checks before accessing them.shesha-reactjs/src/designer-components/radio/radio.tsx (2)
5-5: LGTM!The addition of
ArrayFormatsto the import is necessary for the enhanced data type checking logic.
27-27: No explicit typing needed; null safety is handled correctly.The
dataTypeSupportedfunction follows the established pattern across all designer components in the codebase. The type is defined at the interface level inshesha-reactjs/src/interfaces/formDesigner.tsas(dataTypeInfo: { dataType: string; dataFormat?: string }) => boolean, wheredataFormatis correctly marked as optional.Regarding null safety: when
dataFormatisundefinedand the code evaluatesdataFormat === ArrayFormats.simple, the result isfalse(sinceundefined !== 'simple'), which is the correct behavior. No additional guard is needed.Since this is a new component creation (not a modification), there is no breaking change to existing radio components.
shesha-reactjs/src/designer-components/editableTagGroup/index.tsx (1)
8-8: LGTM! Import addition is necessary.The
ArrayFormatsimport is required for the enhanced data type validation on line 22.
1 participant
89349 Slider Ignores Property-Level Min/Max When Used as Default Editor
75856 Query builder value isn't saved for Data Context (Autocomplete: selected value is shown as id after saving query builder)
89623 Entity type isn't preconfigured when drag and drop List entity property to the form
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.