Proxy revoked error when performing chai deep equality

[Rubilmax]

Using create to apply a reducer on some Class instance marked as immutable and then comparing the result data to some expected class instance with chai's deep equality util throws a Proxy revoked error:

// Where `src` is some non trivial class instance
const data = create(src, (draft) => {}, { mark: () => "immutable" });

expect(data).to.eql(expected); // Cannot perform 'ownKeys' on a proxy that has been revoked

Do you know why?

[unadlib]

hi @Rubilmax, the deep attributes contain some special objects that are not finalized draft in src, so data already includes a draft based on a Proxy.

Unless you are certain that all sources are generally finalizable draft instances, it is not recommended to directly use {mark: () => "immutable"}.

For example,

  class Foobar {
    bar = 1;
  }

  const foobar = new Foobar();

  const state = create(
    foobar,
    (draft) => {
      draft.bar = 2;
    },
    { mark: () => 'immutable' }
  );
  const foobar2 = new Foobar();
  foobar2.bar = 2;
  expect(state).toEqual(foobar2);

Could you provide a minimal reproducible example?

[Rubilmax]

Unless you are certain that all sources are generally finalizable draft instances

How can I make sure of that?

i am working on a minimal reproducible example

[Rubilmax]

Alright so here's the minimal reproducible case I could find: https://github.com/Rubilmax/mutative-jest

It is actually using jest but the bug is the same: some proxy is revoked when a property is trying to be accessed

I defined some classes:

export class Entity {
  constructor(public readonly name: string) {}
}

export class Group {
  constructor(
    public readonly id: string,
    public readonly children: Entity[],
  ) {}

  public clone() {
    return new Group(this.id, this.children);
  }
}

export class Data {
  constructor(public readonly groupData: Record<string, Group>) {}
}

And a failing test:

import { makeCreator } from "mutative";
import { Data, Entity, Group } from "../src/Data";

const create = makeCreator({
  mark: () => "immutable",
});

const entity1 = new Entity("entity1");
const entity2 = new Entity("entity2");

const data = new Data({
  id1: new Group("id1", []),
  id2: new Group("id2", [entity1]),
  id3: new Group("id3", [entity1, entity2]),
});

describe("Data", () => {
  it("should not fail", () => {
    expect(
      create(data, (draft) => {
        draft.groupData["id1"] = draft.groupData["id1"]!.clone();
      }),
    ).toEqual({});
  });
});

which throws the following error: TypeError: Cannot perform 'IsArray' on a proxy that has been revoked

The test does not throw this error as soon as we remove the clone call, which creates a new Group class instance. Any leads on the source cause of this behavior? This is the only thing preventing me from using this library

[unadlib]

@Rubilmax , Although you used the mark: () => "immutable" function, the Group instance methods mix drafts with non-drafts.

  public clone() {
    return new Group(this.id, this.children);
  }

return new Group(this.id, this.children); Non-draft instances are generated here; you only need to use unsafe() in the non-draft structure.

create(data, (draft) => {
  draft.groupData['id1'] = unsafe(() => draft.groupData['id1']!.clone());
})

[unadlib]

I noticed that the data structure in the example is a non-unidirectional tree, but you used mark: () => "immutable", which may lead to some unexpected behavior, as Mutative only supports unidirectional trees.

Unless you explicitly ensure that the entire object tree is in a strictly immutable mode, it may produce unintended behaviors. We should use the mark function to distinguish between immutable and mutable nodes.

const nextData = create(data, (draft) => {
  expect(current(draft.groupData['id2'].children[0])).toBe(
    current(draft.groupData['id3'].children[0])
  );
  expect(draft.groupData['id2'].children[0].name).toBe('entity1');
  expect(draft.groupData['id3'].children[0].name).toBe('entity1');
  draft.groupData['id2'].children[0].name = 'new-entity';
  expect(draft.groupData['id2'].children[0].name).toBe('new-entity');
  // !!! it's unexpected
  expect(draft.groupData['id3'].children[0].name).toBe('entity1');
});

[Rubilmax]

@Rubilmax , Although you used the mark: () => "immutable" function, the Group instance methods mix drafts with non-drafts.

  public clone() {
    return new Group(this.id, this.children);
  }

return new Group(this.id, this.children); Non-draft instances are generated here; you only need to use unsafe() in the non-draft structure.

create(data, (draft) => {
  draft.groupData['id1'] = unsafe(() => draft.groupData['id1']!.clone());
})

Indeed, and I didn't know how to handle these. Unfortunately, using unsafe to wrap the clone call does not solve the issue (the same error is thrown: TypeError: Cannot perform 'IsArray' on a proxy that has been revoked

I noticed that the data structure in the example is a non-unidirectional tree, but you used mark: () => "immutable", which may lead to some unexpected behavior, as Mutative only supports unidirectional trees.

That is correct and it purely was for the example but clearly not necessary. In my real-life example it is not the case; and I fully understand the possible unexpected outcomes due to this, thanks!

[Rubilmax]

I tried this and it doesn't throw anymore but the test doesn't pass:

import { makeCreator, unsafe } from "mutative";
import { Data, Entity, Group } from "../src/Data";

const create = makeCreator({
  mark: (value) => {
    if (value instanceof Group) return "mutable";

    return "immutable";
  },
});

const entity1 = new Entity("entity1");
const entity2 = new Entity("entity2");

const data = new Data({
  id1: new Group("id1", []),
  id2: new Group("id2", [entity1]),
  id3: new Group("id3", [entity2]),
});

describe("Data", () => {
  it("should not fail", () => {
    expect(
      create(
        data,
        (draft) => {
          draft.groupData["id1"] = unsafe(() => draft.groupData["id1"]!.clone());
        },
        { strict: true },
      ),
    ).toEqual(
      new Data({
        id1: new Group("id1", []),
        id2: new Group("id2", [new Entity("entity1_cloned")]),
        id3: new Group("id3", [new Entity("entity2_cloned")]),
      }),
    );
  });
});

I'm sorry this may sound easy for you but I don't fully understand the expected usage of mark, even after reading the documentation 5 times. I'll commit something once this is resolved, so the README feels clearer at least to me on this point

[unadlib]

@Rubilmax

Sorry, the docs for the mark function weren't clear enough and caused you some confusion. We'd really welcome your PR.

These test cases should work fine.

import { makeCreator, unsafe } from 'mutative';

export class Entity {
  constructor(public readonly name: string) {}
}

export class Group {
  constructor(public readonly id: string, public readonly children: Entity[]) {}

  public clone() {
    return new Group(this.id, this.children);
  }
}

export class Data {
  constructor(public readonly groupData: Record<string, Group>) {}
}

const create = makeCreator({
  mark: (value) => {
    if (value instanceof Group) return 'mutable';

    return 'immutable';
  },
});

const entity1 = new Entity('entity1');
const entity2 = new Entity('entity2');

const data = new Data({
  id1: new Group('id1', []),
  id2: new Group('id2', [entity1]),
  id3: new Group('id3', [entity2]),
});

describe('Data', () => {
  it('should not fail', () => {
    expect(
      create(
        data,
        (draft) => {
          draft.groupData['id1'] = unsafe(() =>
            draft.groupData['id1']!.clone()
          );
        },
        { strict: true }
      )
    ).toEqual(
      new Data({
        id1: new Group('id1', []),
        id2: new Group('id2', [new Entity('entity1')]),
        id3: new Group('id3', [new Entity('entity2')]),
      })
    );
  });
});

[unadlib]

Although the custom mark function in Mutative v1.0.4 does not support processing mixed structures of drafts and non-drafts for finalizing, it is indeed capable of supporting this. I will fix it soon and release v1.0.5.

Thank you very much for reporting this issue.

[unadlib]

@Rubilmax Mutative v1.0.5 has been released. Feel free to use it.

[Rubilmax]

Amazing!! Saw you implemented this issue's example as a test. Will check this out asap. Thanks for your reactivity

[Rubilmax]

It seems changes included during cloning are not included in the final state:

import { makeCreator } from "mutative";

export class Entity {
  constructor(public readonly name: string) {}
}

export class Group {
  constructor(
    public readonly id: string,
    public readonly children: Entity[],
  ) {}

  public clone() {
    return new Group(
      this.id,
      this.children.map((child) => new Entity(child.name + "_cloned")),
    );
  }
}

export class Data {
  constructor(public readonly groupData: Record<string, Group>) {}
}

const create = makeCreator({
  mark: () => "immutable",
});

const entity1 = new Entity("entity1");
const entity2 = new Entity("entity2");

const data = new Data({
  id1: new Group("id1", []),
  id2: new Group("id2", [entity1]),
  id3: new Group("id3", [entity2]),
});

describe("Data", () => {
  it("should not fail", () => {
    expect(
      create(data, (draft) => {
        draft.groupData["id1"] = draft.groupData["id1"]!.clone();
      }),
    ).toEqual(
      new Data({
        id1: new Group("id1", []),
        id2: new Group("id2", [new Entity("entity1_cloned")]),
        id3: new Group("id3", [new Entity("entity2_cloned")]),
      }),
    );
  });
});

For example, during cloning, it is expected children are appended _cloned at the end of their name, but they are not. Do you know why?

The original issue reported is indeed fixed though, thanks a lot!

[unadlib]

You need to execute their clones.

draft.groupData['id2'] = draft.groupData['id2']!.clone();
draft.groupData['id3'] = draft.groupData['id3']!.clone();

[Rubilmax]

This is ridiculous lmao I'm so sorry, thanks again for your reactivity. It's all sorted out