docs(helpers): document non-JSON type loss in stringifyFnsInObject - #42040
docs(helpers): document non-JSON type loss in stringifyFnsInObject#42040PedroHenrique0713 wants to merge 1 commit into
Conversation
WalkthroughChangesSerialization Documentation
Estimated code review effort: 1 (Trivial) | ~2 minutes 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 |
![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: Correct the conversion note near the JSON round-trip to
accurately describe JSON.stringify behavior: Dates become strings,
Set/Map/RegExp values generally become empty objects, and undefined object
properties are omitted. Clarify that collected functions are re-injected as
source strings rather than restored callable functions; do not claim they are
preserved as functions.
🪄 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: 06ff42ff-3746-4bdb-81c9-f183b1379603
📒 Files selected for processing (1)
app/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.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Document conversion and function stringification precisely.
JSON.stringify converts Date values to strings rather than stripping them, while Set, Map, and RegExp typically serialize as {} and undefined object properties are omitted. Also, lines 119-120 re-inject function source strings—not callable functions—so “functions are preserved” overstates the contract.
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, changes or drops other
+ // non-JSON-safe values, and omits undefined object properties.
+ // Functions are replaced with source strings and re-injected at their paths;
+ // they are not restored as callable functions.📝 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, changes or drops other | |
| // non-JSON-safe values, and omits undefined object properties. | |
| // Functions are replaced with source strings and re-injected at their paths; | |
| // they are not restored as callable functions. |
🤖 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, Correct
the conversion note near the JSON round-trip to accurately describe
JSON.stringify behavior: Dates become strings, Set/Map/RegExp values generally
become empty objects, and undefined object properties are omitted. Clarify that
collected functions are re-injected as source strings rather than restored
callable functions; do not claim they are preserved as functions.
|
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
Summary
stringifyFnsInObjectusesJSON.parse(JSON.stringify(userObject))for its serialization round-trip. This stripsDate,Set,Map,RegExp, andundefinedvalues — all of which become{}ornullsilently.Functions are preserved (collected before serialization, re-injected after), but any other non-JSON-safe types in the user's data tree are silently corrupted with no warning.
Fix: Added documentation comment to warn callers about this behavior.
Summary by CodeRabbit