[mobx6] observer decorator not working when the class property is not assigned

[xaviergonz]

When using @observable over an unassigned property it won't be actually made observable It needs to be assigned to undefined for it to work

import { observable, makeObservable, isObservableProp } from "mobx";

class C {
  @observable
  x?: number;
  // x?: number = undefined; // this is ok

  constructor() {
    makeObservable(this);
  }
}

const c = new C();
console.log(isObservableProp(c, "x")); // returns false

CodeSandbox: https://codesandbox.io/s/intelligent-feistel-4kmru?file=/src/index.ts

Also it is not possible to use makeObservable over an undefined property unless it is actually assigned undefined

import { observable, makeObservable, isObservableProp } from "mobx";

class C {
  x?: number;
  // x?: number = undefined; // this is ok

  constructor() {
    makeObservable(this, {x: observable}); // error: cannot decorate undefined prop x
  }
}

const c = new C();

[mweststrate]

This is working as intended if you didn't configure babel / typescript to use the standardized classfields. See https://mobx.js.org/migrating-from-4-or-5.html#getting-started, bullet 4 & 5

For some background to why it behaves this way, read: https://github.com/mobxjs/mobx/issues/2458

[xaviergonz]

I see... would you accept a PR that made these cases also work when "useDefineForClassFields" is set to false?

The point is that (unless that is fixed) if mobx6 requires that to be true then it means that any library that uses experimental decorators then most probably won't be usable at all with mobx6 + decorator syntax.

[mweststrate]

Yeah that would be ok I think, but only make it work then for the decorator code path. calling makeObservable with an annotations map should keep failing to give early signal on mistakes in the property names. (without define prop semantics, not-existing- and not-yet-assigned properties are indistinguishable)

Op wo 30 sep. 2020 21:23 schreef Javier Gonzalez notifications@github.com:

I see... would you accept a PR that made these cases also work when "useDefineForClassFields" is set to false?

The point is that (unless that is fixed) if mobx6 requires that to be true then it means that any library that uses experimental decorators then most probably won't be usable at all with mobx6 + decorator syntax.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx/issues/2468#issuecomment-701624131, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBGIJD6KNXWEFPA54LLSIOHS7ANCNFSM4R7NGC5A .

[mweststrate]

Curious what is the problem with useDefine in your case? Another library relies on the old behavior? Note that it will break eventually 😉

Op wo 30 sep. 2020 21:41 schreef Michel Weststrate mweststrate@gmail.com:

Yeah that would be ok I think, but only make it work then for the decorator code path. calling makeObservable with an annotations map should keep failing to give early signal on mistakes in the property names. (without define prop semantics, not-existing- and not-yet-assigned properties are indistinguishable)

Op wo 30 sep. 2020 21:23 schreef Javier Gonzalez <notifications@github.com

:

I see... would you accept a PR that made these cases also work when "useDefineForClassFields" is set to false?

The point is that (unless that is fixed) if mobx6 requires that to be true then it means that any library that uses experimental decorators then most probably won't be usable at all with mobx6 + decorator syntax.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mobxjs/mobx/issues/2468#issuecomment-701624131, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBGIJD6KNXWEFPA54LLSIOHS7ANCNFSM4R7NGC5A .

[xaviergonz]

Thanks :)

For example I'd like to release a version of mobx-keystone that works with mobx 4-5-6 when useDefineForClassFields set to false (even though it would still break with it set to true) and later, eventually, probably offer an API closer to mobx-6.

The idea is to give a smoother transition phase rather than an all-in or nothing approach.

[mweststrate]

Closing issue as things are working as intended.