fix(WidgetOperation): avoid empty string for missing list widget container - #42043
fix(WidgetOperation): avoid empty string for missing list widget container#42043PedroHenrique0713 wants to merge 4 commits into
Conversation
When evaluation produced an empty tree, parsedUpdates[0] was undefined and accessing .rhs threw TypeError. Now checks .length before access.
… widget children When a ListWidget had no children, mainContainerId was set to empty string '' instead of undefined. Downstream code treating '' as a valid widget ID produced silent failures.
WalkthroughThe changes add an empty-update guard in evaluation, document swallowed form-expression errors and JSON cloning behavior, and change the ListWidgetV2 container fallback from an empty string to ChangesEvaluation and binding safeguards
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 Warning |
![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
🤖 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 `@app/client/src/workers/Evaluation/helpers.ts`:
- Around line 114-116: Update the JSON round-trip comment near fnStrings to
accurately describe behavior: Dates become strings, undefined object properties
are omitted while array elements become null, non-JSON-native types may
serialize as empty objects, and functions collected via fnValue.toString() are
reinserted as strings rather than callable values.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: eeca8e1b-eb5d-40e6-80f5-95fe6818a37f
📒 Files selected for processing (4)
app/client/src/sagas/WidgetOperationUtils.tsapp/client/src/workers/Evaluation/formEval.tsapp/client/src/workers/Evaluation/handlers/evalTree.tsapp/client/src/workers/Evaluation/helpers.ts
| // Note: JSON round-trip strips Dates, Sets, Maps, RegExps, and undefined values. | ||
| // Functions are preserved (collected before, re-injected after), but other | ||
| // non-JSON-safe types may be lost. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Repository files matching helpers:\n'
fd -a 'helpers(\.test)?\.(ts|tsx)$' . | sed 's#^\./##' | rg 'app/client/src/workers/Evaluation/helpers' || true
printf '\nRelevant helper lines:\n'
sed -n '100,130p' app/client/src/workers/Evaluation/helpers.ts || true
printf '\nRelevant tests outline/lines:\n'
wc -l app/client/src/workers/Evaluation/helpers.test.ts
rg -n "JSON|function|Functions|helper|toJSON|call|()" app/client/src/workers/Evaluation/helpers.test.ts -C 2 || true
rg -n "collect|functions|JSON\.stringify|JSON\.parse|String(Replaces|Map)|inject|invoke" app/client/src/workers/Evaluation/helpers.ts -C 2 || true
printf '\nBehavioral JS probe for JSON stringify/parse semantics:\n'
node - <<'JS'
const cases = {
date: new Date('2026-07-01T12:34:56.789Z'),
undefInObj: {a: undefined, b: 1},
undefInArr: [1, undefined, 2],
map: new Map([['x', 1]]),
set: new Set([1, 2]),
regexp: /abc/,
func: function named(){ return 42; },
object: {a: undefined, b: [undefined]},
sparse: [undefined],
};
for (const [name, value] of Object.entries(cases)) {
const serialized = JSON.stringify(value);
const rounds = JSON.parse(serialized);
console.log(name + ': ' + serialized + ' -> ' + JSON.stringify(rounds) + ' [typeof: ' + typeof rounds + ']');
}
JSRepository: appsmithorg/appsmith
Length of output: 10390
Correct the JSON round-trip behavior comment.
Date values become strings, object properties with undefined are omitted while array elements become null, and non-JSON-native types serialize as empty objects rather than being simply “stripped”. Also, fnStrings stores fnValue.toString(), so functions are reinserted as strings, not preserved as callable values.
Suggested wording
- // Note: JSON round-trip strips Dates, Sets, Maps, RegExps, and undefined values.
- // Functions are preserved (collected before, re-injected after), but other
- // non-JSON-safe types may be lost.
+ // Note: JSON round-trip converts Dates to strings and does not preserve
+ // undefined, Maps, Sets, RegExps, or other non-JSON-safe values.
+ // Functions are reinserted as their string representations.📝 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.
| // Note: JSON round-trip strips Dates, Sets, Maps, RegExps, and undefined values. | |
| // Functions are preserved (collected before, re-injected after), but other | |
| // non-JSON-safe types may be lost. | |
| // Note: JSON round-trip converts Dates to strings and does not preserve | |
| // undefined, Maps, Sets, RegExps, or other non-JSON-safe values. | |
| // Functions are reinserted as their string representations. |
🤖 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 `@app/client/src/workers/Evaluation/helpers.ts` around lines 114 - 116, Update
the JSON round-trip comment near fnStrings to accurately describe behavior:
Dates become strings, undefined object properties are omitted while array
elements become null, non-JSON-native types may serialize as empty objects, and
functions collected via fnValue.toString() are reinserted as strings rather than
callable values.
|
Closing this one. I opened a stack of PRs in this repo today and, because each branch was cut from the previous one instead of from the base, they overlap: this PR carries the commits of the earlier ones as well. I am consolidating the work in #42033 and will resubmit the remaining fixes individually, on top of the base branch, once that one has been reviewed. Sorry for the noise. |
1 participant
Empty string as widget container ID causes silent failures downstream.
Summary by CodeRabbit
Bug Fixes
Documentation