Clarify that the first result from the producer is not immutable unless a change is made

[rickmugridge]

🚀 Feature Proposal

As expected, when the producer is first applied to an object (or array) and a change is made, an immutable object results. However, if no such change is made, the result is mutable.

Motivation

Docs are unclear on this matter. I suggest that it would be better to make the result immutable regardless. But if that doesn't work, it would be good for the docs to be clear on this matter.

With the current implementation, I'm forced to make an arbitrary change to the object to make sure that it is immutable immediately.

Example

    it("Simple object becomes immutable as the produce() call made a change", () => {
        const c = {id: 3};
        const immutable = produce(c, draft => {
            draft.id = 4;
        });
        expect(c.id).eql(3);
        expect(immutable.id).eql(4);
        expect(() => (immutable as any).id = 5).to.throw("");
    });

    it("Simple object is not made immutable as the produce() call made no change", () => {
        const c = {id: 3};
        const immutable = produce(c, draft => {
        });
        (immutable as any).id = 4;
        expect(immutable.id).eql(4);
    });

[rickmugridge]

Immer is a great and convenient structure-sharing system, but does not confer immutability except at points of change. Here's another example that illustrates this:

    it("Parts of simple object becomes immutable as the produce() call made a change", () => {
        interface D {
            a: { b: number };
            z: { y: number };
        }

        const d: D = {a: {b: 2}, z: {y: 3}};
        const immutable = produce(d, draft => {
            draft.a.b = 1;
        });
        expect(() => immutable.a.b = 5).to.throw("");
        immutable.z.y = 4;
        expect(immutable.z.y).eql(4);
    });

[mweststrate]

Although the freezing itself is not really the point of immer (there are separate packages for that), it seems a lot of peepz expect a full freeze from immer. To avoid more confusion in the future, a PR is welcome :)

[rickmugridge]

I'm happy to (recursively) pre-freeze. Is it worth mentioning your favourite one in the docs, near the start to ensure immutability? I'll be more than satisfied with that.
Thanks.

[mweststrate]

A PR is welcome, but not urgent, I hope to get around to implement the change anyway in a couple of weeks, as it would be the least suprising behavior it seems in general :)

[ianstormtaylor]

@mweststrate can you clarify what change you're thinking of making? I also expected that produce always returns deeply frozen objects. Are you thinking of changing to make that so? Or just changing the docs to have a warning?

[mweststrate]

The first 😊 Op ma 9 sep. 2019 23:16 schreef Ian Storm Taylor <[email protected]>:

…

@mweststrate <https://github.com/mweststrate> can you clarify what change you're thinking of making? I also expected that produce always returns deeply frozen objects. Are you thinking of changing to make that so? Or just changing the docs to have a warning? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#412?email_source=notifications&email_token=AAN4NBGWR6PD3B6GZYV4OUTQI3DKBA5CNFSM4IPIBQO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6JGPWQ#issuecomment-529688538>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAN4NBHQXW4GPQUCYUUUMQLQI3DKBANCNFSM4IPIBQOQ> .

[aleclarson]

🎉 This issue has been resolved in version 4.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀