Support updates through getters

[kettanaito]

Hi!

I'm having a rather interesting use case as a part of mswjs/data#346. Consider this:

import { create } from 'mutative'

const record = {
  get comments() {
    return []
  }
}

const nextRecord = create(record, (record) => {
  record.comments.push(1)
})

Expected behavior

nextRecord.comments // [1]

Current behavior

nextRecord.comments // []

Insights

Mutative does not recognize this update at all, which is evident if you use the enablePatches: true approach. The patches array for this mutation is [].

Since the update function does not perform any direct reassignment or mutation of the record, Mutative thinks no changes have occurred, while they have.

Proposal

Would it be feasible for Mutative to support updates through getters? With the goal that the use case above would work identically whether record.comments is a getter or a plain array.

[kettanaito]

I would also be curious to learn why Mutative skips this as an update. Does it have to do with some Proxy deduplication? Does it recognize record.comments as a getter and thinks it has already been proxied by Mutative? Or is it something else entirely?

[unadlib]

hi @kettanaito, I haven’t had a chance to read your source code yet, but based on the use case you mentioned, it behaves exactly as expected.

Maybe you're using the wrong example.

const record = {
  arr: [],
  get comments() {
    return this.arr;
  }
};

const [nextRecord] = create(record, (record) => {
  record.comments.push(1)
}, {
  enablePatches: true
});

expect(nextRecord.comments).toEqual([1]);

Whether enablePatches is set to false or true, the result is as expected.

If I’ve misunderstood anything, please point it out. Thanks a lot.

[kettanaito]

Hey, @unadlib! Thank you for looking into this. I've published a minimal reproduction test in https://github.com/kettanaito/mutative-157-getters, which is currently failing.

You are correct that nextRecord contains the updated property. For some reason, that doesn't happen in msw/data, but it might be related to what we're doing in the library.

But an isolated test showed a few interesting details:

• record.comments stops being a getter after Mutative updates. It becomes a regular property, which is incorrect.
• The array record.comments getter references does not get updated, although you're pushing directly to it.

Does this look like a bug to you? 🤔

I am not familiar with Mutative enough to state if this is a bug or not. I'm trying to figure out how to do updates of and through referenced properties in msw/data, where we're using getters to implement record relations (one-of/many-of/etc).

[unadlib]

hi @kettanaito , By default, Mutative performs a relaxed shallow copy. From your example, however, a custom strict shallow copy is required to preserve the integrity of its getter. Considering the potential issues that a custom copy function may introduce when applying patches, Mutative currently does not support using a custom shallow copy function together with enablePatches: true.

For your example, you might consider using it like this:

import { it, expect } from "vitest";
import { create } from "mutative";

it("updates through getters", () => {
  const user = {
    _comments: [],
    get comments() {
      return this._comments;
    },
  };

  expect(Object.getOwnPropertyDescriptor(user, "comments")).toEqual({
    configurable: true,
    enumerable: true,
    get: expect.any(Function),
    set: undefined,
  });

  const nextUser = create(
    user,
    (draft) => {
      draft.comments.push(1);
    },
    {
      mark: (target) => () =>
        Object.create(
          Object.getPrototypeOf(target),
          Object.getOwnPropertyDescriptors(target)
        ),
    }
  );

  expect.soft(nextUser.comments, "Returns the updated user").toEqual([1]);
  expect
    .soft(
      Object.getOwnPropertyDescriptor(nextUser, "comments"),
      "Preserves the getter"
    )
    .toEqual({
      configurable: true,
      enumerable: true,
      get: expect.any(Function),
      set: undefined,
    });
});

Please note that Mutative cannot create draft proxies for objects whose getters rely on external references.

[unadlib]

It seems that, in mswjs/data, using enablePatches: true together with a custom shallow copy is basically unavoidable?

[kettanaito]

Thanks for looking into this and for the suggestion above. I think I understand the current behavior. Sadly, I cannot use an intermediate _comment key as that would mutate the user's input (their record).

Hmm, I wonder what I can do in this situation... I already make a serialized copy (no getters) of the record for internal purposes, but exposing that to the user in the data (think draft update) function would break relations that the user expects to work

.update({
  data(user) {
    // Consider this is a circular reference.
    user.posts[1].author // Oops! Nothing.
  }
})

Moreover, serializing the entire record can be costly for big records and potentially impossible if the record contains circular references with another record.

Is there a way to "teach" Mutative how to perform through-getter updates? Or, maybe, you have some other ideas on your mind. Sorry, I understand this isn't a Mutative issue, but I would highly appreciate your opinion on this situation. Thanks.

[unadlib]

hi @kettanaito, I think the memoization approach is a good choice. Please review this PR for the relevant details: mswjs/data#350.

If I’ve misunderstood anything, please let me know. Thank you very much.

[kettanaito]

Thanks for that pull request. I will take a look! You can close this issue.