mobx: makeObservable throws in dev environment when used with annotations on decorated object
PR #3154 caused a breaking change in dev environments for us, now that it dies when calling makeObservable passing annotations on a target object with decorators. The change was IMHO intended to prevent users from not noticing that the annotations are used instead of the stored annotations from the decorators.
As this may help in avoiding issues like #3152 , this was a breaking change for our use case, where we call both makeObservable with annotations and without on a target. The use case is e.g. if you have a common interface implemented by multiple classes and you always want to use the same annotations. Or if you want to automatically make all fields of an object observable (using a helper function), but still have specific properties that you want to mark as computed.
Intended outcome:
Sample code:
interface MyCommonInterface {
field1: number;
}
class MySpecialClass implements MyCommonInterface {
field1 = 42;
@computed
get doubled(): number { return this.field * 2; }
}
makeObservable(target, { field1: observable });
makeObservable(target);
// no error
No error.
Actual outcome:
makeObservable throws error “makeObservable second arg must be nullish when using decorators. Mixing @decorator syntax with annotations is not supported.”
How to reproduce the issue:
Run the above code in a dev environment. Works fine in production.
Versions
mobx >= v6.3.6
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 4
- Comments: 18 (6 by maintainers)
I have recently upgraded to the most recent mobx and am now getting this error. The underlying routing package we use https://github.com/nareshbhatia/mobx-state-router uses mobx without decorators, however we use it with decorators.
Does anyone have a solution to the situation?
I assume this is going to be an issue for a lot of mobx packages that use the alternate to what the developer is using.
Sorry for the late response, I did not get to this issue any earlier.
I have now changed our helper function to use decorators internally and it works well - just like the former helper function using annotations. So this seems to suite and fix my use case. Thank you!
If you mean all in a sense that you tried to resolve these fields dynamically, then it must be done in
constructoras fields do not exist on the class (prototype). That’s whymakeObservablemust be called inconstructor(unless you rely on some TS specific reflection feature). Should be unrelated touseDefineForClassFieldsthough. If you setup a codesandbox I can tinker with, I may try to help.Unfortunately that’s the case. Tbh I don’t remember if there is a fundamental problem or if it’s just to avoid complicating things any further without solid justification. The number of possible usage scenarios tends to grow quite fast here and it’s likely we will miss that particular one that breaks things and I can guarantee someone will eventually hit this “unthinkable” case 😃 Anyone is free to send a PR and convince others, that all is good and well. I personally don’t want to take this responsibility atm.
I dunno what the motivation here is, but design wise, I think it would be fine having mobx as a depedency on “backend”. Quotes are intentional, because the class is designed to be isomorphic. It is part of the frontend, in the same way as it is part of the backend - you can’t change it’s implementation without affecting both. It’s implementation either pretends that it doesn’t need to be observable or it assumes that the observability can be provided later by frontend, which is fine, but the assumption is still part of it’s design - the frontend implementation details still leak into the backend. Don’t want to go into lengthy discussions here. Just saying it’s an option I personally would take into consideration, especially if it could decrease complexity.
sure