discussions store: updateNote uses deep merge where splice used full replacement

Context

In !226171 (merged), the UPDATE_NOTE mutation in app/assets/javascripts/notes/store/legacy_notes/mutations.js was changed from using splice (full object replacement) to delegating to useDiscussions().updateNote(), which uses lodash merge (deep merge into the existing object reference).

This was done to preserve reactive properties (isEditing, editedNote) that were being lost when splice replaced note objects wholesale.

Concern

splice and merge have different semantics for property removal:

  • splice replaced the entire note object. If the server response omitted a property that was previously present, that property was dropped.
  • merge deep-merges into the existing object. If the server response omits a property, the stale value is preserved on the existing object.

This is not currently causing any known bugs, but could be a subtle source of issues in the future if the API response shape for notes ever changes to intentionally omit previously-present properties.

An alternative like Object.assign(existingNote, note) would do a shallow replacement of all top-level keys on the same object reference (preserving reactive props added at the top level) while more closely matching the old replacement semantics.

Original discussion

  • @thomasrandolph started a discussion:

    The old splice did full object replacement; merge does deep merge. If the server response ever omits a property that was previously present on a note, splice would have dropped it, but merge preserves the stale value on the existing object. Would something like Object.assign(existingNote, note) better match the old replacement semantics while still keeping the same object reference (and its reactive props)?

    ❔ What is this
    Type Question
    Blocker No
    Tags correctness, semantics
    Concern Level 1-10 3
Edited Mar 06, 2026 by 🤖 GitLab Bot 🤖
Assignee Loading
Time tracking Loading