function-properties in observable objects are not enumerable

[misantronic]

Intended outcome:

function-properties in observable objects are expected to be enumerable

Actual outcome:

function-properties are gone when iterating an observable object

How to reproduce the issue:

I created this example: https://codesandbox.io/s/jovial-black-rq0pw?file=/src/index.ts

The onMore-prop is actually not contained when calling Object.keys(x.notification) since functions are not enumerable in observable objects. I am coming from mobx 4 and this used to work and now it breaks my app in some cases, since I use spreading to pass observable-objects to components like <Component {...notification} />

Versions

Mobx 6

[urugator]

We may change it so that enumerability is respected, but I can't give you a promise atm, it's being discussed/worked on.
Related #2586

[urugator]

For the time being as a workaround try:

const obj = {
      text: "abc",
      onMore: action(() => {})
};

Or

const obj = observable({
      text: "abc",
      onMore: () => {},
}, {
  onMore: false
});

[mweststrate]

Please note that spreading observable objects into components as props is an anti pattern: https://mobx.js.org/react-integration.html#tip-grab-values-from-objects-as-late-as-possible. The idiomatic way to pass the notification down is <Component notification={notification} />, while making sure <Component> is an observer.

If you can't make Component observer because it is not under your control, I recommend pass down properties down explicitly. Especially with actions you probably want to do that anyway to make sure callbacks are invoked by with the right context (<Component title={notification.title} onSetTitle={(title) => notication.onSetTitle(title)} /> rather than <Component {...notification} /> which will break if notification.onSetTitle wasn't bound)

[jrmyio]

It seems very unexpected that properties may suddenly be removed from an object when enumerating, especially since imo mobx' power has always been that it makes you interact with the observables like they are native object/maps/arrays etc (with a little magic when you wrap them into observers).

I can see how it is an anti-pattern but I've been using it mostly in tests and stories to quickly create a set of observable values that I then spread into a react component. When I upgraded to MobX 6 many of these stories broke. There might be tons of more use-cases but I think most important is that its just very DX unfriendly .

[airhorns]

EDIT: This is mostly wrong, and applies just to MST, not plain old observed mobx object, and doesn't have anything to do with the property being enumerable, just is fixed by similar work to fix this issue

Just in case it wasn't clear, upgrading to mobx 6 from mobx 5 and beginning to use proxies causes a regression in mobx-state-tree because of this change, see the above linked issue. Because the properties aren't enumerable, the in operator stops working against MST nodes. Nothin' to do with object spreading.

The in operator makes duck typing possible and really nice. I think it would tend to get used a lot more with mobx-state-tree but applies to plain old makeAutoObservable class instances too.

Before we could pretty easily do something like this:

class Image {
  constructor() {
    makeAutoObservable(this)
  } 

  highlight() { ... }
}

class Paragraph {
  constructor() {
    makeAutoObservable(this)
  } 

  highlight() { ... }
}

class Spacer {
  constructor() {
    makeAutoObservable(this)
  }
}

// duck type on the incoming node type to highlight it if it can be highlighted
const maybeHighlight = (node: Image | Paragraph | Spacer) => {
  if ('highlight' in node) {
    node.highlight()
  }
}

But now, in will always be false, so this doesn't work. You could imagine the same thing in MST with Instance<typeof Paragraph> | Instance<typeof Spacer> etc. The in operator is a really powerful way to decouple code from doing strict instanceof checks (or Type.is checks in MST), and for the kind of application scale state that MST is designed to make possible, I've found myself using it a lot. The inoperator also decouples the implementation of some function from the exact concrete types that support what it needs. Compared to this:

const maybeHighlight = (node: Image | Paragraph | Spacer) => {
  if (node instanceof Image || node instanceof Paragraph) {
    node.highlight()
  }
}

where you have to maintain these lists of instanceof checks throughout a codebase, and go find and update them all when you add a new type, it's much more maintainable code. It'd be really great to get support for in restored which I think necessitates the properties being enumerable.

[airhorns]

Looks like #2641 might change some of this, but if I understand correctly, flows won't be enumerable?

[mweststrate]

Note that class methods are never enumerable by default, unless you turn them into fields: https://www.typescriptlang.org/play?#code/MYGwhgzhAEAa0G8BQ1XQLYFMAuALA9gCYAUAlIkimgL5K1LD4B2E20AHtALzROYDucMpUYt8ITADoQ+AObEkAeQBGAK0zBsk2TkX8mABQBO+AA6Yj2AJ4ARTBGBGAlqez4jxWJNMm318wA00ABEWHhEwaRIUaIQ4lIy8gDkYQSESdBOTBxRSEA. But that isn't needed either for the in operator to work. Could you provide a minimal code sandbox instead to demonstrate the problem?

Also note that !!node.highlight might suffice for your ducktyping?

[airhorns]

Alas, I have lied, you are right @mweststrate! My bad! The in operator works fine for auto-observed plain old mobx objects. I incorrectly assumed in checked enumerability.

Here's a code sandbox explaining the issue from the MST perspective: https://codesandbox.io/s/loving-benz-3czkp?file=/src/index.ts

in returns what I would expect for plain old classes, for makeAutoObservable classes, but not what I'd expect for MST types.model()s.

The in operator doesn't seem to need properties to be enumerable according to MDN, so apologies for the mistake. However, I think when invoking it on an mobx-state-tree node, MST ends up invoking some has check inside mobx proper which returns false. I am guessing that has something to do with the work done to support observable enumerability, but doesn't strictly related to the property being enumerable. Sorry for the noise here!

Also FWIW the !! check would definitely work at runtime, but in acts as a TypeScript type guard too, and !! doesn't from what I can tell, so I think it's still worth trying to get in to work.

[mweststrate]

@airhorns I'd file that in the MST repository if it is still an issue after #2641 landed

[mweststrate]

6.1.0 has landed. Closing this issue now.