use expression on datalist

[HackGenesis]

Summary by CodeRabbit

• Bug Fixes
  • Expression-based forms now derive a unique, form-specific identifier when a form ID is present, ensuring consistent selection and caching across per-record preparation and rendering. This fixes inconsistencies so expression forms behave uniformly in all data list scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

For expression-based forms (formSelectionMode === 'expression'), the code derives the entity type from the form ID via ConfigurableItemIdentifierToString(fId) when a form ID exists, otherwise uses $expressionForm$; this change is applied both during per-record form preparation and in renderSubForm.

Changes

Cohort / File(s)	Summary
Expression Form Selection shesha-reactjs/src/components/dataList/index.tsx	For expression-based forms, derive the entity type from the form ID when present (via ConfigurableItemIdentifierToString(fId)), otherwise use the special entity type $expressionForm$; applied in both per-record form preparation and renderSubForm to ensure a form-specific cache key.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Single-file change with two symmetric edits.
• Review focus:
  • Correct usage of ConfigurableItemIdentifierToString(fId) and null/undefined handling for fId.
  • Confirm cache-key construction and any assumptions about uniqueness.
  • Ensure symmetry between the per-record loop and renderSubForm code paths.

Suggested reviewers

• James-Baloyi

Poem

🐰 I hopped through code where forms would roam,

I found a name to give them, a homely home,
$expressionForm$ or the ID's true song,
Now per-record and sub-forms both belong,
A carrot-coded cheer for changes made strong!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'use expression on datalist' is vague and generic, lacking specificity about what expression-based functionality was changed or why.	Use a more descriptive title that specifies the change, such as 'Fix expression form caching in dataList component' or 'Derive entity type from form ID for expression-based forms in dataList'.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d12900b and bb545e8.

📒 Files selected for processing (1)

• shesha-reactjs/src/components/dataList/index.tsx (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type 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/components/dataList/index.tsx

🧠 Learnings (5)

📚 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/dataList/index.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/dataList/index.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/components/dataList/index.tsx

📚 Learning: 2025-11-05T08:12:08.149Z

Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/providers/layersProvider/index.tsx:35-39
Timestamp: 2025-11-05T08:12:08.149Z
Learning: In shesha-reactjs LayerGroupConfiguratorProvider (src/providers/layersProvider/index.tsx), the provider uses a unidirectional data flow pattern after initialization: props.items provide initial state to the reducer (line 35-38), user actions modify the reducer state internally, and changes sync back to the parent via onChange callbacks in consuming components (e.g., modal.tsx line 55-57 uses useDeepCompareEffect). Do NOT add a useEffect to sync props.items back to the reducer state, as this creates a feedback loop that breaks layer updates in the Calendar UI. The pattern is intentionally "controlled initialization, uncontrolled updates" where the provider manages state internally after the initial mount.

Applied to files:

• shesha-reactjs/src/components/dataList/index.tsx

📚 Learning: 2025-11-05T09:03:47.960Z

Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4132
File: shesha-reactjs/src/components/calendar/configurable-actions/refresh-calendar.tsx:5-18
Timestamp: 2025-11-05T09:03:47.960Z
Learning: In shesha-reactjs calendar configurable actions (src/components/calendar/configurable-actions/refresh-calendar.tsx), the useRefreshCalendarAction hook registers a configurable action with an empty dependency array []. Do NOT add setRefreshTrigger to the dependency array, as it is not stable (re-created when state.refreshTrigger changes) and would cause a registration loop. The executer uses a functional update pattern setRefreshTrigger((prev) => prev + 1) which doesn't need the current closure, so the action only needs to be registered once on mount.
<!--

Applied to files:

• shesha-reactjs/src/components/dataList/index.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 (3)

shesha-reactjs/src/components/dataList/index.tsx (3)

8-8: LGTM: Import added to support form ID string conversion.

The import of ConfigurableItemIdentifierToString is necessary for converting dynamic form IDs to stable string keys for caching, which addresses the critical caching issue identified in previous reviews.

---

399-402: Consistent application of the fix in renderSubForm.

The logic correctly mirrors the approach from the records loop (lines 373-377), ensuring that renderSubForm uses the same form ID derivation and caching strategy. The duplicate call to getFormIdFromExpression(item) is necessary here because renderSubForm can be invoked independently, and the expression may be stateful.

The same verification concerns from the previous comment apply here regarding error handling and type coverage of ConfigurableItemIdentifierToString.

---

373-377: Excellent fix for the critical caching bug, but verification needed on error handling and type coverage.

This change addresses the critical issue where all expression-based forms previously shared the same cache key. By using ConfigurableItemIdentifierToString(fId) to derive unique cache keys per form ID, different items can now render with their correct forms.

However, please verify the following:

1. Error handling: Confirm that ConfigurableItemIdentifierToString handles invalid or unexpected inputs gracefully, or add error handling if needed.
2. Type coverage: Verify that ConfigurableItemIdentifierToString correctly handles all possible FormIdentifier types (string and object variants).
3. Empty/null returns: Confirm behavior when ConfigurableItemIdentifierToString returns an empty string or fails gracefully.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 52019fa and d12900b.

📒 Files selected for processing (1)

• shesha-reactjs/src/components/dataList/index.tsx (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CodeRabbitReview-Project-Specific-Guidelines.md)

**/*.{ts,tsx}: Eliminate the any type; use unknown type 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/components/dataList/index.tsx

🧠 Learnings (3)

📚 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/dataList/index.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/dataList/index.tsx

📚 Learning: 2025-10-21T10:57:14.087Z

Learnt from: Lihlu
Repo: shesha-io/shesha-framework PR: 4027
File: shesha-reactjs/src/components/calendar/index.tsx:76-87
Timestamp: 2025-10-21T10:57:14.087Z
Learning: In shesha-reactjs calendar components, when using executeScript with allData to evaluate external date expressions, allData must be included in the useEffect dependency array because the scripts may reference form data or global state that changes over time, and the dates need to be re-evaluated when that context changes.

Applied to files:

• shesha-reactjs/src/components/dataList/index.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