0

This is an odd one... I have a singleton (providedIn: "root") service that has a BehaviorSubject, that is private, but exposed as an observable:

private readonly _caseData$ = new BehaviorSubject<CaseData | null>(null);
Context = this.caseData$.asObservable();

context gets set by another function which calls next on the subject and then values are retrieved off of Context.

In a component I have a function that takes a value, and pipes off of Context removes some values from an array on CaseData and calls another function in the service:

unassign = (tag: Tag) => {
  this.caseContext.Context.pipe(
    switchMap((caseData) => {
      const index = caseData.tags.findIndex(tagId => tagId === tag.tagId);
      if (index > -1) {
        caseData.tags.splice(index, 1);
      }
      return of(caseData);
    }),
    concatMap((caseData: CaseData) => {
      return this.caseContext.patchCase(caseData);
    })
  )
}

This works fine but when I call patchCase and subscribe to the behavior subject in that function it has changed. The tags that were removed in the switchMap above are also removed in the CaseData in the behavior subject.

public patchCase(updated: CaseData): Observable<CaseData> {
    return of(updated).pipe(
      mergeMap((changed: CaseData) => {
        return combineLatest([this._caseData$, of(changed)]);
      }),
      map(([original, changed]: [CaseData, CaseData]) => { 
            ^- this value has the tags removed that were removed in the previous function.      
        return {
          caseId: changed.caseId,
          operations: createPatch(original, changed)
        };
      }),
      concatMap((caseOperations: { caseId: string; operations: Operation[] }) => {
        return this._caseApi.patch(caseOperations.caseId, caseOperations.operations);
      }),
      tap((updated: CaseData) => {
        this.load(updated.caseId);
      })
    );
  }

Im sure there is a better way to structure this whole thing than what I have above, and I am able to get around this by making a copy of the object in the unassign function before I remove the tags, but why does case data change when I change it in another observable stream that is piping off an observable and not even touching the behavior subject? And if this is expected, is there a better way to do this sort of thing without mutating the value in the subject aside from just making a copy?

Brandon
  • 830
  • 1
  • 15
  • 35
  • 1
    [splice](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/splice) is a destructive and mutating operation, working on arrays, which are passed by reference... so I suppose, that's why the underlying stuff is getting changed... – Nalin Ranjan Feb 24 '22 at 15:09
  • Yea I figured it had something to do with case data here being a reference to the case data in the subject, but I always thought it was supposed to be immutable. I'm wondering it if it would make more sense to just always return a copy on the "Context" observable. – Brandon Feb 24 '22 at 15:17
  • Objects are [passed by reference](https://stackoverflow.com/a/3638034/6513921) in Javascript. – ruth Feb 24 '22 at 15:18
  • 1
    @Brandon: You don't have to explicitly return a copy of the object. The answer from @Andrei returns a copy implicitly anyway. Note: [`Array#filter`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/filter) method _"creates a new array"_. And it's more elegant IMO. – ruth Feb 24 '22 at 15:20
  • @ruth, yea I agree with you that the spread syntax is a better solution all around. And I am definitely aware that objects are passed by reference in JS, this just wasn't one of those cases where I expected that rule to apply lol! I will definitely be more aware of this behavior in the future! Thank to you both! – Brandon Feb 24 '22 at 15:37

1 Answers1

1

it happends because of this line

caseData.tags.splice(index, 1);

in this case .tags array is mutated. to get rid of it try to modify your code like this:

unassign = (tag: Tag) => {
  this.caseContext.Context.pipe(
    map((caseData) => ({
      ...caseData,
      tags: caseData.tags.filter(tagId => tagId !== tag.tagId)
    })),
    concatMap((caseData: CaseData) => {
      return this.caseContext.patchCase(caseData);
    })
  )
}

switchMap was useless here, as you are basically mapping simple value to simple value, not the observable.

Andrei
  • 10,117
  • 13
  • 21
  • Thanks, yea I actually had other stuff happening before and after that that I removed for the example, which is why I had the switchMap there. But yea I agree the spread syntax and the filter make a lot more sense here for sure. It just seems odd to me that anything should be able to mutate the value from the subject like that. – Brandon Feb 24 '22 at 15:20
  • 1
    It doesn't have anything to the with the subject. The observable only emits the object held by observable. And JS makes it so that it's passed by reference. The plain JS object is then mutated by the JS built-in `Array#splice` method. There are use cases where such mutating behavior is expected. – ruth Feb 24 '22 at 15:22