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

Excluding a single property from Object freezing #928

Open
2 tasks done
TimoHocker opened this issue Apr 18, 2022 · 6 comments
Open
2 tasks done

Excluding a single property from Object freezing #928

TimoHocker opened this issue Apr 18, 2022 · 6 comments

Comments

@TimoHocker
Copy link

🙋‍♂ Question

I am trying to buld a simple Subject/Observer pattern together with immerjs to ensure consistent update propagation in my project.
Right now immerjs is causing some trouble because I want to register an observer to an object that has run through immer and is now frozen.

Error: [Immer] This object has been frozen and should not be mutated

Basically I have to exclude the array of observers from getting touched by immer. How could I achieve that?
If it's not possible, why?

Link to repro

The codesandbox tests don't trigger my observers for some reason.
Running the same code locally gives the correct result.
The first test passes and the second test fails with the above error.

https://codesandbox.io/s/xmzig2

Environment

  • Immer version: 9.0.12
  • typescript 4.6.3
  • Jasmine 4.1.0
  • Occurs with setUseProxies(true)
  • Occurs with setUseProxies(false) (ES5 only)
@mweststrate
Copy link
Collaborator

Sorry that is a bit too much code to understand to see why or why not your project is working and the error thrown in the test is not the one you've posted above, but immutability is indeed on the object level, see structural sharing and all other benefits of immutable objects would be pointless / confusing if attributes where still allowed to be modified.

@TimoHocker
Copy link
Author

I've modified the sandbox and removed everything not needed for the test.
Previously it was just a copy of my original code because I tried to replicate the same behaviour, that now worked with the simpler code.
The test uses an array to store the observers, so it produces a different error than the previous set

Cannot delete property '0' of [object Array]

I do understand your concern about making bad practices possible, but this simple requirement forces me to fall back to the standard shallow freeze and to develop my own methods for cloning and modifying.

I need some way of notifying other parts of my program about an object change. In this case by calling all observers with the newly produced object. Those observers have to be stored somewhere and I don't think any other place than the object itself would make any sense.

@mweststrate
Copy link
Collaborator

mweststrate commented May 2, 2022

You could use a stable ref to make a part mutable again, by creating a small wrapper object and make sure to never reassign _observers itself, e.g.:

class TestClass {
  [immerable] = true

  private readonly _observers = {
    [immerable]: false,
    observers: new Array<Observer<TestClass>> 
  }

Those observers have to be stored somewhere and I don't think any other place than the object itself would make any sense.

WeakMap's might be a great option here.

@TimoHocker
Copy link
Author

Thanks for the suggestions. WeakMaps sound like a good alternative. The wrapper method doesn't change anything because the object is frozen recursively regardless of [immerable] being true or false. I'll just stick with Object.freeze, it's the cleanest solution so far.

@mweststrate
Copy link
Collaborator

The wrapper method doesn't change anything because the object is frozen recursively regardless of [immerable] being true or false.

Did you test that by any chance? [immerable]=false shouldn't freeze, so if it does, that might be an actual bug.

@TimoHocker
Copy link
Author

It results in the same error with just the _observer property swapped out and all functions changed to use the inner array.
Code is in the sandbox (src/wrapper.test.ts). It seems like codesandbox is having issues again, so I had to run the tests locally.
The package versions in my original post are still valid.

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

2 participants