Mobx: 🚀 Proposal: MobX 6: 🧨drop decorators,😱unify ES5 and proxy implementations, 💪smaller bundle

[mweststrate]

MobX 6

Hi folks, I've tinkered a lot about MobX 6 lately, so I want to layout the vision I have currently

Goals

🧨 1. Become compatible with modern ES standards

Let's start with the elephant in the room.
I think we have to drop the support for decorators.
Some have been advocating this for years, others totally love decorators.
Personally I hate to let decorators go. I think their DX and conciseness is still unparalleled.
Personally, I am still actively engaged with TC-39 to still make decorators happen, but we are kinda back to square one, and new proposal will deviate (again) from the implementation we already have.

Dropping decorators has a few advantages, in order of importance (imho)

1. Become compatible with standard modern JavaScript Since fields have not been standardized with [[define]] over [[set]] semantics, all our decorator implementations (and the decorate utility) are immediately incompatible with code that is compiled according the standard. (Something TypeScript doesn't do, yet, by default, as for TS this is a breaking change as well, unrelated to decorators). See #2288 for more background
2. MobX will work out of the box in most setups MobX doesn't work with common out-of-the-box setup in many tools. It doesn't work by default in create-react-app which is painful. Most online sandboxes do support decorators (MobX often being one of the few reasons), but it breaks occasionally. Eslint requires special setup. Etc. etc. Dropping decorators will significantly lower the entry barrier. A lower entry barrier means more adoption. More adoption means more community engagement and support, so I believe in the end everyone will win.
3. Less way to do things currently it is possible to use MobX without decorators, but it is not the emphasized approached, and many aren't even aware of that possibility. Reducing the amount of different ways in which the same thing can be achieved simplifies documentation and removes cognitive burden.
4. Reduce bundle size A _significant_ amount of MobX is decorator chores; that is because we ship with basically three implementations of decorators (TypeScript, Babel, decorate). I expect to drop a few KB by simply removing them.
5. Forward compatibility with decorators I expect it will (surprisingly) be easier to be compatible with decorators once they are officially standardized if there is no legacy implementations that need to be compatible as well. And if we can codemod it once, we can do that another time :)

The good news is: Migrating a code base away from decorators is easy; the current test suite of MobX itself has been converted for 99% by a codemod, without changing any semantics (TODO: well, that has to be proven once the new API is finalized, but that is where I expect to end up). The codemod itself is pretty robust already!

P.s. a quick Twitter poll shows that 2/3 would love to see a decorator free MobX (400+ votes)

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

I'd love to have MobX 6 ship with both Proxy based and ES5 (for backward compatibility) implementations. I'm not entirely sure why we didn't combine that anymore in the past, but I think it should be possible to support both cases in the same codebase. In Immer we've done that as well, and I'm very happy with that setup. By forcing to opt-in on backward compatibility, we make sure that we don't increase the bundle size for those that don't need it.

P.S. I still might find out why the above didn't work in the past in the near future :-P. But I'm positive, as our combined repo setup makes this easier than it was in the past, and I think it enables some cool features as well, such as detection of edge cases.

For example we can warn in dev mode that people try to dynamically add properties to an object, and tell them that such patterns won't work in ES5 if they have opted-in into ES5 support.

💪 3. Smaller bundle

By dropping decorators, and making sure that tree-shaking can optimize the MobX bundle, and mangling our source aggressively, I think we can achieve a big gain in bundle size. With Immer we were able to halve the bundle size, and I hope to achieve the same here.

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.

But I think that is a bridge too far as many already rely on these features (including Mobx-state-tree). Anyway I think it is good to avoid any further API changes beyond what is being changed in this proposal already. Which is more than enough for one major :). Beyond that, if goal 2) is achieved, it will be much easier to crank out new majors in the future :). That being said, If @urugator's proposal does fit nicely in the APIs proposed below, it might be a good idea to incorporate it.

4. 🛂Enable strict mode by default

The 'observed' one, that is.

🍿API changes

__UPDATE 22-5-20: this issue so far reflected the old proposal where all fields are wrapped in instance values, that one hasn't become the solution for reasons explained in the comments below__

This is a rough overview of the new api, details can be found in the branch.

To replace decorators, one will now need to 'decorate' in the constructor. Decorators can still be used, but they need to be opted into, and the documentation will default to the non-decorator version. Even when decorators are used, a constructor call to

class Doubler {
  value = 1 

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

  increment() {
    this.value++
  }

  constructor() {
    makeObservable(this, {
      value: observable,
      double: computed,
      increment: action
    })
  }
}

• If decorators are used, only makeObservable(this) is needed, the type will be picked from the decorators
• There will be an makeAutoObservable(this, exceptions?) that will default to observable for fields, computed for getters, action for functions

Process

• 
  
  1. [x] Agree on API
  
  
• 
  
  1. [x] Implement code mod
  
  
• 
  
  1. [x] Implement API changes
  
  
• 
  
  1. [x] Try to merge v4 & v5
  
  
• 
  
  1. [x] Try to minimize build
  
  
• 
  
  1. [ ] Update docs (but keep old ones around)
  
  
• 
  
  1. [x] Provide an alternative to keep using decorators, e.g. a dedicated babel transformation or move current implementation to separate package?
  
  
• 
  
  1. [ ] Write migration guide
  
  
• 
  
  1. [ ] Beta period?
  
  
• 
  
  1. [ ] Create fresh egghead course
  
  

Timeline

Whatever. Isolation makes it easier to set time apart. But from time to time also makes it less interesting to work on these things as more relevant things are happening in the world

CC: @fredyc @urugator @spion @Bnaya @xaviergonz

[spion]

You probably already know this :grinning: 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.

[Bnaya]

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

[urugator]

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...

[xaviergonz]

🧨 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.

[kubk]

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?

[FredyC]

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.

[webernir]

April fools @mweststrate ?

[mweststrate]

No, being serious here 🙈. Missed opportunity though!

Op do 2 apr. 2020 06:50 schreef Nir Weber [e-mail]:

April fools @mweststrate https://github.com/mweststrate ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-607635350, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBGYSQOTJNWFY5ZPUYLRKQRTHANCNFSM4LZQ2B5Q
.

[mweststrate]

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.

[benjamingr]

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).

[benjamingr]

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++
  }
});

[FredyC]

• 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? 🤓

[mweststrate]

Doh, I can't count.

Subclassing doesn't work, as the fields are not visible to the subclass
when it runs its constructor. Wrapping is probably a bit scary for many
people, but more importantly it is troublesome in TypeScript, as generic
arguments aren't nicely preserved that way, and with circular module
dependencies, as const expressions are not hoisted the same way as classes
IIRC (that is an recurring issue in MST).

On Thu, Apr 2, 2020 at 10:56 AM Daniel K. [e-mail] wrote:

>

• 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 https://github.com/mweststrate Just realized, why
MobX 7? Do we need to skip V6 for some reason? 🤓

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-607744180, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBE6PWAMDR7QF4WVJFDRKROL7ANCNFSM4LZQ2B5Q
.

[benjamingr]

@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?

[mweststrate]

@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.....

[spion]

@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)

[benjamingr]

@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.

[benjamingr]

@spion

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).

TC39 has legitimate considerations and concerns regarding decorators. TC39 does not owe us decorators and people have been collaborating in good faith.

Like most TC39 stuff - this is blocked on people actually doing the work and heavy-lifting to help address all the concerns (usability, implementation for engines, spec etc).

If you would like to help push decorators along I recommend you do the same thing as Michel and get involved. If you are not sure how to get involved I can happily connect you to some TC39 members that can help point you towards ways to contribute.

This (like most proposals and things) is harder than it sounds to do right, the default in TC39 for things that are half-baked is to not progress with them and force of will and determination is usually required.

[spion]

@benjamingr TC39 owes a lot more than a 4-page hand-wavy document. For a feature already used by thousands of projects including the most popular NodeJS framework and one of the most popular frontend frameworks. I would expect to see a proper design doc discussing the alternatives, pros & cons and elaborating on engine constraints in deep detail.

[benjamingr]

@spion I think we might have an expectation problem with how much TC39 owes anyone and us in particular. I also have a different expectation regarding how much detail parties have to provide to third parties like us. This is also true for projects (like Node.js) where the project has TC39 representation.

In any case these people aren't very far and they are pretty accessible, if you want to understand these concerns and get involved feel free to hit me up on FB and I'll connect you to members happily.

I would prefer to keep future discussion in this issue focused on the MobX 7 proposal and API
bikeshedding :]

[spion]

I understand the concerns, but I see at least a few avenues that are left unexplored. I will likely try to get involved. (One of the unexplored options is "banned" syntax that will never be standardized, to guarantee that transpilers can do their compile-time decorator work, then TypeScript can proceed with confidence)

Regardless I think its extremely important for userland libraries and users to continue to signal that we care about decorators, precisely because TC39 has limited resources and has to prioritize based on some criteria. One of those criteria is likely interest.

[FredyC]

Guys, let's not dive into TC39 problems here, this is about MobX. If you want to argue, take it at least to them, they won't see it here.

[urugator]

maybe we should introduce a separate 'marker' to for observable properties

That was sort of the idea :D. I used field = as(options, value), the type is infered from value/options combination, like { type: computed } or { computed: true }.
But it still poses the problem with return type or not? For fake array, T=>T isn't possible as well...?
It still seems to require a lot more "tinkering" than the seperate options map, which on the other hand looks quite clean to me. The only disadvanatage being it's not colocated. But my reasoning is that with default auto behavior one should need this extra map a lot less often and no one complained about decorate so far(?).

I also wanted to ask how that es5/next configuration works, does it require bundler/tree shaking?

[mweststrate]

For fake array/map, T=>T isn't possible as well...?

Strictly speaking not, but that has always been that way already, @observable field = [] doesn't expose the MobX utilities either, but generally that seems to be fine for everybody.

_Edit: actually the proposal will make it better, gives some flexibility, because it could produce the signature T[] => IObservableArray<T> for example so that the utility methods do become visible. Not sure if that is a good idea, as that would make field assignments harder_

It still seems to require a lot more "tinkering" than the separate options map, which on the other hand looks quite clean to me.

Yes, I agree, I'm still 60/40 on this approach. Using a decorate map is probably more efficient, and is definitely easier to migrate from and to decorators. The biggest drawback I really see, is that it is more prone to making mistakes. People already make a lot of mistakes with observer. And using a decorate map is very prone to both forgetting and refactoring errors, especially when not using TS. I'll update the proposal with this alternative though, for completeness.

[mweststrate]

I also wanted to ask how that es5/next configuration works, does it require bundler/tree shaking?

Yes, the idea is to put the entire ES5 implementation in a closure (or make sure that it is only referred by that closure), so that if the opt-in method is never called, it will be tree-shaken away. That is how it now works in immer

[matijagrcic]

Alternative API is great, the explicit action to
initializeObservables trumps the DX. And VSCode refactoring can probably make this an action based on code in the class.

[urugator]

The biggest drawback I really see, is that it is more prone to making mistakes.

Therefore the suggested auto behavior and strict checking by default. If there is a chance you forgot or misused anything it should immediately throw.
Ideally you should just check console, if there isn't any error, everything should work as intended, if there are some, follow the instructions.

that would make field assignments harder

That was my concern (that it returns ObservableArray or BoxedPrimitive instead of just array/primitive), not the lack of utilities... just dunno how this is handled

[szagi3891]

class Doubler {
  value = observable(1)

  double = computed(function () {
    return this.field * 2
  })

  increment = action(function () {
    this.value++
  })

  constructor() {
    initializeObservables(this)
  }
}

Maybe it is worth reducing the memory consumption?

With each new instance of such a class, two new clouseras will be created. The problem is made when we have a lot of models and they have a lot of methods. Memory consumption increases exponentially.

A zero cost abstraction approach would be good.

[mweststrate]

@szagi3891 they are partially avoidable (see the notes), but that is definitely a good argument for the alternative api, where that is easier.

[mweststrate]

@benjamingr good point, @spion proposed proxy trapping constructors as well here, I thought I had tried and it didn't work (typewise), but reading back that conclusion is nowhere, so I have to re-investigate that :)

[mweststrate]

@urugator good reminder, I think we should definitely enable strict mode in this major as well

[urugator]

Did a quick naive test in chrome (results in comments):

<script>
  // bound
  function onClick() {
    console.log('start');
    const o = {};
    class Clazz {
      constructor() {
        this.fn = this.fn.bind(this);
      }
      fn(arg1, arg2, arg3) {
        console.log(arg1, arg2, arg3);
      }
    }
    for (let i = 0; i < 100 * 1000; i++) {
      o[i] = new Clazz();
    }
    console.log('end');
    window.document.getElementById("status").textContent = "DONE";
  }
  /*
  JS Heap by iterations:
  10k = 1.6MB
  100k = 6.6MB
  1M = 52MB
  10M = 519MB
  */
</script>

<button onclick="onClick()">Run</button>
<div id="status"></div>

<script>
  // prototype
  function onClick() {
    console.log('start');
    const o = {};
    class Clazz {
      fn(arg1, arg2, arg3) {
        console.log(arg1, arg2, arg3);
      }
    }
    for (let i = 0; i < 100 * 1000; i++) {
      o[i] = new Clazz();
    }
    console.log('end');
    window.document.getElementById("status").textContent = "DONE";
  }
   /*
  JS Heap by iterations:
  10k = 1.5MB
  100k = 3.2MB
  1M = 25MB
  10M = 263MB
  */
</script>

<button onclick="onClick()">Run</button>
<div id="status"></div>

[seivan]

If I can give my 2 cents,

I like using classes for my stores, and I like things being predictable which means I always use arrow functions for my stores so I know what this means.
I also love using decorators, but only use it for Mobx (as of now), and I actually quite like it.

Though I understand the reasoning for removing them, I only hope whatever the alternative is, can be just as non-intrusive as decorators currently are for me as a consumer .

I would love something like this:

class Doubler {
  value = observable(1)
  implicitValue = 1

  double = computed(() =>  {
    return this.value * 2
  })

  increment = action(() =>  {
    this.value++
  })

  betterIncrement = (with: number = 1) =>  {
    this.implicitValue += with
  })

  constructor() {
   //Should this get called before values are assigned in the constructor or after? 
    autoInitializeObservables(this, {only: ["increment", "implicitValue"]})
  }
}

Since the other members are explicitly set as actions and observables, the initializer would just do it for the ones defined.
Would love an only and/or except version of the autoInitializeObservables.

Now seeing this issue, I'll be moving towards this API for existing code, as the writing is on the wall for decorators. If there's shaky support for it, it's better not to use it.

decorate(Doubler, {
   value: observable
    implicitValue: observable,
    betterIncrement: action,
    betterIncrement: action,
})

The downside is, that it is error prone.

Also curious, in the alternative API what type would value have in this case?

I like it that Mobx hides the actual types when using Typescript letting me work with what I am expecting instead of wrapping everything in Observables<T>.

I generally prefer explicitness, but it's a nice way of not being obtrusive and letting me refactor when I need to, or even remove Mobx as my components wouldn't rely on them. I've always appreciate that, and I'd hate to see that go away.

However, in classes, we typically want more fine grained control, and I think the above should be avoided typically.

I don't agree, I only use classes for Mobx, and with that in mind it's because I want thing to be observables out of the box without me thinking about manual input, though I understand the need for granular control, I just hope it's not enforced when using classes.

Out of curiosity, will setters automatically be converted to actions if the getter is defined as it currently is?

[vivainio]

Also consider new package name for the decorator-less api (mobx-core?). Full break from the existing API often deserves it.

[tkrotoff]

1. Support proxy and non-proxy in the same version

I wouldn't do that. IE is the only platform without Proxy support.

• https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Proxy#Browser_compatibility
• https://caniuse.com/#feat=proxy

IE global market share is currently at 1.71%

Even Bootstrap v5 drops IE: https://github.com/twbs/bootstrap/pull/30377

By the time MobX 6 is released, IE will be irrelevant.

[elektronik2k5]

I wouldn't do that. IE is the only platform without Proxy support.

Unfortunately, that's only true for the Web. But JS and mobx run on other platforms too. Namely, React Native is still bundled with an old version of JavaScriptCore which doesn't support proxies. :cry:
Furthermore, Facebook's new JS runtime "Hermes" for Android also doesn't support proxies, yet.

And those are just the examples I know of. There are more esoteric JS runtimes out in the wild, which we should support.

BTW, as much as I'd love to ditch IE (which I personally refused to work on for the last several years), that's just not an option for a LOT of corporate/legacy codebases.

[seivan]

I think it's great if Mobx6 is going to support both ES5 and ES6, as if it's combining the implementation of Mobx4 and Mobx5 in a single package.

We target Mobx4 because we want to support IE11, but we're doing two outputs per target, and support both ES5 and ES2015 (ES6) via <script nomodule> and <script type="module">

My question is, how can we use Mobx6, and pick Proxy for ES2015 bundle and old Mobx4 implementation (observable?) for ES5 outputs, any docs or ideas prepared for this?

[mweststrate]

@seivan yes, that would be a matter of calling something like enableES5Support() in one of the bundles, and not in the other one. That could be done in an entrypoint, if you have different ones, or behind a environment variable that is compiled away, depending your setup.

[mweststrate]

Ok, I think I'm leaning more towards the alternative proposal after giving this some more thought for a couple of reasons:

1. sharing methods and thereby reduced memory footprint for actions and computes
2. no subtleties in (not) binding context nor in types
3. less noisy syntactically
4. easier to make compatible with contemporary and future decorator implementations

The biggest downside still being: no co-location

Thinking about the API, two ideas:

A decorator map, so it would look like

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

Calling just makeObservable(this) would automatically mark all fields with the best fitting thing

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, exceptionsMap?) looks a bit ugly, although it is not too bad

A fluent interface

constructor() {
  makeObservable(this)
    .observable("field1", "field2")
    .observable.shallow("field3")
    .action("method")
}

Downside is that strings always look ugly, despite being type-checked. Upside is that it is less verbose for large classes as observable is not repeated over and over again for each member (and requires no additional imports).

Exceptions are easy as well, for example if we don't want to mark field1 as observable, field2 should be shallow, and all others should be observables that could be expressed like:

constructor() {
  makeObservable(this)
   .ignore('field1')
   .observable.shallow('field2')
   .auto() //whatever seems fit for all the others
}

No auto decorating for subclassed classes

A subtlety is that we probably should forbid automatically decorating member if there is a super class, as we can't detect which members come from this class and which one from the super, and marking all fields could break internal assumptions of the super class.

[FredyC]

Downside is that strings always look ugly, despite being type-checked.

Um, your first example has strings too, just quotes are omitted from object keys ;) But yea, TypeScript would help a lot with that. I like the first variant more, fluent feels way too noisy. And exceptions are easy in the first approach too...

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

No auto decorating for subclassed classes

Isn't that a good thing? Inheritance can be so confusing. The composition usually works better.

[szagi3891]

@mweststrate
Experiment :)
And what if you tried to go down a level?

type Getter<T> = () => T;

const firstName: MobxValue<string> = new MobxValue("John");
const secondName: MobxValue<string> = new MobxValue("Black");
const full: MobxValue<boolean> = new MobxValue(false);

const computedFullName: MobxValue<string> = computed(
    firstName,
    secondName,
    function(getFirstName: Getter<string>, getSecondName: Getter<string>): string {
        const name = getFirstName();             //subscribe for firstName
        const surname = getSecondName();         //subscribe for secondName
        return `${name} ${surname}`;
    });


const computedForDisplay: MobxValue<string> = computed(
    full,
    firstName,
    computedFullName,
    function(getFull: Getter<boolean>, getFirstName: Getter<string>, getFullName: Getter<string>): string {
        const full = getFull();

        if (full === true) {
            return getFullName();           //subscriptions for "full" and "computedFullName"
        }

        return getFirstName();              //subscriptions for "full" and "firstName"
    }
);

console.info(computedForDisplay.get());     //should show "John"
full.setValue(true);
console.info(computedForDisplay.get());     //should show "John Black"

In such api you can easily share combinator functions without unnecessary memory allocation.

[FredyC]

@szagi3891 How is that related to classes? There are obviously cleaner ways when you go functional, but some people prefer classes for reflection and stuff.

[urugator]

sharing methods and thereby reduced memory footprint for actions and computes

Imo unless someone shows some really concerning numbers, memory consumptions shouldn't be a major argument.
I don't want to give that test above much relevancy, but it shows that if you have under 10k functions (which is quite a lot already) there is no noticable difference and for anything above it's 2x more memory.
With 10 milion times more functions, the memory doesn't grow milion times more, but only twice as much.
But even if it would be more, it doesn't matter. If a memory is a real concern for you, you won't be creating thousands of instances in the first place. Actually you may not even use mobx.
If your app takes too much memory it's not because mobx autobinds functions, but because it's poorly designed.
On the other hand keeping functions on the object is practical and can simplify a lot I believe (impl wise and ux wise).

"auto" decorating with some exceptions

ignore decorator. I think we will need an internal "decorator" marking the field ignored anyway.
So why not exposing it and keeping it simple/transparent.

A fluent interface

The pattern is used to help with building/configuring objects (often immutable).
But the premise is that the manual configuration should be really rarely needed (due to sane automatic defaults). Therefore I don't think it adds much of a value.

we can't detect which members come from this class and which one from the super, and marking all fields could break internal assumptions of the super class.

If everything is observable/action/computed by default, then we know which fields must not be made observable, because they must have been marked as ignored in superclass.
EDIT: That's the whole idea - by default everything is under our control, therefore we can make various assumptions and even eg throw on places we couldn't before...

[mweststrate]

Yeah I don't think we need any changes in dealing with non-classes. Also in
mechanisms like the above getting this and self referencing types is
really tricky, that is a recurring painpoint in MST as well.

On Sat, Apr 4, 2020 at 4:11 PM Daniel K. [e-mail] wrote:

@szagi3891 https://github.com/szagi3891 How is that related to classes?
There are obviously cleaner ways when you go functionals, but some people
prefer classes for reflection and stuff.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-609042910, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBDXNEUEI5LWY4VPZELRK5EZ5ANCNFSM4LZQ2B5Q
.

[szagi3891]

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.

Imo unless someone shows some really concerning numbers, memory consumptions shouldn't be a major argument.

I had very big problems with memory usage.
I also had a problem with decorators. The application I use must create huge amounts of models and just initializing decorators took a lot of time.

[spion]

I don't like the explicit specification of observables. It was already easy enough to forget the @observable decorator for fields that need to be observable (a very common mistake, happens all the time). It is going to be even easier to forget if you have to specify it in the constructor, potentially several dozen lines away from the declaration of the properties, getters and methods.

autoInitializeObservables with opt-out is much better, but memory consumption for computers is a potential issue, and higher control for refs/shallow/deep observables is another. But I think it's going to end up very usable, potentially even better than explicit @observable decorators. Plus, it can also be used as a class-level decorator that does constructor trapping via proxy or something similar.

My question is, what problem are we trying to solve here? If it's the field initializers issue, we can still use decorators as pure metadata which will then be read by autoInitializeObservables. Decorators as pure metadata are not likely to go away and are the most likely subset that could get eventually standardized.

[mweststrate]

Deserves a more extensive answer, but I think the benefit of the alternative is that it is easy to keep supporting both decorators (just not as the default advertised approach), so all these 3 approaches would be valid:

class Test {
  field = 3

  constructor() {
    makeObservable(this, { field: observable }) 
  }
}

// or, leverage meta data emitted by decorators

class Test {
  field = 3

  constructor() {
    // doesn't need a second argument, but would accept a second argument with _exceptions_
    makeAutoObservable(this) 
  }
}


// or, leverage auto observable

class Test {
  @observable
  field = 3

  constructor() {
    // without the second argument, see if we have meta data or die
    makeObservable(this) 
  }
}

This still solves the problem of having a small bundle as the meta data decorators don't need to provide a semantic implementation, and babel has a plugin to support TS style decorators as well

I still don't see having proxies as having a clear benefit over a constructor, I think wrapping a class is a lot more 'advanced' for many JS devs than having a constructor

[olee]

I also like this option a lot.
I think it's a good idea to use decorators only for specifying metadata, as this will always be possible no matter the final decorators implementation will look like.
However I would like to propose some slight changes:

class TestAutoObservable {
  observableField = 3

  constructor() {
    // 2nd argument true => auto observable
    makeObservable(this, true)
  }
}

class TestAutoObservableAdvanced {
  observableField = 3

  unobservableField = 3

  constructor() {
    // 2nd argument true with 3rd argument options => auto observable with exclusions / adjustments
    makeObservable(this, true, {
      unobservableField: false, // false could be an alternative to a special "ignore" value
    })
  }
}

class TestManual {
  observableField = 3

  unobservableField = 3

  constructor() {
    // 2nd argument options => manual observable
    makeObservable(this, {
      observableField: observable,
    })
  }
}

class TestDecorator {
  @observable
  observableField = 3

  unobservableField = 3

  constructor() {
    // only manual decorator support (mixed decorators / options could also be possible)
    makeObservable(this);
    // makeDecoratorObservable(this)
  }
}

class TestDecoratorAuto {
  observableField = 3

  @ignore
  unobservableField = 3

  constructor() {
    // 2nd argument true => auto observable with decorators to ignore fields
    makeObservable(this, true);
    // makeDecoratorObservable(this, true)
  }
}

This implementation would only require a single function with an easy to understand API.

Another idea that might be useful would be to put the decorator support into a separate package (or separate import like mobx/decorator) which would provide a function like makeDecoratorObservable.
This function would then be a higher order function on makeObservable which would take the provided (or empty) options and fill it with the decorator metadata from the class.
This would allow completely excluding any code regarding decorator support from bundles which do not need it.

EDIT:
One more idea: I think it should also be possible to provide a function like makeClassObservable, which would take a class instead of this as the first argument with the remaining signature being the same as makeObservable.
This function would then return a subclass where the constructor automatically calls makeObservable after the call to super:

export const TestAutoObservable = makeClassObservable(class {
  observableField = 3
  actionMethod() {
    // do stuff
  }
}, true)

[FredyC]

@olee Sorry, but how is makeObservable(this, true, {...}) better than makeAutoObservable(this)? Not only it's longer, but boolean arguments are so confusing because people unfamiliar with API have to look up what it means. When you have named variant that clearly expresses difference, it's so much better DX-wise imo.

Let's not do that just to have a single universal overloaded function, that's the wrong reason. I like your makeClassObservable if that's possible, but once again rather than boolean arg I would go with makeClassAutoObservable or even makeClassObservable(class {}, { auto: true }) is better.

[olee]

Yeah when you say it like that, it definitely makes sense (that's also the reason why I added it just as an addition.

What about the other points?

• Allowing "false" as option value to make fields unobservable
• Extracting decorator logic into separate import/package which uses metadata-decorators to populate options

[mweststrate]

@olee I think the first one is neat. For the second one, that is probably only a couple of lines, so I will not will be worth the effort of separating it.

If everything is observable/action/computed by default, then we know which fields must not be made observable, because they must have been marked as ignored in superclass.

@urugator that assumes always using _auto_ observable in the superclass, and it assumes having the superclass under control in the first place. Simple counter example, using makeAutoObservable on a class MyComponent extends React.Component could blow up a lot React internals we don't even know about.

[olee]

But the decorator support could be built tree-shakeable, I guess?

[szagi3891]

https://blog.logrocket.com/new-decorators-proposal/?amp=1
This part is interesting: "Problems with JIT and decorators"

[urugator]

@mweststrate Idea:
makeObservable - anything that isn't listed in decorator map is decorated with ignore. So when you call makeAutoObservable in subsclass it won't attempt to redecorate already decorated fields.

makeAutoObservable always throws when called from a subclass of parent that hasn't called
makeObservable/makeAutoObservable. That is to prevent messing up with "uncontrolled" classes in general.

[olee]

makeObservable - anything that isn't listed in decorator map is decorated with ignore. So when you call makeAutoObservable in subsclass it won't attempt to redecorate already decorated fields.

This might not work well, because there could be class fields you don't know about.
For example a ts field declaration like public unobservedValue?: number; in the parent class would not be picked up by this strategy if it does not have any decorator on it.
But for fields that have an initialized value this might be an option.

However, the more I think about this whole solution with makeObservable in general, the more I see it possibly becoming incredible performance heavy.

• Collecting all that decorator metadata
• Observableizing all the fields on each object construction
• Memory impact on storing (parent) class field information

I think it would be great to have a kinda PoC early to investigate how the performance of such a solution would compare to previous decorators. Could this be done maybe with current's mobx extendObservable as a test?

makeAutoObservable always throws when called from a subclass of parent that hasn't called makeObservable/makeAutoObservable. That is to prevent messing up with "uncontrolled" classes in general.

This should definitely not happen - it would prevent adding observable fields for subclasses where you do not have control over the parent class.

[urugator]

public unobservedValue?: number;

Hm, if I follow correctly, such field won't be picked even by makeAutoObservable, so it shouldn't break the parent, but it seems like another problem of it's own.

This should definitely not happen - it would prevent adding observable fields for subclasses where you do not have control over the parent class.

It wouldn't, the point is that you are forced to use makeObservable (be explicit about what will be observable) instead of makeAutoObservable. That is to somehow address "No auto decorating for subclassed classes", so it's not breaking things silently.

[olee]

Well you said

makeAutoObservable always throws when called from a subclass of parent that hasn't called makeObservable/makeAutoObservable

But I guess you rather mean that only makeAutoObservable should throw if the parent class didn't call makeAutoObservable?
Because otherwise it is as I said - it would prevent adding observable fields to subclasses where the parent wasn't observable - which is just a different wording of the quote above, ain't it?

[urugator]

class NonObservable {};

class SubclassOfNonObservable extends NonObservable {
  constructor() {
    super();
    makeAutoObservable(this) // throws because parent didn't called makeObservable/makeAutoObservable
    makeObservable(this); // never throws - allows adding observable fields
  } 
}

class Observable {
  constructor() {
    // Any of
    makeAutoObservable(this)
    makeObservable(this)    
  }
}

class SubclassOfObservable extends Observable {
  constructor() {
    super();
    makeAutoObservable(this) // doesn't throw because parent called makeObservable/makeAutoObservable
    makeObservable(this); // never throws
  }
}

[olee]

Yeah that makes sense, thanks for the example 😅

[unadlib]

@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();
  }
}

[mweststrate]

Requiring a base class removes flexibility, for example using decorators in
react component classes

Op di 7 apr. 2020 06:45 schreef Michael Lin [e-mail]:

@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
https://github.com/mobxjs/mobx/issues/2325#issuecomment-610186845, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBH3JOHOH3TMXU56E23RLK4V5ANCNFSM4LZQ2B5Q
.

[unadlib]

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 [e-mail]:
…
@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 .

[szagi3891]

@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?

[mweststrate]

Too early to tell but I don't expect any significant memory gains.

Op za 11 apr. 2020 19:51 schreef Grzegorz Szeliga <[email protected]

:

@mweststrate https://github.com/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?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-612489057, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBHI56FCAMTG43ORPB3RMC32XANCNFSM4LZQ2B5Q
.

[robbiewxyz]

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.

[inoyakaigor]

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?_

[mweststrate]

They won't be dropped :-P. I was just wishful thinking :)

On Thu, Apr 16, 2020 at 4:15 PM Igor «InoY» Zvyagintsev <
[email protected]> wrote:

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?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-614716511, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBFRTLUX2VV7FH6NC63RM4OKJANCNFSM4LZQ2B5Q
.

[seivan]

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.

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

decorate(Store, {
  _todos: observable
})

Edit: Added solutions and workarounds for the issues brought up: https://github.com/mobxjs/mobx/issues/2340#issuecomment-619160940

[trusktr]

I know this issue is for MobX, but I'm interested in the topic of dropping decorators in general (until a proposal actually has success), and this issue happens to be one of the top results on Google when searching for terms relating to moving away from decorators and TypeScript.

@mweststrate Regarding the example with the method,

  increment = action(function () {
    this.value++
  })

and then in the constructor initializing the decoration: the [[HomeObject]] of the method will not be set, and we will not be able to access super unfortunately, or if we use arrow functions super will not work as we'd like in subclasses (we won't be able to extend methods). This is a hindrance for method use in the general sense.

Would perhaps the decoration step in the constructor also store the method on the prototype (only the first time) just to make it possible for a subclass to do something like

  incrementMore = action(function () {
    SuperClass.prototype.increment.call(this)
    this.value++
  })

as an alternative to using super?

I haven't used MobX before; maybe actions in MobX don't need to call super.method()s anyways, but I'm still curious about methods in general for any lib moving away from decorators.

One problem with the decorative values (for lack of a term of the above pattern) is with Custom Elements and decorating fields that should map to/from DOM attributes.

Decorators allow us to do things like modify static observedAttributes based on decorated properties _before construction time_. With the decorative value pattern (setting class fields with special values then calling initializeObservables(this) in class constructors), the decoration happens too late: customElements.define calls read values from observedAttributes immediately, before any instance of a passed-in class is ever created, therefore if initializeObservables(this) adds items into the observedAttributes array after the element is already defined those attributes will never be observed; we can not instantiate a custom element class before calling customElements.define (it will throw).

For the Custom Element case, we have to use the older pattern that MobX already uses, f.e. calling a function after the class definition:

class MyEl extends WebComponent(HTMLElement) {
  foo = 'bar'
  bar = 'baz'
}

mapAttributesToProps(MyEl, ['foo', 'bar']) 
// ^ The WebComponent base class may utilize meta data in attributeChangedCallback for example

Another way could be to rely on pre-defined observedAttributes values:

class MyEl extends WebComponent() {
  static observedAttributes = ['foo', 'bar']
  foo = 'bar'
  bar = 'baz'

  constructor() {
    super()
    mapAttributesToProps(this)
  }
}

or use the explicit decorative values to avoid clobbering super properties that weren't meant to be mapped:

class MyEl extends WebComponent() {
  static observedAttributes = ['foo', 'bar']
  foo = attribute('bar')
  bar = attribute('baz')

  constructor() {
    super()
    initialize(this)
  }
}

Either way there's some unwanted duplication.

One of the best things about decorators is easily avoiding duplication while also being more terse.

With decorative values, how do we apply them to accessors (and not ruin super calls)? The nice thing about accessors and legacy decorators, is that setters can be the handlers for incoming attribute strings:

class MyEl extends WebComponent() {
  this._foo = 0
  this._bar = 0.0

  @attribute
  set foo(val) { this._foo = parseInt(val) }
  get foo() { return this._foo }

  @attribute
  set bar(val) { this._bar = parseFloat(val) }
  get bar() { return this._bar }

  constructor() {
    super()
    initialize(this)
  }
}

How would we use decorative values with get/set? Maybe we can have an accessor backed by a value-decorated field:

  this._foo = attribute(0)

  set foo(val) { this._foo = parseInt(val) }
  get foo() { this._foo }

  constructor() { super(); initialize(this) }

but that has various issues (like how the decoration would map foo instead of _foo (possible but easy to break), plus now _foo is probably a second accessor layer).

Is there another way?

Your "property read/write trapping decorators" proposal would make this terse and clean, eliminating the cache variables and eliminating duplication.

Something I've seen libraries do and that I'd like to avoid, is accept handlers in decorators, which I feel makes things harder to understand and read (especially when stepping over code in devtools due to indirection in the code paths):

  @attribute((value) => {
    // ... handle the input value ...
  }) foo = 0

(which only works in Babel with lose fields, or for now TypeScript by default but soon not, and hence the whole reason to avoid decorators which will save people headaches and confusion).

The equivalent of the last example with decorative values would be:

  foo = attribute((value) => {
    // ... handle the input value ...
  }, 0)

which is still somewhat awkward, but perhaps a little better considering that foo = now comes in front, so the intent of creating a property is still clear up front. Maybe we can swap the arg order to make it better:

  foo = attribute(0, (value) => {
    // ... handle the input value ...
  })

This is reasonable. It is almost as if we're creating or instantiating an attribute.

The bottom line is decorators will do wonders to code clarity and terseness. I _can't wait_ to use them. But for now, I agree it is better to stick to patterns that will work everywhere (Babel, TypeScript, or vanilla JS, etc) without any doubts, unlike the current decorators that might work perfectly in TypeScript only to fail utterly when a Babel user imports them (which is a nasty problem for any library to have). These problems cause time waste and headaches that library authors and end users don't want to have.

---

If only class fields didn't land with [[Define]] semantics, then keeping legacy decorators might at least be ok because they'd work consistently across environments...

Is there any chance implementors of major JS engines would ever consider moving class fields to [[Set]] semantics? We broke the web. Can we break it one more time to fix it?

[mweststrate]

@trusktr although I agree with the general sentiment, this is not the repository to discuss the future of JavaScript. That is what, in this case, https://github.com/tc39/proposal-decorators is for. So I'll gonna mark the post as off-topic

[molszanski]

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.

[Maaartinus]

https://github.com/mobxjs/mobx/issues/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.

[urugator]

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...

[urugator]

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())
})

[mweststrate]

@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

| method | base state | new state |
| ---- | --- | --- |
| track* | NONE | DERIVING |
| track | UPDATING | DERIVING |
| action | NONE | UPDATING |
| action | DERIVING | DERIVING |
| runInAction / allowStateChanges | * | UPDATING |
| untracked | * | NONE |

* 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
   
   
   
   1. batching keeps working as is
   
   

[urugator]

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)?

[trusktr]

@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 https://github.com/mobxjs/mobx/issues/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.

[mweststrate]

MobX 6 won't go with that proposal, but the second one

On Mon, May 4, 2020 at 2:47 AM Joe Pea [e-mail] wrote:

@mweststrate https://github.com/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 https://github.com/mobxjs/mobx/issues/1864, you said

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

But if MobX gives people a class-based tool, they expect to be able to use
class features.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-623224126, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBHML3MRNXDQZHSILX3RPYNBZANCNFSM4LZQ2B5Q
.

[trusktr]

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

[mweststrate]

Yes

On Mon, 4 May 2020, 07:03 Joe Pea, [e-mail] wrote:

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

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-623274109, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBDJANONQFSOPHE3Z3LRPZLEVANCNFSM4LZQ2B5Q
.

[trusktr]

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]

Correct

Op zo 10 mei 2020 19:03 schreef Joe Pea [e-mail]:

I suppose private class fields are out of the question, as there is no way
to access those dynamically from initializeObservables(this, ...)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-626365889, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBF4G6AZKWOGKN4MOQ3RQ3T5JANCNFSM4LZQ2B5Q
.

[seivan]

@mweststrate What happened with this idea to tackle that?

@trusktr

[mweststrate]

The last comment was about JS privates, not TS privates

On Mon, 11 May 2020, 01:08 Seivan, [e-mail] wrote:

@mweststrate https://github.com/mweststrate What happened with this idea
https://github.com/mobxjs/mobx/issues/2339#issuecomment-619376221 to
tackle that?

@trusktr https://github.com/trusktr

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-626411131, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBD6I5ARFPRIXUU2YVTRQ46WBANCNFSM4LZQ2B5Q
.

[Dakkers]

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.

[trusktr]

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).

[mweststrate]

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.

[danielwalczak]

I love @robbiewxyz idea, lets simplify API, I would like to see better models API

[faassen]

I'm trying to understand the behavior of makeObservable and makeAutoObservable correctly so I can document them properly.

From the code I see that if you call makeObservable without arguments and no decorator members are found, it errors out. I think that's good.

I imagine this may be difficult to implement, but it would be nice if we could get some kind of warning if you forget to call makeObservable and use decorators.

What should happen if you call makeObservable with an annotations argument but there are also decorators? Should it complain?

Concerning makeAutoObservable, I see its exceptions argument is an annotation map. How do you then exclude a particular key from being made observable? I think that's done with 'false' if I read the type correctly.

What should happen if you use makeAutoObservable on a class that uses decorators?

I think it's interesting to consider that since the names and signatures of makeObservable and makeAutoObservable are quite similar what happens if someone mixes them up by accident. What are the scenarios where this would really confuse developers, and are there ways we can help?

[faassen]

Some comments about the 'undecorate' codemod. This turns decorator-based code into code that doesn't use decorators, and also converts code that uses 'decorate'.

Since MobX still supports decorators with makeObservable, should there be a codemod that only adds makeObservable(this) everywhere? [edit: I see that there is a keepDecorators option to the codemod, though no tests for it yet]

Should there a feature that uses makeAutoObservable when it detects this is possible?

[mweststrate]

Ok, pushed some changes to v6 branch, I think there is now a decent solution to the is-a-function-an-action-or-derivation dance. I introduced a separate concept of autoAction, that is to be used primarily by library functions, not directly by users, that is used by makeAutoObservable, observable(object) and the lite hooks. autoActions will on the fly choose whether they act as action (if not deriving anything already) or derivation (when inside another derivation). They batch in both cases, does only untracked in the first case, and only allows state changes in the first case as well.

If the user does updates in an autoAction that is called from a derivation, it will warn that it must be wrapped in runInAction to clarify the intend. I also updated that the predicates of when and reaction don't allow state changes by default, just like computed, for consistency.

[mweststrate]

@faassen

I imagine this may be difficult to implement, but it would be nice if we could get some kind of warning if you forget to call makeObservable and use decorators.

Would be awesome, but no idea how to go about that :) We could maybe at random places (e.g in observableValue) check in DEV mode if a value is a class instance with decorators, but with undecorated members. Might be a bit of a performance bummer though.

What should happen if you call makeObservable with an annotations argument but there are also decorators? Should it complain?

I think it should be ok? not sure :)

Concerning makeAutoObservable, I see its exceptions argument is an annotation map. How do you then exclude a particular key from being made observable? I think that's done with 'false' if I read the type correctly.

Correct

What should happen if you use makeAutoObservable on a class that uses decorators?

Decorate remaining members? or die? Not sure :)

I think it's interesting to consider that since the names and signatures of makeObservable and makeAutoObservable are quite similar what happens if someone mixes them up by accident. What are the scenarios where this would really confuse developers, and are there ways we can help?

No idea :)

Since MobX still supports decorators with makeObservable, should there be a codemod that only adds makeObservable(this) everywhere? [edit: I see that there is a keepDecorators option to the codemod, though no tests for it yet]

Correct. Feel free to add 😅

Should there a feature that uses makeAutoObservable when it detects this is possible?

No I don't think so. MobX is already considered to be quite magically so I think simple > short. Especially if the code is generated anyway 😄 . I think we can apply the same simple > short principle to the docs, thinking about it.

P.s. for detailed semantic questions, I recommend to ask them on the v6 PR rather than in this thread, as it might spam a lot of subscribers :)

[faassen]

Something that I imagine needs to be in mobx-react is a codemod that converts the observer from a decorator into a function call. I mention it here as I don't see it in notes.md yet.

[FredyC]

Something that I imagine needs to be in mobx-react is a codemod that converts the observer from a decorator into a function call. I mention it here as I don't see it in notes.md yet.

Do you mean for class components? I don't think it necessary, at least not at the moment. There are no technical difficulties with supporting @observer. In fact with class components, it feels like an easier way. Chances are that people will eventually migrate toward functional components so it's probably premature.

[jeremy-coleman]

Just a personal observation, but i too want to use mobx without decorators. I have refactored 500-1k line class stores to use non-decorator syntax, easy peasy. But have you considered the cost of maintaining the non-decorated models compared to the inline decorator syntax? Its 100% impossible for humans to mentally keep track of 2 objects that large , so you have to comment above each class member as //action, obs, obs.ref, computed etc. So if you’re going to do that, why not just inline decorate? jsx / vue / angular / svelte etc arent es spec either , so does being es spec compliant really matter when you have to transpile anyway? I too strongly want to run mobx code without transpiling first, but when i self reflect , i cant help but think its just something i have to deal with because the alternatives are worse

[faassen]

Since we're changing defaults anyway, should the computedRequiresAction be the default in MobX 6? Or perhaps in the spirit of "there should be only one way to do it" remove it entirely as the docs say (somewhat unclear to me): "Though this restriction is confusing and contradictory Computeds can be altered to work in a direct access manner with some of the following methods..."

[elektronik2k5]

I wouldn't remove computedRequiresAction entirely cause some people like it. I definitely support making it the default though!

[johnhamm]

I agree with what seems like the majority sentiment here - to keep decorators. There hasn't been any stated legitimate reason to remove them honestly. This whole "conundrum" just seems odd to me.

[FredyC]

@johnhamm Have you actually read Goals in the first comment? There is a bunch of legitimate reasons for such a decision.

[olee]

Also, afaik decorator support will not be removed.
Instead it's just that decorators are not the primary implementation method and instead in the new implementation they are just providing metadata which the make-observable function can use to know how to handle which field.

@mweststrate does the new v6 branch already contain decorator support? Or is it for now code-configuration only?
If not, did you already decide whether decorator support will be a separate package & when it will be added?

[mweststrate]

yeah they are in the v6 branch already and don't require a separate package.

On Wed, Jul 15, 2020 at 4:22 PM Björn Zeutzheim [e-mail]
wrote:

Also, afaik decorator support will not be removed.
Instead it's just that decorators are not the primary implementation
method and instead in the new implementation they are just providing
metadata which the make-observable function can use to know how to handle
which field.

@mweststrate https://github.com/mweststrate does the new v6 branch
already contain decorator support? Or is it for now code-configuration only?
If not, did you already decide whether decorator support will be a
separate package & when it will be added?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-658832152, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBCS3RD6HAAZL56ITJDR3XCTNANCNFSM4LZQ2B5Q
.

[nighca]

Seems that the decorator discussion starts from the [[Set]] semantics -> [[Define]] semantics change introduced by the new class fileds proposal. However I found there is possibility to decide the semantics by decorators in current decorators proposal (stage 2), here are some details:

• The tool @initialize
• Also a mobx-observable-like decorator example @tracked

IMO if the decorators proposal went to stage 3, with supports from tools like babel / typescript, it will not be a problem for us to keep current decorator-way of using MobX. I'm confused why nobody here mentioned that.

[mweststrate]

There is currently no decorator proposal ready to go to stage 3, they've
been all send back to the drawing board by the committee, so it is unlikely
we will have anything in stage 3 in less than 2 years.

Op di 28 jul. 2020 04:40 schreef Hanxing Yang [e-mail]:

Seems that the decorator discussion starts from the [[Set]] semantics -> [[Define]]
semantics change introduced by the new class fileds proposal. However I
found there is possibility to decide the semantics by decorators in current decorators
proposal https://github.com/tc39/proposal-decorators (stage 2), here
are some details:

• The tool @initialize
  https://github.com/tc39/proposal-decorators/#initialize
• Also a mobx-observable-like decorator example @tracked
  https://github.com/tc39/proposal-decorators/#tracked

IMO if the decorators proposal went to stage 3, with supports from tools
like babel / typescript, it will not be a problem for us to keep current
decorator-way of using MobX. I'm confused why nobody here mentioned that.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-664757728, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBHSVX4WFC72XJJUZ4TR5ZCEBANCNFSM4LZQ2B5Q
.

[episage]

I think it would be more beneficial for MobX to stay focused and unopinionated. Providing a core package and sibling packages for deep integration with other frameworks will make MobX a more efficient and versatile tool. Especially regarding bundle size which may be very important. At the moment it is 15kB which some people find to be a large showstopper.

In particular, I am talking about frameworks which focus on small bundle size and speed such as Svelte. Svelte's runtime is ~3kB and provides a similar but not a comprehensive alternative to MobX.
Also, I totally agree that 15kB might not matter that much in React +40kB bundle. However, it still contributes ~30% to the runtime size.

Let's make web efficient!

NB
Decorators are, obviously, very convenient but having them as a separate package would be double awesome

[charliematters]

I appreciate that the decorators are still up in the air for version 6, but if we _are_ keeping them, are we expecting them to look and act like the ones from version 4/5?

I tried using the 6.0.0-rc1 branch and found that the @action.bound decorator doesn't bind functions any more.

I have some tests here which work for 4/5 but fail for 6 (although I can't get codesandbox and Mobx to agree about Symbols). I'm happy to refactor these bits out of our codebase when we move to 6, but I'm just trying to work out how much work that will be!

[mweststrate]

In MobX you will need to call makeObservable(this) (without
further arguments) in the constructor to apply the decorators. Beyond that,
they should behave the same (except for some property enumerability issues,
which were incorrect before)

On Mon, Aug 3, 2020 at 10:40 AM charliematters [e-mail]
wrote:

I appreciate that the decorators are still up in the air for version 6,
but if we are keeping them, are we expecting them to look and act like
the ones from version 4/5?

I tried using the 6.0.0-rc1 branch and found that the @action.bound
decorator doesn't bind functions any more.

I have some tests here https://codesandbox.io/s/distracted-frost-oi1eg
which work for 4/5 but fail for 6 (although I can't get codesandbox and
Mobx to agree about Symbols). I'm happy to refactor these bits out of our
codebase when we move to 6, but I'm just trying to work out how much work
that will be!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-667921306, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBDS4GUUMLX4OYCUWYTR62AZTANCNFSM4LZQ2B5Q
.

[mweststrate]

See https://deploy-preview-2327--mobx-docs.netlify.app/best/decorators.html

On Mon, Aug 3, 2020 at 10:56 AM Michel Weststrate [e-mail]
wrote:

In MobX you will need to call makeObservable(this) (without
further arguments) in the constructor to apply the decorators. Beyond that,
they should behave the same (except for some property enumerability issues,
which were incorrect before)

On Mon, Aug 3, 2020 at 10:40 AM charliematters [e-mail]
wrote:

I appreciate that the decorators are still up in the air for version 6,
but if we are keeping them, are we expecting them to look and act like
the ones from version 4/5?

I tried using the 6.0.0-rc1 branch and found that the @action.bound
decorator doesn't bind functions any more.

I have some tests here https://codesandbox.io/s/distracted-frost-oi1eg
which work for 4/5 but fail for 6 (although I can't get codesandbox and
Mobx to agree about Symbols). I'm happy to refactor these bits out of our
codebase when we move to 6, but I'm just trying to work out how much work
that will be!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-667921306, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBDS4GUUMLX4OYCUWYTR62AZTANCNFSM4LZQ2B5Q
.

[charliematters]

Thanks for the doc link - I knew it was likely something I'd forgotten. Adding that line makes all the tests pass.

[mweststrate]

@episage the original idea for MobX 6 was to have them separate indeed, in MobX 5 the decorator implementation size was signficant (3 times; TS, Babel, and decorate). But with makeObservable the decorator implementations have become so simple that the overhead code from separating it out and the additional complexity isn't worth it, since the implementation is now basically just https://github.com/mobxjs/mobx/blob/mobx6/src/api/decorators.ts, once minified the amount of code added by it can be measured in few hundred bytes.

[ashconnell]

Is MobX 6 an opportunity to make async actions part of the core api? Eg makeObservable and makeAutoObservable can convert generators to async actions (ala flow())?

It's always felt weird to define async actions differently to everything else:

class Auth {
  * login() {
    this.user = yield api.post('/login', { /*...*/ })
  }

  constructor() {
    makeObservable(this, {
      login: flow,
    })
    // ... or ...
    makeAutoObservable(this)
  }
}

[mweststrate]

Yeah, that is exactly one of my few remaining todo's to investigate for 6
:) I think the biggest annoyance will be that the inferred return type in
TypeScript will be incorrect (X instead of Promise)

On Mon, Aug 3, 2020 at 10:29 PM Ash Connell [e-mail]
wrote:

Is MobX 6 an opportunity to make async actions part of the core api? Eg
makeObservable and makeAutoObservable can convert generators to async
actions (ala flow())?

It's always felt weird to define async actions differently to everything
else:

class Auth {

• login() {
  this.user = yield api.post('/login', { /.../ })
  }

constructor() {
makeObservable(this, {
login: flow,
})
// ... or ...
makeAutoObservable(this)
}}

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-668250980, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBHMGD67DZZ2SWMB5STR64T2XANCNFSM4LZQ2B5Q
.

[zhangchao828]

mobx4、5 work well

mobx6 not work

I will put common code in BaseStore

[mweststrate]

@zhangchao828 , makeAutoObservable has to be called in the class that _declares_ the members, so, in your example in the BaseStore to make count observable

[zhangchao828]

@mweststrate still doesn't work

[mweststrate]

See the error, use makeObservable instead.

On Mon, Aug 17, 2020 at 3:08 PM 止水 [e-mail] wrote:

@mweststrate https://github.com/mweststrate still doesn't work
[image: 微信图片_20200817220637]
https://user-images.githubusercontent.com/15683846/90405261-16e5bf00-e0d6-11ea-9fd4-317026e4741d.png

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-674903806, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBH46X4BGPTPK5MFCXLSBE2XBANCNFSM4LZQ2B5Q
.

[zhangchao828]

@mweststrate think you ,this is ok , but I want to use makeAutoObservable,because it is easier, can you support it

in the Child I can not use makeAutoObservable

[mweststrate]

maybe later, but it is tricky, error prone, as it might or might not affect
members declared in subclasses, depending on how the code is actually
transpiled. So for now I want to keep things a bit simple, to keep them a
predictable. (In generally I'd recommend to avoid subclassing as much as
possible anyway, it is quite a footgun)

On Mon, Aug 17, 2020 at 3:44 PM 止水 [e-mail] wrote:

@mweststrate https://github.com/mweststrate think you ,this is ok , but
I want to use makeAutoObservable,because it is easier, can you support it

[image: 微信图片_20200817224216]
https://user-images.githubusercontent.com/15683846/90408803-f0765280-e0da-11ea-83c3-00d10b8fc566.png

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-674924151, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBHZ5HHCVVEQLZYJOPTSBE647ANCNFSM4LZQ2B5Q
.

[zhangchao828]

but where do i put the common code,mobx4,5 I have BaseStore,in mobx6 what should I do

[FredyC]

@zhangchao828 Ever heard about composition over inheritance? If you want some common code then instead of subclassing, just have a private (or public) instance of BaseStore inside your Child and access it there. You can even pass instance of Child when instancing BaseStore and with interfaces to represent common contract, the BaseStore can communicate with Child no matter what it is.

I really recommend you to read something about this pattern, it's not MobX specific. Inheritance has rarely useful benefits.

[trusktr]

Yes, composition might be a workaround.

But some people (like myself) like inheritance. The main problem here is not inheritance, but the [[Define]] semantics of the new ES class fields...

which ruins inheritance and causes issues that many people may have no idea about (especially if they come from other languages, which by the way seems to defeat the very purpose of class fields).

It is easy for anyone familiar with inheritance (especially in the general sense, regardless of language) to ask "can you support it?" because it _seems_ like it should be easy to support, but sadly the new class-fields language feature makes the ask very difficult to support.

One possible solution would be to tell everyone to never use class fields and assign only in the constructor, but the class field syntax is so darn convenient that people are inevitably going to use it and shoot themselves in the foot.

[zhangchao828]

@FredyC @trusktr because there are many such BaseStores in our old project,and it's easy to use,here is an example:
before use BaseStore:

then create BaseStore

after when I use Table, it is so easy

[mweststrate]

Let's keep this thread on the subject. TL,DR Superclasses are supported but
require a little more boilerplate. Beyond that how stores are organized is
up to the dev.

On Tue, Aug 18, 2020 at 11:31 AM 止水 [e-mail] wrote:

@FredyC https://github.com/FredyC @trusktr https://github.com/trusktr
because there are many such BaseStores in our old project,and it's easy to
use,here is an example:
before use BaseStore:
[image: 微信图片_20200817220637]
https://user-images.githubusercontent.com/15683846/90502128-f2deb800-e17f-11ea-8d4e-6af38016ca5a.png

then create BaseStore
[image: 微信图片_20200818182651]
https://user-images.githubusercontent.com/15683846/90502475-7698a480-e180-11ea-9367-d3fe62562e09.png

after when I use Table, it is so easy
[image: 微信图片_20200818182850]
https://user-images.githubusercontent.com/15683846/90502611-b65f8c00-e180-11ea-9fa3-af077e0075f1.png

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-675399798, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBHCPF2ZQMIDMBSRXXLSBJKBTANCNFSM4LZQ2B5Q
.

[danfma]

Guys, if you need to handle the inheritance problems and you are using Typescript, maybe you could give a try to this library https://github.com/danfma/mobx-activator. I'm still organizing the code, but it is working well until now.

You will apply a decorator, that will be removed at compile time for a function call in the constructor, which is a wrapper over the makeObservable function of Mobx.

[trusktr]

Is this how to do subclassing?

class Foo {
  @observable foo = 123

  constructor() {
    makeObservable(this)
  }
}

class Bar extends Foo {
  @observable bar = 456

  constructor() {
    super()
    makeObservable(this)
  }
}

class Baz extends Bar {
  @observable baz = 'hello'

  constructor() {
    super()
    makeObservable(this)
  }
}

const baz = new Baz

autorun(() => {
  console.log(baz.foo, baz.bar, baz.baz)
})

Does the decorator code (https://github.com/mobxjs/mobx/blob/mobx6/src/api/decorators.ts) work regardless if Babel decorators are in legacy mode or not, and with TS decorators?

Decorator scenarios to test:

• TS, useDefineForClassFields:false
• TS, useDefineForClassFields:true
• Babel, class-properties loose:false, decorators legacy:false
• Babel, class-properties loose:true, decorators legacy:false
• Babel, class-properties loose:false, decorators legacy:true
• Babel, class-properties loose:true, decorators legacy:true

EDIT,

Ah, based on the code

https://github.com/mobxjs/mobx/blob/ca40b7f7abab7f3110b382473967c678bb6bcad0/src/api/decorators.ts#L76-L80

Yes, the super class skips props that will be defined by the subclass and handled by the subclass makeObservable calls.

[trusktr]

Perhaps if the API were as follows, the construction performance could be improved at a slight cost in DX:

class Foo {
  @observable foo = 123

  constructor() {
    makeObservable(this, Foo)
  }
}

class Bar extends Foo {
  @observable bar = 456

  constructor() {
    super()
    makeObservable(this, Bar)
  }
}

class Baz extends Bar {
  @observable baz = 'hello'

  constructor() {
    super()
    makeObservable(this, Baz)
  }
}

const baz = new Baz

autorun(() => {
  console.log(baz.foo, baz.bar, baz.baz)
})

It would be faster because the decorators could map constructors to keys (f.e. Ctor[symbol].push(propName)), and the
makeObservable calls would not have to traverse the prototype and not have to check own props to see if they exist.

[olee]

@trusktr I think this is an interesting idea and one should not forget performance in the whole matter.
Are there any performance comparisons yet between mobx 4/5/6?

[trusktr]

I haven't made any comparisons, but if we have 1000 instances of the above Baz, then in the MobX 5 version there would have only been 3 decorate calls (O(1)), while in the MobX 6 version there will be 3000 makeObservable calls (O(n), previous optimization idea aside).

Not sure if it is worth it, but perhaps having both is an option: makeObservable(this) is more concise and fine for most cases, while makeObservable(this, Baz) could be an optional way to call it that entirely short-circuits the prototype traversal and the prop checking (still doesn't eliminate the 3000 calls).

[urugator]

then in the MobX 5 version there would have only been 3 decorate calls (O(1))

Even in Mobx4/5 there is a logic similar to makeObservable that runs per instance, it just happens lazily when you access a decorated field for the first time.
https://github.com/mobxjs/mobx/blob/9a4d67f283ccb046851f07349a86d862d036dd79/src/v5/utils/decorators.ts#L43
Moving the call to constructor actually makes it a lot less complicated.
I don't say a one is necessarily faster than other, just saying it's not simply 3 vs 3000 calls.

[mweststrate]

We have benchmark tests for creating 100.000 class instances. The results are as follow:

| version | test | creation | updates |
| --- | --- | --- | --- |
| 6 | typescript + decorators | 7616 | 11319 |
| 6 | typescript + makeObservable | 9029 | 13025 |
| 6 | babel + decorators | 7801 | 11762 |
| 6 | babel + makeObservable | 9266 | 13490 |
| 5 | typescript + decorators | 2525 | 5140 |
| 5 | babel + decorators | 2755 | 5426 |
| 4 | typescript + decorators | 2766 | 7416 |
| 4 | babel + decorators | 2813 | 7470 |

So things have became a bit slower indeed. I can imagine there are some optimisation opportunities here and there still. But the change isn't in a different order of magnitude, and so far object creations or updates being to slow have never been reported as an issue with mobx (IIRC), so I'm not too worried for now; it is quite likely you hit other bottlenecks first when dealing with 100k object, so without real life scenarios in which MobX object interactions prove to be the bottleneck, I'm not worried at this moment.

I suspect however that the primary reason for the slow-down is the babel / TS config change to use [define] instead of [set] to initialise fields, but I didn't actually measure it as no benchmarking tool seems allow playing quickly with babel options. But see this link for why that might matter a lot

[olee]

Ouch - that link you shared really shows how bad this change regarding define semantics for class fields is.
Would it be possible to run a v6 benchmark without the transpilation to define semantics enabled so it can be exluded from the comparison?

[mweststrate]

honestly I would worry too much about such microbenchmark, they give really
little relevant information. First of all, engines might still be catching
up with their optimizations and a few months from now things might look
entirely different. Secondly object creating and field assignments is
probably like 0.01% of the work your app is doing and unlikely to be the
bottleneck of your application, and not making a difference in user
experience if you make it either 3 times slower or faster.

On Thu, Sep 10, 2020 at 11:52 PM Björn Zeutzheim [e-mail]
wrote:

Ouch - that link you shared really shows how bad this change regarding
define semantics for class fields is.
Would it be possible to run a v6 benchmark without the transpilation to
define semantics enabled so it can be exluded from the comparison?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-690772912, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBGY7HYADII4UYU77CLSFFKBZANCNFSM4LZQ2B5Q
.

[bb]

Edit: Fixed in [email protected]

Not sure if this worth a separate issue... I decided to post it here as it's something new introduced with MobX 6:

When upgrading from TypeScript 3.9.7 to TypeScript 4.0.2 I'm receiving an error when providing a second argument to makeAutoObservable. I assume something was made stricter in newer TS version and MobX needs to catch up (but I'm not really sure).

Here's a code sandbox, relevant section highlighted: https://codesandbox.io/s/ecstatic-lake-s93v4?file=/src/App.tsx:351-480

The Code Sandbox is currently fine as it is using TypeScript version 3.9.7 which can be seen in the lower right corner. It seems to me that I cannot change the TS version inside CodeSandbox.

However, when downloading that via Menu File→Export as Zip, doing npm install and then running react-scripts build I get the following error:

TypeScript error in /Users/bb/Downloads/s93v4/src/App.tsx(16,30):
Argument of type '{ exampleShallowMap: Annotation & PropertyDecorator; exampleShallowArray: Annotation & PropertyDecorator; }' is not assignable to parameter of type 'Partial<Record<keyof this, AnnotationMapEntry>>'.  TS2345

    14 |
    15 |   constructor() {
  > 16 |     makeAutoObservable(this, {
       |                              ^
    17 |       exampleShallowMap: observable.shallow,
    18 |       exampleShallowArray: observable.shallow
    19 |     });

The same error is also marked in latest VS Code (August Update, which is using TS 4.0.2) but was not yet marked in the July Update (which is using TypeScript 3.9.7 like the Code Sandbox).

[olee]

Secondly object creating and field assignments is
probably like 0.01% of the work your app is doing and unlikely to be the
bottleneck of your application

I'm not sure about that - especially when having something like a virtual repeat which loads pages of entities, even small delays could cause the view to lag.
However I guess you are right - more often deserialization etc. takes up waay more time than something like class instantiation (though I would like to test this out sometime)

[stephy]

Honestly... I am sad to see the decorators go. I am big fan of their simplicity and clarity. I am happy to see that there will be a work around to be able to continue to use them. However, I am thinking it will be quick to adapt to the _new syntax_

I am guessing this choice was probably a good call. Maybe now without decorators I am hopeful I will be able to convey to people how amazing Mobx is and at least I won't hear the decorators excuse anymore. I have never seen an "@" sign scare so many people.

[olee]

@mweststrate if you have some time, could you give us an update on what is left to do before we can maybe get a mobx v6 beta and when we (roughly) might expect a v6 release?

[bb]

@olee -- I don't know about the release schedule but I'm using RC7 in production already. See also https://www.npmjs.com/package/mobx/v/6.0.0-rc.7?activeTab=versions. Except for my issue with TS 4.x described above, it works fine (with mobx-react-lite v.3 beta https://www.npmjs.com/package/mobx-react-lite/v/3.0.0-beta.1).

However, you should be aware that there's no mobx-utils prerelease yet. See https://github.com/mobxjs/mobx-utils/pull/276.

[mweststrate]

@bb thanks for pointing that out, after upgrading MobX to 4.0.2 the internal tests fail as well

[mweststrate]

New beta available:

[email protected]
[email protected]
[email protected]
[email protected]
Docs: https://deploy-preview-2327--mobx-docs.netlify.app/readme
Migration guide: https://deploy-preview-2327--mobx-docs.netlify.app/faq/migrate-to-6.html

I don't expect any API changes anymore, still polishing docs

[bb]

@mweststrate I confirm the TS 4.0.2 issue is fixed. Thanks a lot; both for fixing that and for releasing the mobx-utils RC.

[roeycohen]

so @mweststrate, decorators are out in v6? It doesn't exists in the new docs anymore, but will using them fail?

[ar7casper]

@roeycohen

so @mweststrate, decorators are out in v6? It doesn't exists in the new docs anymore, but will using them fail?

https://deploy-preview-2327--mobx-docs.netlify.app/best/decorators.html#how-to-enable-decorator-support

[yordis]

@mweststrate is there any concerns using MST with v6?

[episage]

I'd like to note that [email protected] does not seem to be tree shakable. No matter what you import, you end up with the whole bundle in all cases. Is this intentional?

Source: https://bundlephobia.com/[email protected]

[mweststrate]

@yordis there is an open PR that should support MobX 6 fully. But I do not intend to release it personally as there are enough people and companies actually using the thing, benefiting from it but not really contributing back. So I leave releasing and double checking the update to the community, but the PR is here: https://github.com/mobxjs/mobx-state-tree/pull/1569

[mweststrate]

@episage if I run it to terser's tree-shaking, I get results between 13.9 and 15.2 k depending on the imports (yarn test:size in the project). I think things could be better, so if someone wants to play with that feel free to take a stab at it, but I won't be further looking into it.

[yordis]

@mweststrate I am sorry, I am confused with what you said.

But I do not intend to release it personally as there are enough people and companies actually using the thing, benefiting from it but not really contributing back

Are you saying that people are using the package but they don't contribute back, therefore, you will no release a new version using mobx@v6?

So I leave releasing and double checking the update to the community

So we (the community) should take the lead on upgrading MST itself, and verify that works

Is that correct?

[mweststrate]

Exactly, I think all technical issues are addressed at this point. If
someone steps as volunteer to take care of doing the final steps and
releasing, and keep an eye on the project I'll gladly provide the necessary
permissions. If nobody is interesting in investing time, I don't see any
motivation / reason to put my time into it either :).

On Mon, Sep 21, 2020 at 8:40 PM Yordis Prieto [e-mail]
wrote:

@mweststrate https://github.com/mweststrate I am sorry, I am confused
with what you said.

But I do not intend to release it personally as there are enough people
and companies actually using the thing, benefiting from it but not really
contributing back

Are you saying that people are using the package but they don't contribute
back, therefore, you will no release a new version using mobx@v6?

So I leave releasing and double checking the update to the community

So we (the community) should take the lead on upgrading MST itself, and
verify that works

Is that correct?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/2325#issuecomment-696329906, or
unsubscribe
https://github.com/notifications/unsubscribe-auth/AAN4NBDD6O77FQRAFAPWCHDSG6T4FANCNFSM4LZQ2B5Q
.

[israelKusayev]

Hey, I started to try mobx 6, and I have few questions and problems

here is the commit with the new mobx setup
https://github.com/reflexology/client-V2/commit/b283e11dbf66d235fbdc8be0086dd09878145fa1

1. The example of combining multiple stores is a little bit confusing because all the stores are in one file.

   Do I have to pass the instance of rootStore to all other stores? or it is just to share the data between the stores?

   Also I'm getting typescript error about { rootStore: false }
Argument of type '{ rootStore: boolean; }' is not assignable to parameter of type 'AnnotationsMap<this, never>'. Object literal may only specify known properties, and 'rootStore' does not exist in type 'AnnotationsMap<this, never>'.ts(2345)

2. I saw the recommendation to use react context to insert the rootStore into the component tree, and I found this example, I guess if I have rootStore with multiple stores, I should pass the whole rootStore (instead of one store like you did in the example)

   now when I want to use the store in react FC, do I need to do const rootStore = useContext(StoreContext);?

   How can I get one store?

Thanks

[mweststrate]

Closing since released: https://michel.codes/blogs/mobx6

For any follow up questions, open a separate issue.