fix: ObservableSet.replace only emits events for actual changes

[chatman-media]

Closes #3761

Problem

ObservableSet.replace(...) was implemented as clear() followed by re-adding every value of the source. As a result it always emitted a delete event for every existing element and an add event for every replacement element — even for elements that were unchanged — and unnecessarily retriggered reactions.

import { observable, configure } from "mobx"
configure({ enforceActions: "never" })

const s = observable.set(["a", "b", "c"])
s.observe(console.log)

// Before: delete a, delete b, delete c, add b, add c, add d  (6 events)
// After:  delete a, add d                                     (2 events)
s.replace(["b", "c", "d"])

// Before: delete x, delete y, add x, add y  (4 events)
// After:  (no events)
observable.set(["x", "y"]).replace(["x", "y"])

This mirrors the long-standing complaint that ObservableMap.replace already handles correctly (see #1243 / #1980 in the map tests).

Fix

replace now:

• deletes only the values that are not part of the replacement, and
• adds only the values from the replacement (which are no-ops when already present).

add/delete already short-circuit (and emit no event / no reportChanged) when a value is respectively already present or already absent, so this is sufficient to make replace emit events only for genuine changes. The approach matches ObservableMap.replace.

Tests

Added a #3761 describe block in set.js covering:

• replace with array / ES6 Set / observable Set only emits events for removed & added values
• replace with identical content emits no events and does not report a change to reactions
• interceptors are still honored during replace

These tests fail on main and pass with the fix. The existing set crud test (which exercises replace with disjoint sets) still passes unchanged. Added a changeset (patch).

[changeset-bot]

🦋 Changeset detected

Latest commit: 57c170d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
mobx	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[mweststrate]

Thanks for making this improvement! I left some notes how the implementation can be slightly improved, but overall looks solid to me!

[mweststrate]

This generally looks great! Note that this seems to be making a semantic change:

• Sets to preserve insertion order (per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Set#description), the old behavior would preserve the insertion order of the new set (e.g. {a,b,c}.replace({d,b,a}) would result in a set that is iterated like d,b,a. With this change, the iteration would become a,b,d. Now I think a good argument could be made that is semantically more accurate (and thus a bug fix), but it is a observable change of behavior, so we should mention it clearly in the release notes
• The new behavior now does to two iterations (first loop the old set to delete items, then loop the new set to add items), so this is likely a bit slower. I think this is fine. But it would be great to short-circuit the scenario where either the this or replacement is empty.

[mweststrate]

if other is already a Set, let's not allocate another one since it isn't mutated anyway.

[mweststrate]

if you'd reorder the items in this array it would verify the change in behavior is discussed below

[mweststrate]

Same

[chatman-media]

Thanks for the thorough review! Addressed the notes in 57c170d:

• Iteration order — agreed this is an observable change (and, as you say, arguably the more correct one). I did not try to preserve the old ordering; surviving values keep their original position and new values are appended. Documented this clearly in the changeset, with the {a,b,c}.replace({d,b,a}) -> a,b,d example.
• Two iterations / short-circuit — added fast paths: an empty replacement is just a clear(), and replacing into an empty set only runs the adds, so the common trivial cases skip the second loop entirely.
• Avoid the extra Set allocation — when other is already a Set it is now reused directly for the membership checks (observable sets are snapshotted earlier, and the Set is only read, never mutated). Arrays are still wrapped so they get deduped.
• Tests — reordered the array and ES6-Set replacements so they assert the resulting iteration order, which exercises the documented behavior change. The existing set crud test still passes unchanged.

[chatman-media]

Thanks for the review, @mweststrate. Addressed in 57c170d:

• documented the observable iteration-order change in the changeset
• reused existing Set inputs instead of allocating another Set
• added fast paths for empty replacement / empty current set
• updated the tests to lock in the new order semantics

Full set.js suite is green locally. Happy to adjust the order semantics if you'd prefer matching the replacement order instead.