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

The necessity of making each property @observable #927

Closed
sebastian-zarzycki-apzumi opened this issue Apr 3, 2017 · 10 comments
Closed

The necessity of making each property @observable #927

sebastian-zarzycki-apzumi opened this issue Apr 3, 2017 · 10 comments

Comments

@sebastian-zarzycki-apzumi
Copy link

sebastian-zarzycki-apzumi commented Apr 3, 2017

Supposedly I have my own domain object:

export class MySomething {
    property1 = 1;  
    property2 = 2;
    property3 = 3;
}

and a React component, that uses this object as @observable:

    @observable something = new MySomething();

and that component displays something.property1, something.property2 and so on.

I've noticed that if I change the value of one of the properties (i.e. upon click, or in timeout), the component does not re-render. It is only, when I mark each and every property of my model (so, property1, property2, etc.) as @observable, then it works correctly. Is that expected behavior (prototype-less object, vs boxed object, etc?). It seems a bit awkward to introduce annotations into domain objects that shouldn't really know anything about the rest of the application. Am I missing something here, or is it just the price to pay for being able to use them dynamically in React components with MobX?

@urugator
Copy link
Collaborator

urugator commented Apr 4, 2017

domain objects that shouldn't really know anything about the rest of the application.

Yea, but the rest of your application has to know something about your domain object, oterwise there is no way to use it.
You have to expose some API which allows rest of the application to react to domain object changes.
For example, you could do that by making the domain object an event emitter and the rest of your app event listeners. In case of MobX you make the domain object observable and rest of your app observers.
The coupling, even though a loose one, exists no matter which solution you use.

@sebastian-zarzycki-apzumi
Copy link
Author

Well, not necessarily. In AngularJS, you don't modify your domain objects, instead they exist in angular's scope, that manages the changes "externally", in a way. That's not exactly the same thing (as MobX makes them fully observable and that has benefits of its own), but still.

But anyway, I understand. I don't have a big issue with it. I just wanted to know that I'm doing this right, because I was under the impression that if I make an object observable, then all its properties will be, too. But I think it only works for {} objects (no prototype), and your general domain objects need to define observables on their own. Correct?

@mweststrate
Copy link
Member

Yeah the thing is MobX doesn't want to mess with the internal state of things instantiated with a constructor, as there are probably many internal assumptions in the thing, and forcefully making stuff observable might cause all kind of issues. So I think making the properties you intend to be observable explicitly observable is the best approach. But, if you want to convert a complete object at once at your own digression, you could pull this trick: extendObservable(thing, thing)

@sebastian-zarzycki-apzumi
Copy link
Author

Is it possible to do this with decorator?

@mweststrate
Copy link
Member

mweststrate commented Apr 4, 2017

This will probably do the trick:

function allObservable(target) {
  extendObservable(target, target)
  return target
}

@allObservable class Stuff {
  x = 3
}

@mweststrate
Copy link
Member

Or open issue / PR here https://www.npmjs.com/package/mobx-decorators :)

@sebastian-zarzycki-apzumi
Copy link
Author

Yeah, it would be nice to have it wrapped already in the package. But that code you've posted will do nicely, as well. Thank you!

@thunderkid
Copy link

@mweststrate:
I know this thread is a bit old now, but should your allObservable decorator above still work in Mobx5? It's giving me a runtime error of 'extendObservable' expects an object as first argument.

@urugator
Copy link
Collaborator

urugator commented Apr 9, 2019

@lock
Copy link

lock bot commented Jul 21, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants