You probably already know this 😀 but here is my strong vote against removing decorators. I frankly don't care what TC39 think, decorators are here to stay. I'm not removing them from any codebase and I will strongly advocate against any removal from any library as well.

IMO its time to stand our ground.

Quick thought:
What about adding decorators with external package to mobx core?
"mobx-decorators"
Do we expose the needed low-level primitives?

observable(1) cannot be <T>(value: T, options?) => T (as it must return some box), correct?
How does this work with class field value: int = observable(1)?

I worry that this data/metadata duality will backfire eventually. If I remember correctly we had something like this in Mobx2.

Had an idea like:
EDIT: nevermind bad idea...

🧨 1. Become compatible with modern ES standards

Personally I also hate what TC39 is doing to decorators, and maybe it is one of those things that should be left to transpilers. Maybe there could be a mobx-decorators v7 package for those that want to keep using with them? (and also could make adoption easier).

😱 2. Support proxy and non-proxy in the same version

Sounds awesome.

💪 3. Smaller bundle

To further decrease the build, I'd personally love to drop some features like spy, observe, intercept, etc. And probably a lot of our low-level hooks can be set up better as well, as proposed by @urugator.

As long as there are alternatives it should be ok (mobx-keystone for example relies on both observer, intercept and then some more). But the more changes there are, the more likely current mobx libs will become incompatible and stop adoption.

🍿API changes

autoInitializeObservables will reflect on the instance, and automatically apply observable, computed and action in case your class is simple

When is a class considered simple? When there are no fields with mobx "decorators"?
If so, it might be confusing to have a "simple" class, then add an action and see how the whole class becomes something totally different.

observable and extendObservable does currently convert methods to observable.ref's instead of actions by default.

I think 99% of the time you'd want functions to become actions (and actually I didn't even know this observable.ref was the case), so as long as this is explained on some release notes it should be ok.
In the worst case there could be a global flag like "versionCompatibility": 5 or similar that actually makes it work as usual for those migrating from an older version and that prints in the console a warning when a function is passed to observer so you can eventually fix it and remove the flag.

To further decrease the build, I'd personally love to drop some features like spy, observe, intercept, etc.

mobx-logger depends on spy as well as mobx-remotedev (Redux devtools for Mobx). Is there another way to listen to observable mutations in Mobx?

You probably already know this 😀 but here is my strong vote against removing decorators. I frankly don't care what TC39 think, decorators are here to stay. I'm not removing them from any codebase and I will strongly advocate against any removal from any library as well.

IMO its time to stand our ground.

@spion Frankly, that is just a rant. Michel wrote the pros of removing decorators. If you want to "stand ground" and "strongly advocate", then please make constructive counter-arguments instead of just "I like them".

Personally, I am on the other side of this war and I never liked decorators. This fresh new look definitely makes sense to me. However, if there is some possibility to keep decorators support in the external package as it was suggested, then it's probably something we should do. If people feel the necessity to wear a foot gun, it's their choice. Besides, it would be suddenly more apparent where that decorator magic is coming from and that it's something optional.

I worry that this data/metadata duality will backfire eventually. If I remember correctly we had something like this in Mobx2.

Yeah, I think that is the part I like the least as well. Maybe we should introduce a separate 'marker' to for observable properties, e.g. field = tracked(value), and use observable only for instantiating collections (mostly relevant when not using classes). But not sure whether we should have an alternative for computed as well, and what would be a good name.

I like decorators but I totally understand this direction. There are a few reasons for this, let me try and make Gorgi's case @FredyC :

• Decorators are explicit, I get to say exactly what is reactive, no fields are reactive implicitly.
• Decorators are declarative, it feels like another meta-property of the type (like private, or readonly) which addresses a concern of the field (reactivity).
• Decorators are the way this is done in other systems (like Angular) and languages (like UI systems in C# or Python)
• Decorators have been the "mostly standard MobX way" for a while and removing them is 5 years of breakage.

Also, removing something so widely used because the spec committee can't proceed always leaves a bad taste in my mouth. Even as I acknowledge they are all acting in good faith and that decorators are a hard problem for engines because of the reasons outlined in the proposal (I've been following the trapping decorators discussions).

Some API bikeshedding:

Decorators

class Doubler {
  @observable value = 1

  @computed get double() {
    return this.field * 2
  }

  @action increment() {
    this.value++
  }
}

Michel's original:

class Doubler {
  value = 1

  get double() {
    return this.field * 2
  }

  increment() {
    this.value++
  }

  constructor() {
    autoInitializeObservables(this)
  }
}

Subclass:

class Doubler extends ObservableObject {
  value = 1

  get double() {
    return this.field * 2
  }

  increment() {
    this.value++
  }
}

Can be interesting, but is not very good in terms of coupling. Or with a class wrapper:

const Doubler = wrapObservable(class Doubler {
  value = 1

  get double() {
    return this.field * 2
  }

  increment() {
    this.value++
  }
});

• Decorators have been the "mostly standard MobX way" for a while and removing them is 5 years of breakage.

Don't forget there will be a codemod to make most of the hard work for you, so this isn't really a valid argument. Besides, nobody is forced to upgrade to the next major. It probably depends if we will be able to maintain v4 & v5 or abandon those. And if we separate package will exist for decorators then it might be fairly non-braking change.

Btw, @mweststrate Just realized, why MobX 7? Do we need to skip V6 for some reason? 🤓

@FredyC hey, I would prefer it if we avoided terms like "isn't really a valid argument" when talking about each other's points.

I think having a common and standard way to do something in a library (decorators) is definitely a consideration and API breakage is a big concern - even with a codemod. I think removing decorators is unfortunately the way forward - but breaking so much code for so many users definitely pains me.

@mweststrate subclassing is also not very ergonomic and mixes concerns here IMO.

I'm not sure I understand the wrapping issue in TypeScript but I know there are challenges involving it. Wrapping doesn't actually have to change the type similarly to initializeObservables:

class Doubler {
   ...
}
initializeObservables(Doubler); // vs in the constructor

Or even decorate the type in a non-mutating way:

const ReactiveDoubler = initializeObservables(Doubler); // vs in the constructor

Wouldn't it make more sense to initializeObservables on the type and not on the instance? Is there any case I'd want to conditionally make an instance reactive but not have several types?

@benjamingr yeah that is exactly what decorate does so far. The problem is that initializeObservables won't see the fields if invoked on the type, field x = y is semantically equivalent to calling defineProperty(this, 'x', { value: y }) in the constructor, so the field does never exist on the type.

So even if you don't know the fields, but you do specify them on the type, you can't decorate the type to trap the assignment, because the new property will simply hide it. I think it is still a weird decisions to standardize [[define]] over [[set]] semantics, which has rarely any merits, and deviates totally from what TS and babel were doing. But that is how it is.....

@FredyC It is not a rant, it's a constructive comment. Decorators are widely used throughout the community, with large projects such as Angular, NestJS and MobX taking advantage of them. Thousands of projects depend on them. For TC39 to block their standardization process strikes me as extremely irresponsible, and the arguments for doing so are severely under-elaborated (a vague 3-pager does not an elaboration make - try harder TC39).

The advantages that @mweststrate mentioned are largely the fault of this lackluster standardization which means the argument is cyclic: it ultimately comes down to "we're not supporting decorators because decorators are not well supported". Language features that aren't yet standardized are never well supported, the argument can be used to justify not adopting any new language feature. So if TC39 "paves cowpaths" and everyone adopted this way of thinking, the language would stop evolving.

(Clearly, this is not a new feature so there is some merit to the "not likely to be supported" argument implying its a good idea to give up on them. I just wanted to bring to the table that the other approach - standing our ground - might be good too)

For those of us who do care about decorators, what are our options? Our only hope is to stand our ground, keep using them and keep advocating their standardization. Even if TC39 doesn't standardize them, development within TypeScript might continue, addressing the remaining gaps WRT reflection and types.

If you don't care about decorators, please stay out of it. They have always been optional in MobX and will continue to be optional - no one is forcing you to use them. If you care about a smaller bundle, they can be offloaded to a side module (but I maintain they should still have a first-class place in the documentation)

@mweststrate is there anything stopping us from trapping construct and intercepting those fields then or setting a proxy?

Such an initializeObservables wouldn't do anything until an object is constructed and then return a proxy (or decorate) when the constructor is called.

That is:

• Someone calls makeReactive/nitializeObservable and passes it a class
• That someone gets a new class back that is a proxy over the old class. (vs. a proxy for the instance by intercepting construct)
• When that class is constructed - we intercept the fields at the end of the constructor (after invoking it) and make it reactive. (Or with proxies we just return a proxy since we don't need to do work-per-field).

That way the fact the field is not a part of the type doesn't really matter - since while it looks like we are decorating the prototype/class we are actually decorating the instance and only "decorating" the construct on the type.

Indeed, flexibility is reduced. It can only look like this below, and it looks okay.

class Foo {
  @observable
  field1 = 'value1';

  field2 = 'value2';

  constructor() {
    makeObservable(this);
  }
}

Requiring a base class removes flexibility, for example using decorators in react component classes Op di 7 apr. 2020 06:45 schreef Michael Lin notifications@github.com:
…
@mweststrate https://github.com/mweststrate Maybe it's fine to use a proxy for this, but the downside is that the browser must support Proxy. What do you think about it? class Observable { constructor() { return new Proxy(this, { defineProperty(target: any, key: any, descriptor: any) { // handle decorator }, }); } } class Foo extends Observable { @observable field1 = 'value1'; field2 = 'value2'; constructor() { super(); } } — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2325 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBH3JOHOH3TMXU56E23RLK4V5ANCNFSM4LZQ2B5Q .

@mweststrate
You wrote on Twitter today that the new mobx will be very good. Which is incredibly good news :)
What will it look like from the user's side?
Will the new version eat less memory?

Have you considered an unopinionated MobX core build? Including just your basic TFRP functions (createAtom, computed, autorun/when/reaction, onBecomeObserved, and action) packaged up in a simple API might make for a lightweight and powerful alternative to RxJS and its friends.

That minimal set of functions should be enough to work with MobX React Lite and would let us sidestep the entire ES5 vs ES6 vs decorators question for a decent number of use-cases.

drop some features like spy, observe, intercept

Oh, no!
I use both intercept and observe:

1. I have a store for statistics data with absolute values (count of emails sended, opened etc) and while I put the data to store the hook intercept calls a function which calculate relative data ( e.g. % opened/sended)
2. Also I have a store Parameters and there I use observe for changing url of current page after sone parameters changed

My key message is that stores in Mobx is… umm… smart. It can autorun, lazy compute etc so why it can't avaiblity to manipulate of incoming data? 🤔 Drop observe/intercept will make a store more dumb. That smart black magic is that feature why we really love Mobx. Isn't it?

constructor () {
  makeObservable(this, { 
    field: observable,
    field2: observable.shallow,
    method: action,
  }
)

I also like this version more.

Is it possible to set TS to check that all fields that are not readonly must be marked as observable?
It would also be nice for TS to check that we marked a field readonly with an observer and treat it as an error.

@szagi3891

Now assuming you mean Typescripts readonly keyword and/or Readonly<T> interfaces.

These ideas are bad for two reasons.

Regarding readonly keyword

A field being readonly, doesn't mean its content is immutable, it just means the field itself can't mutate. A use case would be readonly todos: Array<Todo>.
You can't set a new array on it after the constructor assignment, but you most certainly can mutate the array itself.

Regarding Readonly<T> interface

There is a use case where you don't want to allow mutations outside of actions.

You could create private observables like private _todos: Array<Todo> but then also expose public computed getters. But that can easily be cumbersome if there are so many and has its own issues now[1]

Another approach is Readonly<Array<Todo>> with this

type Writeable<T> = { -readonly [P in keyof T]: T[P]};


class Store {
  readonly todos: Readonly<Array<Todo>>
  //assume set up in constructor 

  addTodo = (todo: Todo) => {
    const list: Writeable<Array<Todo>> = this.todos
    return list.push(readyTodo) 
  }
}

So I hope that readonly& Readonly<T> are not disabled from being observables.
Though I would welcome a more succinct and non-intrusive approach for making private mutable observables & public immutable computed for a class that works out of the box.

[1]: My current issue is that there is no way to assign a decorator on private observables

class Store {
  private _todos: Array<Todo>
  //constructor setups
}

decorate(Store, {
  _todos: observable
})

Edit: Added solutions and workarounds for the issues brought up: #2340 (comment)

Another strong vote against removing decorators.
But if it comes to this, I think having mobx-decorators the same way we have mobx-react would be a reasonable tradeoff.

#2325

@mweststrate

The downside of this approach is that I don't know how to do "auto" decorating with some exceptions. An additional API like makeAutoObservable(this, perMemberMap?) looks a bit ugly, although it is not too bad

I found it ugly too. Especially as there may be different ways of auto: In a class of mine all getters are computed, all fields but one are observable, but only about a half of functions are actions, others are views (in the MST sense). So I'd use {field: observable, getter: computed, function: error} as the default, which would force me to enumerate all functions, but probably save me some problems (because of function: error).

So I guess, the auto feature should be configurable, maybe like makeObservable(this, perMemberMap?, perMemberTypeMap?) with perMemberMap mapping members (field1, function2, ...) and perMemberTypeMap mapping member types ("field", "getter" and "function").

Moreover, there probably should be a global defaultPerMemberTypeMap in mobx, with the effective value given by {...defaultPerMemberTypeMap, ...perMemberTypeMap}. This would make clear how exactly "auto" works.

I would leave the makeAutoObservable to userland.
The idea was not to safe keystrokes, but to provide safety (so you cannot forget observable/action etc).
However it presumed that there is only opt-out behavior and that we can reliably itrospect the class, which doesn't seem to be the case (prive fields/optional fields/subclasses...?).
It shouldn't be hard to come up with own implementation that suits the user's needs, while being aware of potential limitations.

I've been thinking whether the view/action problem isn't a little bit artificial:
The action is mainly intended to provide batching. View with batching is fine, but we cannot use action for views because it also applies untracked.
We also cannot use batch alone, because "strict mode" requires action (even though it's still mainly about batching).
The only reason why action applies untracked is so it can be "safely" invoked from derivation (computed/autorun) ... however synchronously mutating state from derivation is most of the time antipattern or edge case.
So potentially we could remove untracked from action (1), so it can be applied automatically to every function and forbid state mutations from derivations unless you explicitely allow that by wrapping it with effect (which applies untracked).

[1] Semantically it doesn't make sense to apply action to view, what I actually mean is to provide something that doesn't apply untracked, while still satisfying the strict mode, therefore can be automatically applied to any function. It doesn't have to be called action and it doesn't mean that action has to go away...

Actually I think it could be simplified like this:
Everything stays as is.
The only difference is that makeAutoObservable will use the following function instead of action:

function wrap(fn) {
  return (...args) => {
     // provide batching
     startBatch()
     // allow reads
     allowStateReadsStart()
     // !!! untracked is missing so it works with views
     // !!! allow state changes only when outside of derivation
     if (notInsideDerivation()) { 
        allowStateChangesStart()
     }
     const result = fn(...args)
     if (notInsideDerivation()) { 
        allowStateChangesEnd()
     }
     allowStateReadsEnd()
     endBatch()
     return result;
  } 
}

As a consequence if the auto decorated function mutates state, it will throw when called from derivation (computed/autorun), so you will be forced to wrap it in runInAction/action manually - either at definition:

makeAutoObservable({
  method: action, // now you can call it even from derivation 
})

or inside derivation:

autorun(() => {
 runInAction(() => store.anAutoDecoratedMethod())
})

@urugator yes, I start to see the light! I think you are spot on. Looking back,

1. The main use case for the untracked part is to make sure actions called from autorun aren't tracked, allowing you to split of the observation and effect part. However, reaction solves that problem now in a more elegant way
2. allow state changes was introduced to prevent side effects in computes etc.

I think we can indeed introduce what you are proposing, let me know if this summarizes it correctly:

MobX has a global context state (or stack) that is either NONE, UPDATING or DERIVING

* track is the underlying primitive of autorun / computed

1. computed and autorun e.a. bring the state to DERIVING
2. action brings the state to UPDATING, but only if the current state is NONE
3. .runInAction and allowStateUpdates(true) brings the state to UPDATING regardless the current state, as escape hatch for lazy-cache-updating scenarios. We might be able to deprecate allowStateUpdates, but IIRC the false case also exists, so would have to investigate
4. untracked brings the state back to NONE (so an action call inside a reaction inside untracked for example will allow state updates again)
5. state updates throw in DERIVING state
6. enforceAction keeps working as is, warning when updates happen in NONE state
7. allowStateChangesInsideComputed can probably be deprecated? I have to search back what that one was good for again
8. batching keeps working as is

Well basically that goes back to the initial idea - it all sort of depends on how much we are willing to change API, introduce BCs and how far we want to go with these checks.

I think you got it mostly right, but:

Semantically it's a bit weird that the action is usable as a view (when it doesn't mutate state).

Throw on write detection to enforce correct context isn't bullet proof:

computed(() => {
  // this throws so the user is forced to use `runInAction` here
  myAction();	     
})

const myAction = action(() => {
  // but actually it throws here, so we can fix it here
  runInAction(() => {
     observable.set(5);
  });
})

Potentially we could restrict from which to which context the transition can occur, but then we may need to again differentiate between view/action (not sure atm).

runInAction shouldn't have a different meaning than action and allowStateChanges is just some low level thing which by itself doesn't map to any real use case, that's why I introduced that effect function, so it has semantic meaning (also it's analogous to reaction's second arg).

untracked imo shouldn't have any effect - it shouldn't (dis)allow reads/writes. It should simply disable subscriptions without changing the context. Again a low level thing that doesn't map to any real use case.

There is obserevableRequiresReaction, which basically says: Throw if you read something outside of DERIVING or UPDATING - which also means that you wouldn't be able to perform reads inside untracked (if it would change the context to NONE), which is mental, because untracked is used for reads in the first place.

EDIT: One more concern... Is it possible for useEffect to be ever invoked immediately during render (inside derivation)?

@mweststrate Sorry, I didn't mean to hijack the issue. I was showing my custom element example hoping to discuss similar issues that would happen in MobX.

Let me ask smaller MobX-specific questions one at a time:

If MobX rolls with the approach of

class Doubler {
  value = observable(1)
  double = computed(function () {
    return this.value * 2
  })
  increment = action(function () {
    this.value++
  })
  constructor() { initializeObservables(this)}
}

then we want to extend it,

class DoubleIncrementer extends Doubler {
  increment = action(function () {
    super.increment() // ?
    super.increment() // ?
  })
}

how would we do that considering that super wouldn't work there?

In #1864, you said

Don't bind methods in super classes, it doesn't mean in javascript what you
hope it means :-)

If MobX gives people a class-based tool, they may expect to be able to use class features like they do in plain JS without MobX.

The explicit one like initializeObservables(this, { foo: observable, bar: computed, etc })?

I suppose private class fields are out of the question, as there is no way to access those dynamically from initializeObservables(this, ...). They're locked out until decorators finally do land, it seems.

@mweststrate What happened with this idea to tackle that?

@trusktr

I think the example with value: computed(function () { ... }) is much better than the initializeObservables approach - the latter reminds me of MobX-without-decorators, which led to a lot of debugging because I would forget to mark certain fields as observable/action/computed. The "auto initialize" concept is interesting (similar to autobind) but there are cases where I don't care about a field being an observable, and if it's bad for memory/CPU then I'm fine without it.

reminds me of MobX-without-decorators

Yep, but in the constructor to overcome [[Define]] semantics of class fields.

Maybe that's best: it remains familiar and without performance cost of "auto initialize" (though an auto can be convenient in cases where the perf hit doesn't matter).

To clarify the original post didn't reflect anymore the new api of V6, instead we opted to implement the alternative api as found in the original post as discussed earlier in this thread. I just updated the post to better reflect the new api. So we won't be using value wrappers, but rely on makeObservable(this, map) instead.