What about Object.keys / why are actions enumerable:false?

[JMS-1]

Consider the following code snippet:

const someObject = { data: 'data', func: () => {} }

console.log(Object.keys(someObject))

const someObservable = observable(someObject)

console.log(Object.keys(someObservable))

The POJO reports ["data", "func"] and the observable only ["data"]. In most situations this makes no problem especially since most observables will be used to carry data and not functions.

But we have some use case where the observable is something that will be passed as spread to a React component (<Comp {...props} />) [in this case through semantic-ui-react but I'm currently not sure if this matters]. Somewhere in there is some Object.assign({}, defaultProps, props) which in the case of the POJO will consider func (to be specific: onClick) but the observable will just get data from the props. Loosing onClick isn't really funny...

If the situation was as simple as the repro we could just use annotations on the observable to force func not to become an action but an observable itself.

const someOtherObservable = observable(someObject, { func: { annotationType_: 'observable' } })

console.log(Object.keys(someOtherObservable))

But live isn't that easy: the func (onClick) is on a deeper level in the observable making it quite hard to explicitly disable auto-generation of an action. And we have absolulty no influence on the library getting the fields from the object.

For now the first short question: is there a way to tell the observable proxy generator just to keep ALL fields in EVERY nesting level as simple observables - and not just guessing functions will be actions? Or is the current behavior in principal (say better than 90:10) "right" and we have to check for some other way to come around the problem?

Thanks

Jochen

[danielkcz]

But we have some use case where the observable is something that will be passed as spread to a React component (<Comp {...props} />)

That sounds like a really bad idea. If you are not careful, you might lose reactivity for any primitive values. I would really recommend passing a whole object and destructure in the component.

So generally I think your use case is built around wrong assumptions on how to use observables. Besides, we can't really afford to change this behavior as it might break a whole number of codebases relying on the opposite.

You can also try toJS before you use Object.keys, but I can't promise it will work.

[JMS-1]

Well although personally I don't like it either it has the charm to save some typings in the component call. If it's a bad idea considering mobx: may be. But for most React library builders it seems to be "common ground". It's the Object.assign() thing that doesn't play well with mobx - or vice versa depending on which side you stand ;-) [Considering this: maybe manually adding the functional properties after the spread could work - i.e. <Comp {...props} func={props.func} />, will check this later.]EDIT: NO IT DOES NOT

As I said Object.key ist not what WE do - this was the easiest way I could reproduce the behaviour for discussion, the library is in fact using Object.assign() which lands somewhere in mobx's ownKeys (?) - I had to debug to check again.

In fact we have one wrong assumption: someObject behaves in all :-) (ok, best as possible all) aspects like observable(someObject). But it doesn't! If you call observable yourself you can tweak this using annotations (so in a way: someone thought about it before) but this won't be usefull on nested fields.

If I could write it on my wishlist for christmas: add on option to allow disable auto-detection of functions when creating an observable and just make everything an observable field.

[danielkcz]

Actually, I think you can do that "wish" of yours with MobX 6, it has a new overrides option for observable.

[JMS-1]

Ok, this IS mobx 6, I'll check...

[JMS-1]

Ok, this is what I meant with annotations (see sample), does work in repro but no applicable to production code.

[danielkcz]

Yea, I will leave this to others, this is beyond my knowledge, sorry.

[urugator]

In case of plain objects, I think it would be reasonable to preserve enumerability and doing both - making field observable and wrapping the value (function) in action. I dunno if we can do a change like that, but I think it's worth discussing.
Would it solve your problem?

[JMS-1]

I think so - must check if Object.assign() behaves. In my debug session I found it ended up in some mobx method (ownKeys?). But the change sounds risky.

EDIT:

I did some debugging - don't know if this can help in the discussion. Finally the function "property" func is marked with enumerable: false which will exclude it from Object.assign (see https://www.ecma-international.org/ecma-262/#sec-object.assign).

var tmpNameDescriptor = {
  value: "action",
  configurable: true,
  writable: false,
  enumerable: false
};

EDIT2:

Sorry, that's exactly what you said. Anyhow: yes, I'm now quite sure that making the action enumerable will solve this very problem.