fix(shared): fix deepMerge data-corruption bugs and add tests#4136
fix(shared): fix deepMerge data-corruption bugs and add tests#4136kridaydave wants to merge 1 commit into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
250dfdc to
966cd6b
Compare
ApprovabilityVerdict: Needs human review This bug fix introduces meaningful runtime behavior changes to a shared You can customize Macroscope's approvability policy. Learn more. |
Fixes two deepMerge bugs: it destroyed Date/Map/class-instance values (prototype loss) and silently discarded the entire base on a primitive patch.
All gates green (typecheck, check, test).
Note
Medium Risk
Behavior change for invalid primitive patches (now throws) and for non-plain-object values during merge; shared utility so callers relying on old Effect
isObjectrecursion could see different merged shapes.Overview
Fixes data-corruption bugs in shared
deepMergeby only recursing when both the existing value and patch value are plain objects (Object.prototype), via a newisPlainObjecthelper. Date, Map, class instances, arrays, and functions are now replaced whole instead of being field-merged like POJOs.Error handling for invalid patches is tightened: a non-null primitive patch (e.g. a number) now throws instead of silently returning a corrupted result;
nullpatch is still allowed.Adds
Struct.test.tsfor normal merge behavior (immutability, nesting,undefinedskips) andStruct.edgeCases.test.tsto lock in the current contract (array overwrite, null vs undefined, Date/functions, primitive patch throw).Reviewed by Cursor Bugbot for commit 966cd6b. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix data-corruption bugs in
deepMergeand add test coverageisPlainObjectin Struct.ts to restrict recursion to plain objects; dates, maps, and class instances are now overwritten rather than recursively merged.deepMergenow throws when the top-level patch is a non-null primitive (e.g. number, array, function) instead of silently returning it.Macroscope summarized 966cd6b.