Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deep freeze of object property in immerable class #604

Closed
jeron-diovis opened this issue May 24, 2020 · 9 comments
Closed

deep freeze of object property in immerable class #604

jeron-diovis opened this issue May 24, 2020 · 9 comments

Comments

@jeron-diovis
Copy link

jeron-diovis commented May 24, 2020

🙋‍♂ Question

I'm trying to do smth like this:

class Thing {
  [immerable] = true;

  constructor({ x }) {
    this._data = { x };
  }

  get x() {
    return this._data.x;
  }

  set x(x) {
    this._data.x = x;
  }
}

const thing = new Thing({ x: 1 })
produce(thing, draft => { draft.x += 1 })

Here is my sandbox: https://codesandbox.io/s/immerable-83jiv. It does work without any problems.
In console there you can see, that property _data is marked as non-writable and non-configurable, but _data.x is writable and configurable:
Screenshot 2020-05-24 23 25 05
So, it looks like thing is not deeply frozen – despite of setAutoFreeze(true).

At the same time, in my real project, with exactly same setup I get this:
Screenshot 2020-05-24 23 22 38
(it's a redux app with curried producers, as described in docs)
And in console I see that _state.selected is non-configurable and non-writable.

I really can't understand why I can't reproduce the same in sandbox, and which of two cases is actually a correct behavior.
If it's possible, can you give any clue about this situation? Like, it has to be smth very basic about immer which I'm missing.

Environment

  • Immer version: 6.0.8
@mweststrate
Copy link
Collaborator

The handling of getters / setters will change in Immer 7, please revisit after Immer 7 has been released.

@mweststrate
Copy link
Collaborator

Would you mind checking again against immer 7? It handles getters and setters differently, which might be used in your project. If Immer 7 doesn't help, an actual reproduction is needed.

@jeron-diovis
Copy link
Author

Yep, with v7 it works fine.

Still not sure what exactly caused the issue – my case didn't look like #438, I have only primitive props everywhere. But using setUseProxies(false) in v6 also helped, so it was definitely related to changes in v7.

Thanks for new cool major version!

@jeron-diovis
Copy link
Author

jeron-diovis commented Jun 24, 2020

I have to reopen this issue. There is still smth wrong, even on version 7.0.5.
Please check out this sandbox

Current sandbox behavior is:

  • on first click on "inc" button, both i counter and item.x prop are updated in sync
  • on succeeding clicks on "inc" button, only i updates, item.x remains unchanged. Although no errors thrown.
  • after uncommenting line 24 (i.e. if _data prop is read before update), it works normally – both i and item.x are updated in sync.

In my project, in pretty same situation I have Uncaught TypeError: Cannot assign to read only property 'x' of object '#<Object>'. Probably because of strict mode enabled.

@jeron-diovis jeron-diovis reopened this Jun 24, 2020
@mweststrate
Copy link
Collaborator

mweststrate commented Jun 24, 2020 via email

@jeron-diovis
Copy link
Author

simplified sandbox

Now it throws just like it does in my app.

@mweststrate
Copy link
Collaborator

Thanks for the update!

@mweststrate
Copy link
Collaborator

Thanks for the detailed reproduction. Will publish a fix soon!

@aleclarson
Copy link
Member

🎉 This issue has been resolved in version 7.0.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants