createViewModel doesn't respect { deep: false }

[sheerun]

It happens with mobx-utils 5.1.0:

const { observable } = require('mobx')
const { createViewModel } = require('mobx-utils')
const model = observable.object({ unit: null }, {}, { deep: false })
model.unit = { foo: [1,2,3] }
Array.isArray(model.unit.foo)
// => true
model.unit
// => { foo: [ 1, 2, 3 ] }
Array.isArray(view.unit.foo)
// => true
view.unit = { foo: [1,2,3] }
const view = createViewModel(model)
Array.isArray(view.unit.foo)
// => false
view.unit
// => { foo: [Getter/Setter] }
view.unit.foo
// => ObservableArray [Array] {}

It happens that I pass model to library that tries to do isArray check on some internal array of unit which mobx converted to ObservableArray. I think createViewModel could preserve shallowness to prevent such errors.

[sheerun]

Even worse: after doing view.submit() the original model.unit.foo becomes observable which should never be

[sheerun]

@mweststrate I there any way to know that provided mobx observable has been created shallow?

[ItamarShDev]

isn't the note from the docs says it would? @sheerun

You may use observable arrays, maps and objects with createViewModel but keep in mind to assign fresh instances of those to the viewmodel's properties, otherwise you would end up modifying the properties of the original model. Note that if you read a non-dirty property, viewmodel only proxies the read to the model. You therefore need to assign a fresh instance not only the first time you make the assignment but also after calling reset() or submit().

[sheerun]

It would be true for deep observed objects, but shoudn't be the case for { deep: false } observables

[ItamarShDev]

Looking at the code, it seems that this should be an observable by design.
Not sure its a bug...

@mweststrate

[sheerun]

It should be an observable, but in my case it should be shallow observable, not deep one.

[mweststrate]

I think this is a bug indeed, and that the modifiers should be preserved. @sheerun would you mind creating a PR with some unit tests demonstrating the erroneous behavior and verifying a potential fix?

[sheerun]

I can but first I need an answer to my previous question because I couldn't find how, and without this it's not possible to implement any fix:

Is there any way to know that provided mobx observable has been created shallow?

[urugator]

Hacky inefficient solution:

function isShallow(thing) {
  if (isObservableObject(thing)) {
    return !isObservable(thing.$mobx.defaultEnhancer([]));
  }
  if (isObservableArray(thing)) {
    return !isObservable(thing.$mobx.enhancer([]));
  }
  if (isObservableMap(thing)) {
    return !isObservable(thing.enhancer([]));
  } 
  return false;  //?
}

[
  // deep
  observable({}),
  observable([]),
  observable(new Map()),
  // shallow
  observable({}, {}, { deep: false }),
  observable([], { deep: false }),
  observable(new Map(), { deep: false })
].forEach(thing => console.log(isShallow(thing)));

[mweststrate]

I think this PR requires some additional reflection methods in MobX first, a proposal is welcome (on that repo)

[mweststrate]

Closing for inactivity