-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
New decorators and enumerability #4248
Comments
If you want to know a bit more about the reason why it works the way it does, would you mind rephrasing the question in such a way that it touches more on the factual matter of your problem and less on your value judgement about it? |
The "value judgements" result in very concrete problems. The facts are that his behavior conflicts what normal Javascript objects are. Unless we go through the different kinds of pains of hiding things, every property is enumerable and is owned by the containing object. Why should that be different from observable properties? toJS / JSON.stringify are just broken as are any way of treating OOP objects in a generic way. Functionality just gone. No more easy logging, we need an elaborate JSONification scheme for our domain. And the only reasons we get for that are your value judgements about closeness to the ECMAscript decorators standard and OOP purity. I'm unhappy with losing all that in the code that uses MobX. |
Let's stick to the concrete problems then. Telling me what my convictions are is at best very presumptions. Fwiw I'd agree in general it'd be great if these would be own and enumerable, it's a while ago since I worked on it, but I recall the spec didn't allow for an accessor property to be installed efficiently as own property on a targeted instance (for accessors the target is prototype), or it wasn't possible to compose them . But happy to be proven wrong if you have a small sandbox or something that does set these properties correctly for a noop accessor decorator. |
I’m not sure if this is the right place to mention this, but I wanted to add that with the new decorators (2023-11), the toJS function no longer works as expected. The returned object is always empty. Additionally, apiOwnKeys only returns a single Symbol(mobx administration) instead of the expected keys. |
FWIW, getter in object is enumerable
|
I agree this is a big breaking change, losing enumerability may cause many APIs to work unexpectedly after the migration to modern decorator |
I suggest we add a warning to new decorator doc before we come up with a solution |
I just realize (mostly as a stop-gap) that toJS itself probably should be updated, e.g. instead of the apiOwnKeys we should probalby just lookup the observable adminstration and map over the internal properties (filtered on ObservableValues): https://github.com/mobxjs/mobx/blob/main/packages/mobx/src/types/observableobject.ts#L104 |
I'm just catching up on MobX and the new decorators with the accessor syntax, which I don't really like (far more noise than one makeObservable call), but I get it.
What I don't get is the fact that the new properties are not enumerable, and not own properties?
If we have a very basic class like
in what world does it make sense that "name" is neither an own property nor enumarable but num is?
This makes toJS() totally useless, especially for stuff like just logging and knowing what the hell is going on in my application. Same for JSON.stringify(). Useless.
So now I need to write manual JSON-ification schemes or use external libraries just to know what is going on in my application?
The reasons given in https://mobx.js.org/enabling-decorators.html / "Decorator changes / gotchas" seem incredibly hand-wavy compared to these actual problems?
"following the spirit of the ES language and what accessor means" is nice, but how about you follow what MobX means and what it presents as abstraction to the user? Same for OOP purity. Is there any other situation where MobX enforces OOP purity at the expense of functionality?
The text was updated successfully, but these errors were encountered: