Proposal: Improve automatic conversion to observables

[mweststrate]

This is a proposal to kill the automatic conversion of objects / arrays to observables:

const todo = { done: false}
isObservable(todo) // false

const todos = observable([]) // observable array
todos.push(todo)
isObservable(todo) // true

1. Stop automatic conversion of observables

While this, on one hand, is very cool and convenient for beginners, because any changes deep inside the todos collection can automatically be tracked. It also is a source of confusion. Although not often a cause of trouble, the mutation-of-the-righ-hand-side-of-an assignment (or push in the above example) is weird and hard to track if you didn't expect it. Several people already run into that. See for example #644

So after this proposal one would need to do the following:

const todo = observable({ done: false})
isObservable(todo) // true

const todos = observable([])
todos.push(todo)
isObservable(todo) // true

2. Kill modifiers

Currently, MobX provides an opt out mechanism of this behavior, modifiers like asFlat and asReference.

For example modifying the original listing to todos = observable(asFlat([])) would have prevented the behavior. So it can be opted-out.
Since this proposal removes automagic observable conversion, the need to know about these modifiers is gone and they can be removed. (computed, asMap and asStructure are still useful modifiers though). This solves a lot of design questions about the most convenient api around modifiers. See for example #211, #197, #56

3. observable(object) should clone instead of enhance

const todo1 = { done: false }
const todo2 = observable(todo1)
todo1 === todo2 // true

const todos1 = []
const todos2 = observable(todos1)
todos1 === todos2 // false

The above example shows a nice inconsistency between observable objects and arrays (and maps). Objects being converted keep their identity, while arrays produce a fresh object (originally due to limitations in ES5). However to keep things explicit and predictable, I think it is nicer to prevent objects from being enhanced in place, and instead produce fresh objects. In other words, currently observable(object) is an alias for extendObservable(object, object). With this proposal observable(object) would be the same as extendObservable({}, object)

4. Migration path / questions

1. There should be a mobx version that can logs where in the current code base automatic conversion is applied, so that before moving to the next mobx version these cases can be updated
2. @observable decorator is barely effected. Except when assigning objects, arrays. Should those be converted automatically to observables one level deep? E.g. should one need to write @observable todos = observable([]) or should @observable todos = [] suffice (and convert []) still automatically? Or should the whole api be more more explicit: @observable todos = observableArray() ?
3. calling observable on an already observable thing should probably warn

TL; DR

I think this proposal reduces the perceived level of magic introduced by MobX, as the developer is now completely in control when objects become observable and has to be explicit about this. This might reduce the wow-factor for beginners a little bit, but reduce the confusion for intermediate (and beginner) MobX users.

This is definitely a breaking change, so run time guidance on all cases where behavior is changed would be very important I think (unless api method names are also changed, in that case old behavior could live side by side for a while)

[mweststrate]

@andykog @urugator @capaj @Strate @mattruby @donaldpipowitch @kenotron Very interested in your thoughts about the above :)

[matteoantoci]

This is great! I didn't know about this automagic conversion in past and I had to spend a lot of time investigating what was wrong with my code, ending with forcing object back to plain with a toJS method.

[Strate]

What would happen if you do observable on plain object, it became observable only at first level, or deep level too?

[mweststrate]

only first level

[Strate]

Sounds good, but maybe it would be nice to have observableDeep helper (maybe not), I am really don't know :)

[mweststrate]

probably it should make anything observable that is initially in there indeed

[capaj]

@mweststrate

1. and 2. disagree, I think it is more convenient to just use the modifiers asFlat as in the current api, then to force explicitness. I think it is such a common usecase where properties of an object inside an observable array are mutated, that it would be wrong to default into plain non-observable objects.

3.this certainly brings MobX closer to the API we'll have in couple of years with ES6 Proxies. With proxies you won't modify the source object, but you instead generate a proxy object and use that instead of the original. It is a good idea in that regard to leave the source untouched.

calling observable on an already observable thing should probably warn

warn? Why not go a step further and throw an error? This would make sense if it is perf expensive. I don't think it is because it just checks one property right?
If it is indeed perf cheap, I'd probably don't bother throwing/warning. If someone does that a lot, it is not nice, but it won't be a problem.

[mattruby]

+1 for observableDeep. Other than that, I do think it's clearer this way.
Many people don't realize how things are working today.

On Nov 12, 2016 5:55 AM, "Artur Eshenbrener" [email protected]
wrote:

Sounds good, but maybe it would be nice to have observableDeep helper
(maybe not), I am really don't know :)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#649 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAIrciAK-Tr2KIcCz151zB0i2cw193BNks5q9akugaJpZM4KwYbt
.

[capaj]

@mattruby

Many people don't realize how things are working today.

Agree, but should we change the API because people don't read the docs? Maybe we can just make it more visible somehow?

[mattruby]

Or have a shallowObservable option? I could go either way. That magic
would still be available with deep. Just more explicit.

On Nov 12, 2016 7:49 AM, "Jiri Spac" [email protected] wrote:

@mattruby https://github.com/mattruby

Many people don't realize how things are working today.

Agree, but should we change the API because people don't read the docs?
Maybe we can just make it more visible somehow?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#649 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAIrcr08CGvMuvY2MjAwcpHMUSUlwJyxks5q9cP8gaJpZM4KwYbt
.

[andykog]

1. rather agree, that makes api more predictable and understandable
2. There modifiers are there to solve problems, that are consequences of 1 and 3. If we provide deepObservable, asFlat/asStructure can still be useful, but not very important.
3. super agree. It feels wrong, when creating observable “spoils” you original object

should one need to write @observable todos = observable([]) or should @observable todos = [] suffice (and convert []) still automatically?

I think class { @observable todos = val; } should give the same result as class { constructor() { extendObservable(this, { todos: val }); } }, so rather 1st option.

[capaj]

@andykog I like the idea of deepObservable. If I understand correctly if used on an array. it would behave the same as current API? I'd definitely can get on board with that! It would still allow for the same simple workflow if explicitly stated on initialization, while observable would be just flat observable.

[andykog]

@capaj, that's @Strate's proposition, I think it should work just the same way as current observable: recursively apply to all the object props, array items…

[urugator]

Basically repeating ideas from #211

const Mobx = require("mobx");

Mobx.Observable.shallow(obj, modifiers) // no default conversions, same as asFlat
Mobx.Observable.deep(obj, modifiers) // same as current observable
Mobx.Observable.computed(obj, modifiers) 
Mobx.Observable.extend(obj, propDefinitions) // no default conversions, modifies passed obj  

Remove observable - name is too vague, provides no hints about existence of other options, function tries to solve too much, logic is hard to predict and understand. Functionallity is replaced by Mobx.Observable.deep(obj) or Mobx.Observable.from(obj, [Mobx.Modifier.deep()])

Remove asReference - it should be default behavior.

Introduce deep modifier/decorator.

should the whole api be more more explicit: @observable todos = observableArray()

Yes, but move the explicitness to the left side (decorator/modifier) so the stickyness is apparent @Mobx.Decorator.deep todos = [] or @Mobx.Decorator.observable(Mobx.Modifier.deep()) todos = []

I think that better naming alone would solve some issues - these changes are in fact just cosmetics, they don't add or remove features. The thing is that current API doesn't provide any hints about what's going to happen (aside from vague "it makes thing observable") or if there are other options available. As a result, users discover these things by making mistakes, which is not ideal.

EDIT: removed Observable.from and replaced by Observable.shallow (does actually the same)

[mweststrate]

Hmm.. I'm not sure about observable should clone instead of enhance. Although modifying RHS is confusing, storing a clone is confusing as well:

This was confusing:

const todo = { done: false}
isObservable(todo) // false

const todos = observable([]) // observable array
todos.push(todo)
isObservable(todo) // true, confusing

But this is confusing as well!

const todo = { done: false}
const todos = deepObservable([])
todos[0] = todo // silently being cloned and converted to observable object
todos[0] === todo // false, it's cloned!

In that sense, I like @capaj's notion that Proxies allow this to be transparent, and moving in that direction would encourage enhancing.

[andykog]

@mweststrate, I thought deepObservable should only create create nested observable structure, without turning on automatic conversion. If so, your second example will loose its meaning.

[mweststrate]

Ok, two proposals. Not too many comments, as the API should be self-explanatory

Proposal 1

//// Non-decorators

deepObservable(object | array ) 
shallowObservable(object | array)
observableMap(object)
observable(any) // creates a boxed observable

//// Decorators

// shallow observable decorator forbids reassignments (or assignments of non-obsrvables?)
// to avoid needing to convert assigned values (e.g. a fture `obj.x = []`)
@shallowObservable x = [] 

// .. has only deep has magic conversion behavior, works as current `@observable`
@deepObservable = {}

// reference only!
@observable = {}

// as is
@computed

Proposal 2

Based on @urugators proposal. Just not sure if from is fit; as that suggests an uniform target structure (like Rx streams), which isn't the case in MobX?

// Shallow:
Observable.object({})
Observable.array([])
Observable.map({})

// Boxed:
Observable.value(anything) // or .reference / .ref / .boxed / .atom / .cell

// Behaves as current `observable`
Observable.deep(array | object)

//// Decorators
@observable prop = value // reference observable

// encouraged form, no reassignments allowed, mutate array instead
@final todos = Observable.array([]) 

// disencouraged form, both modifying and reassigning `todos` is allowed
@observable todos = Observable.array([])

Note that the second proposal introduces @final. This might look counterintuitive, but might actually avoid confusion between the next two examples by simply forbidding the first:

// will RHS become observable? or will it stay plain array and `user.tags.length` not be observable in the future?
user.tags = ["donald", "duck"]

// unambigous; tags are replaced, values are copied over. tags stay observable. Original array is untouched
user.tags.replace(["donald", "duck"])

// the latter pattern can be encouraged by doing:
class User {
    @final tags = Observable.array()
}

asStructure?

Didn't add asStructure to the above examples.
Are people actually using that? Maybe it should be part of mobx-utils or something. As modifier doesn't add much value imho, except if combined with computed? Do people actually do that?

[capaj]

2.seems like a bigger change.

I kinda like MobX as it is now and I am a conservative developer when it comes to the tools I hold dear. So for me, 1. seems like a better choice. I am not thrilled by how long the new names are: shallowObservable for example is gonna be a little tiresome to write without autocomplete.

[mweststrate]

@andykog yeah I have no clear opinion about that yet, I can imagine people might have libraries / architectures relying on that behavior, so it feels it should remain available somehow? Anyway, I got stuck on that with the following:

const store = deepObservable({ todos: [ { completed: true } ]})
isObservable(store.todos[0], "completed") // true

store.todos.push({completed: false})
isObservable(store.todos[1], "completed")  // ... ?
// what is expected now?
// 1. true, automagic conversion of original
// 2. true, automagic conversion in clone
// 3. false. Confusing, first object observable, second not?
// 4. throw / warn somehow you probably wanted to add an observable

[mweststrate]

@capaj can you check the numbering in your answer ;-) seems off

[capaj]

@mweststrate should be fixed now on github. It is probably wrong in the email notification. I should start previewing before posting 🤕

[danieldunderfelt]

Sounds good! I usually use asFlat for arrays anyway.

[RoyalIcing]

Could observable() take another argument with what to transform added inner elements by?

observable(any, innerTransform = (a) => a)

So to do a deep observable, you could write:

const todos = observable([], observable)
todos.push({ completed: false })
isObservable(todos[0]) // true

[rawrmaan]

I do believe that libraries should always evolve if necessary, but I am against all of these changes, except number 3 (which I don't think matters much anyway).

It's really not hard to avoid these problems if you read the docs. Taking the Observable API from 1 to 4 decorators (Proposal 1) or bringing in a bunch of helper functions like Immutable (Proposal 2) really increase the barrier of entry to this library.

One of the things that really sold me on MobX is the simplicity of "it just works". And it REALLY DOES "just work". And when it doesn't work, I read the docs, and there are sane solutions to my problems.

As for @computed({asStructure: true}), yes I do use it and it has really saved my ass in my latest project.

This library has an amazing and easy to learn API and I'd hate to see that change.

[arackaf]

Thinking about it, having observable arrays make future values observable tracks the behavior of observables objects. For example

class Foo{
    @observable obj = null
}

let f = new Foo();

If you set f.obj to an object literal, then everything therein will be made observable. If, however, you set it to an instance of a class (or really anything with a prototype) then this is not done, since the assumption is that you will have already handled making what needs to be observable, observable (in your class definition, presumably).

Arrays behave the same way. If you add a class instance, it's left alone. If you add an object literal, it's made observable.

I'm not sure breaking this symmetry would accomplish you much.

[basarat]

I agree with @arackaf. I have all my state as an obeservable and have no illusion about what is and isn't observable and if its portions should or shouldn't be observable. I trust in the dependency graph you generate to do its work, and it hasn't let me down once 🌹

[urugator]

Edited my original post (removed from)

To Proposal 2:
Observable.deep(array | object) should be Observable.deep(anything)

Autoconversion(stickyness) on property could work like this:

@deep prop = []; // left side, sticky, autoconvert
@observable prop = Observable.deep([]); // right side, non-sticky, don't autoconvert
// Sticky shallows
@shallow prop = []; // = {}; // = new Map();  

[jefffriesen]

I know this thread was probably created because maintainers keep having to help people through edges cases. Early on @mweststrate helped me through the problem of dynamically added object keys are not observable (thank you, btw). But I use Mobx quite a bit and that's the only edge case I've personally ran into. There is a proposal for dealing with dynamic keys here #652. But adding to the available number of functions has overhead for the user (and the maintainers). I love that @observable just works for most cases. That's the kind of magic I want.

This may be an unpopular idea, but just throwing it out there.

Would counting on having ES6 proxies available allow us to have a more streamline API with fewer edge cases? I know it would help when adding keys to observable objects, but my impression is it could help in other ways. It seems like it could at least allow for amazing DX with better warnings.

This chart has changed quite a bit since I last looked about 6 months ago: https://kangax.github.io/compat-table/es6/. Proxies are available in the latest major versions of desktop browsers, Node 6+, Electron, and iOS10. The latest Android browser isn't supported - not sure where mobile Chrome is on that list. That support covers a huge set of use cases. My point is that the current Mobx release is great and it can still be used in cases that require broader support. But for people who can depend on proxies, what could we do?

From my perspective, the proposals above seem to lose convenience or expand the API with too many options (and unfortunately I don't have any better ideas). API design can make or break a library and I'm concerned about where this could go. I don't know the internals of Mobx so it's hard for me to say how much proxies could help. Thoughts?

[mweststrate]

Note that this proposal has a clear way to construct both plain and deep versions for any specific structure:

const myMap = observable.map({ a: 1 })                     // shallow
const myMap = observable.map({ a: 1}, { deep: true}) // deep

// or optionally from ES6 map, will throw on non-string keys:
const myMap = observable(new window.Map())

Also not that it is no longer possible to give specific fields a specific modifier with .object or .props. If you want one exception, you have to make a second function call, the deep: true option affects all properties. This is inconvenient in rare cases, but keeps the api simpler and uniform.

Although there are still computed and action "modifiers". But action is hardly a modifier; it is not further interpreted by props, it just wraps the original value. And for computed I think 98% of the people will use get syntax instead, and it is mainly there for completeness sake atm. (Would be impossible to create a structural comparing computed attribute in ES6 otherwise, although maybe, computedProp api could be invented as alternative)

[JabX]

@mweststrate Could you still do this?

const obj = observable({
    prop1: {a: 1, b: 2},
    prop2: observable.ref({a: 1, b: 2})
});

I believe this is all we'd need to control the automatic conversion and how we do it now (with asReference)

Edit: About the computed modifier, right now I am doing something like in my code:

function makeStore(config) {
    const store = {};
    for (const ref in config) {
        store[`_${ref}`] = undefined;
        store[ref] = computed(() => {
            if (!store[`_${ref}`]) {
                loadRef(ref).then(action(refList => {
                    store[`_${ref}`] = refList;
                }));
            }
            return store[`_${ref}`];
        });
    }
    return observable(store);
}

Which is a terrific way of lazily loading stuff, and I can't really use the get syntax here.

[mweststrate]

@JabX no in my last proposal you would need:

const obj = observable({
    prop1: {a: 1, b: 2}
});

observable.props(obj, { prop2: { a: 1, b: 2 } })

Hmm is it confusing that observable is deep by default, but props isn't? Maybe props and prop should be deep by default as well? In which case second statement will become:

observable.props(obj, { prop2: { a: 1, b: 2 } }, { deep: false })

If you want observable.ref as modifier in the way it is currently, you will probably need observable.shallow([]) back as well?

[mweststrate]

assuming observable.ref is sticky Op ma 5 dec. 2016 22:10 schreef Damien Frikha <[email protected]>:

…

@mweststrate <https://github.com/mweststrate> Could you still do this? const obj = observable({ prop1: {a: 1, b: 2}, prop2: observable.ref({a: 1, b: 2}) }); I believe this is all we'd need to control the automatic conversion and how we do it now. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#649 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhHy73tvcycs_uCNEvbDLhWJXRVjqks5rFH3PgaJpZM4KwYbt> .

[JabX]

@mweststrate Well, just look at my exemple and yours, and tell me which API is nicer :)

What I like to do is building objects from some kind of definitions, and then at the end wrapping it into an observable. To have control about which part of the final object in observable or not, I use modifiers in place when building, which will be taken into account in the end. It would feel out of place for me to use an external declaration to add a property an object I am building. Not to mention the typing nightmare if the prop2 is mandatory.

Right now, I can do something like that:

const myObject = observable({
    constants: asReference({a: 1, b: 2, c: 3})
    prop1: {
        constants: asReference({a1: 1, b1: 2, c1: 3}),
        prop11: {foo: 4, bar: 9},
        prop12: {baz: 2, bac: 8},
        prop13: {x: 9, y: 1, z: 23}
    },
    prop2: {
        // and so on...
    }
    // and so on...
})

I like this approach and I can't think of a better API to do that.

[mweststrate]

yours 😊 but there are several other questions to answer to justify an api: * type soundness * does it save work * level of self explanatoryness * is it regular with the rest of the api * size increase of the api surface * most importantly: how common is the case? is it common enough that it should have a dedicated function? Op ma 5 dec. 2016 22:27 schreef Damien Frikha <[email protected]>:

…

@mweststrate <https://github.com/mweststrate> Well, just look at my exemple and yours, and tell me which API is nicer :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#649 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhJUWWG7b04fRC4ozg_24w1vlgLMnks5rFIHRgaJpZM4KwYbt> .

[spion]

@mweststrate

If you want observable.ref as modifier in the way it is currently, you will probably need observable.shallow([]) back as well?

In a binary conversion-or-non-conversion world, ref simply stops conversion, and you use ref(array([]))

let data = observable({
  coordinates: {lat: 0, lng: 0},
  dates: observable.ref(observable.array([]))
})

if you want to stop automatic conversions, but still have an observable array. With the caveat that any reassignment of that property needs to be also wrapped in array() if push/pop/etc need to be observed:

data.dates = observable.array(plainArray);

My assumption here is that if you are working with refs you want finer control compared to full-automatic, so you explicitly say "an observable reference to an observable array", and explicitly convert regular array values. Automatic one-level-conversion is left out...

p.s. with this sort of API using a shorter local name for observable might become acceptable 😀

let data = O({
  coordinates: {lat: 0, lng: 0},
  dates: O.ref(O.array([]))
})

data.dates = O.array(plainTimestampsArray.map(t => new Date(t)));

p.p.s. does anyone have a more convincing use case than my array of date objects?

[urugator]

I would like to point out that opts is just another name for modifier/descriptor.
The main problem I have with opts is it's just a plain object, so it can't be used to configure single property in extend/props.
If you consider that opts can contain name and extend function (and possibly other options), then syntax: observable.props(target, props, opts?) is absolutely horrid and nonsensical:

constructor() {
  // So I need a bunch of deeps 
  observable.props(this, {
    prop1: [],
    prop2: {}
  }, { deep: true, name: "This name doesn't make any sense", extend: thisFnJustALittle });
  
  // And a bunch of references
  observable.props(this, {  
    prop3: [],
    prop4: {}
  });
  
  // But one deep also needs a name
  observable.prop(this, "namedDeep", [], { deep: true, name: "I am deep and need a name"});
  
  // And one reference needs an extension  
  observable.prop(this, "fancyObservable", {}, { extend: fancyExtension });
  
  // I don't need anything else, because I have commited suicide at this point    
}

Even if it would be pain to declare each observable prop individually it would be better then this nonsense.

---

Another thing to notice: If I understand right, the extend option allows me to intercept assigned value.
By any chance, isn't deep: true just an interceptor which converts assigned value to observable?
In other words, is it possible to implement extend function which does exactly the same?
opts = { extend: autoconvertToObservable }
If it is, can't every option be implemented as extend function? So instead of setting some flags, can't I pass a series of extend functions modifing the prop behavior? Like:
observable.prop(this, "prop", [], [autoconvert, name("name")])

---

observable.array(initialValue?, opts?) // shallow array, unless opts.deep

Why options? Why do I need to search what the options are? Why do I need to search what the default options are? Why don't you let me simply create ShallowArray and DeepArray?

[JabX]

@mweststrate

but there are several other questions to answer to justify an api:

• type soundness
• does it save work
• level of self explanatoryness
• is it regular with the rest of the api
• size increase of the api surface
• most importantly: how common is the case? is it common enough that it should have a dedicated function?

Well it's there already and it works fine. With the changes we would in fact remove all modifiers except for asReference which would become observable.ref and can be used as a decorator as well.

Type soundness isn't an issue since observable.ref would not change anything, and the only case where it can be problematic if when you want to use a decorator that change the type of your property (like all this @observable.map nonsense).

If you want observable.ref as modifier in the way it is currently, you will probably need observable.shallow([]) back as well?
Well, since collections would be shallow by default and we're adding observable.object, there would be no use for observable.shallow. On that subject, I still expect @observable array = observable.array() to be a deep array, as i exposed before.

[spion]

@urugator whats the use case for a shallow array, as opposed to a @observabe.ref a = observable.arary([]) ? I'm trying to come up with a scenario where you'd really want the automatic conversion for array objects/literals but without recursing into their contents, and I can't think of a good example that shows the need... (Note, not saying there isn't one, just that I can't think of one 😀)

[mweststrate]

@spion yes interesting use case is storing a reference to a HTML domNode, or JQuery promise object (which is still plain)

@urugator @JabX what I try to stress is that convenient and small & simple are each other natural enemies. So I think it is the goal to design something convenient in 80% of the cases, and small and simple to grasp and use in the exceptional cases (but it doesn't have to be convenient per se)

So observable(thing) is there because it is convenient. But observable.prop(target, prop, value) is there because it is a small and simple building block.

I understand that the proposed api is not in all cases convenient. But how bad is that? (In this case we could even support chaining for prop and props. For example in our entire SDK code base (40KLOC) I found only 12 occurrences of modifiers. (8 times a shallow array, 4 times a boxed reference). We use classes a lot, so asFlat is not often required. How are these numbers in your code bases? In the spectacle-editor code base I only found 2 asReferences, in the other first few public projects on the example projects none at all.

---

What still haunts me, but maybe it is a too generic problem to solve with mobx, is this example from @spion:

const o = observable({
    dates: observable.ref(observable.array([]))
})

// or:
@observable.ref dates  = observable.array([])

So the ref here is sticky, but the array being shallow isn't. So after a o.dates = [ 1, 2, 3] observability is suddenly gone. It could be argued that is perfectly valid JS behavior, but maybe people don't see those mistakes quickly, as at first glance there is a working array in o.dates, so the mistake will become only visible when 'things don't react anymore' and then need to be traced back. Shallow modifier nicely prevents these mistakes. (With maps it is less of an issue, as the api completely change, so the first get / set would fail).

Instead of shallow a final would maybe an alternative, forcing people to use replace instead of doing a reassignment...

---

@urugator yep, deep could be implemented by means of extend. But primary concern here is a nice and sweet api ;-)

Why don't you let me simply create ShallowArray and DeepArray?

I think that should be fine as well, especially if the top level api stays small by using observable.shallowArray. But note that there will still be opts for e.g. name.

---

If it is, can't every option be implemented as extend function? So instead of setting some flags, can't I pass a series of extend functions modifing the prop behavior? Like:

That goes a bit back to the late first proposal, where these functions like name and autoconvert could be in the modifiers namespace, and the syntax for modifiers in .props (and .object ?!) could be something like:

const myObject = observable({
    constants: with({a: 1, b: 2, c: 3}, modifiers.ref, modifiers.name("floepie"), myCoolLogger)
    dates: with([ ], modifiers.shallow)
})

Not sure what a nice syntax / name is here though. extend or prop maybe? Or decorate(fn, fn, value) Or like real decorators (correct use of parens will scare away people I guess):

constants: modifiers.ref(modifies.name("floepie")(myCoolLogger({ a: 1, b: 2})))

But then again: some options and a lot of repetition in .props feels simpler?

---

TL;DR:

1. revive shallow to prevent users from making stupid mistakes? (mainly with shallow arrays)
2. is options as seperate params that ugly that we need a seperate RHS modifiers mechanism>

[urugator]

@spion Just to make it clear, I haven't said anywhere that sticky shallows must be supported. Actually I said that sticky shallows should be either removed together with sticky map (deep or not) or the autoconversion must be type safe, otherwise it's not possible to ensure consistency I believe.

To your question: Autoconversion(stickyness) is just a conveniency. Anytime you want a setter generated automatically for you, then you want autoconversion. It's just an ability of the API to accept unobservable objects. So anytime you want a publicly settable shallow prop, you probably want autoconversion. It's exactly the same situation as with deep.
So the right question is simply when you need shallow?
The scenarios I can think of:

• When you want to add complexity gradually (which is quite naturall I think)... until you don't need the content of the array to be observable, you don't make it observable (notice that making things observable is not without side effects like Array.isArray not working, cloning, etc... you don't want to deal with these issues, until it's neccessary)
• When the content of array is generic (you don't know it's type). You want to treat such content always as reference.
• When the items in array are plain structures (object without prototype/array/map), but they're not considered as collections. Imagine you're prototyping and you don't have a User class yet, so you represent the user as a plain object, you always need to store actual reference to user (observable or not).
• Optimization

[urugator]

@mweststrate

But primary concern here is a nice and sweet api ;-)

Primary concern of each program should be to provide a solution to given problem.
The things should be removed, because they're not needed, not because they make API more complicated.
At some point the problem can simply became too complex to be expressible by the existing means. In that case I believe it's better to introduce new concepts, with aligned and consistent behavior, than introducing ambiguity and exceptions.
Unfortunately we don't even know what problems we want to solve...so we're going back and forth... making API more complex to support features and then removing features to make API simpler...

revive shallow to prevent users from making stupid mistakes? (mainly with shallow arrays)

There should definitely be a standalone shallow.
In case of stickyness, the decision needs to be made: support only for deep array|object (autos) OR support all, but type can't be changed
I can live with both, but I lean towards the type-safe variant as I can't imagine a situation where I would need to change a type of property from lets say shallow array to shallow object/map... (and in that case own setter would be more appropriate than some weird background magic....)

Is options as seperate params that ugly that we need a seperate RHS modifiers mechanism:

Either don't allow defining multiple observable props at the same time, or yes it should be handled differently. (Do you apply single decorator to multiple props at once?, no because it doesn't make sense...)
Just a reminder, options ARE modifier mechanism, the modifier is just represented by a plain object called options.
You should align decorators with modifiers (technically they have the same purpose - to add a meta information to a property), instead of trying to align decorators with functions, which creates standalone observables (they, on the other hand, have totally different nature and purpose, that leads to wierd situations like "ouch, tricky overload with the non-decorator...?").

Btw observable.value is always described as boxed, so wouldn't observable.boxed be more self-explanatory?

[mweststrate]

Ok, probably we can't escape the modifiers RHS, they are too convenient. So what we get then is:

* observable(initialValue)                   // picks best overload, with deep = true
* extendObservable(target, props)            // as is, deep by default

* observable.box(initialValue?, name?)
* observable.shallowBox(initialValue?, name?)
* observable.array(initialValue?, name?)
* observable.shallowArray(initialValue?, name?)
* observable.map(initialValues?, name?)
* observable.shallowMap(initialValues?, name?)
* observable.object(initialValues, name?)
* observable.shallowObject(initialValues, name?)

// modifiers, to use with `observable(object)` / `extendObservable(object)`
* observable.ref                                 // sticky modifier, like asReference
* observable.shallow                         // sticky modifier, like asFlat, throwns on non convertable structure

* @observable                                 // deep, as is
* @observable.ref                            // doesn't convert assigned values
* @observable.shallow                    // converts it's values to shallow collections

* computed(getter, setter?, computedOpts?)   // largely as is
* @computed
* @computed.struct                           

For comparision what changed to current (MobX2) behavior:

• Can now easily create specific observable collections, shallow and non-shallow (default)
• Fixed issues with stickyness and inability to name certain collections
• observable(asFlat([])) is not allowed anymore, should use observable.shallowArray() instead
• @observable todos = asFlat([]) is not allowed anymore, should use @observable.shallow todos =[]
• @computed(asStructure) is not allowed anymore, should use @computed.struct
• (auto) plain object to observable object conversion always clones. To enhance extendObservable can still be used
• passing an argumentless function to observable no longer converts it automatically to a computed value
• internally shallow (and deep conversion) will be expressed in terms of intercept?

[JabX]

@mweststrate

What still haunts me, but maybe it is a too generic problem to solve with mobx, is this example from @spion:

const o = observable({
   dates: observable.ref(observable.array([]))
})

// or:
@observable.ref dates  = observable.array([])

So the ref here is sticky, but the array being shallow isn't. So after a o.dates = [ 1, 2, 3] observability is suddenly gone.

Maybe this is my fault for not understanding how MobX works, but I've always assumed that it was the actual behaviour, since it looked too magic to me to autoconvert my array into an observable array with a direct affectation like that.

(@)observable.ref should definitely not autoconvert, that much seems pretty obvious to me. The observable reference and the array being shallow observable are two distinct things. If anything, @observable merging the two concepts is a bigger problem since we are effectively doing two things at once with the default operator. But it's convenient and it's there already, so as long as it's made clear it can (and should) stay that way (that why I wanted to swap deep and ref originally).

About your proposal, why would we need both shallow observables and the shallow modifier?
Wouldn't those be the same ?

@observable arr = observable.shallowArray()
// and
@observable.shallow arr = observable.array()
// and
@observable.shallow arr = []

And if I wanted a ref to a shallow array, I would do

@observable.ref = observable.shallow([])

I don't think we even need either version of observable.array, observable.boxed and observable.object with a shallow modifier/decorator, and a single Map construct would suffice, which can be or not the ES6 one.

[mweststrate]

Shallow observables are primarily to create shallow observables in ES5, because const myArray = shallow([]) would create just a property descriptor (which is needed to make it sticky when used in side observable, not an actual array). const myArray = observable(observable.shallow([])) could work here, but feels ugly somehow. Using modifiers just to describe properties is a bit cleaner imho.

[JabX]

Yeah I realized that moments after posting. That means we would still need data structures, but only shallow ones then? observable([]) but being deep and observable.array() being shallow? I don't think we need multiple ways of declaring the same things, it would only clutter the API I believe.

[urugator]

// Autos
deep(initialValue?, name?) // replaces observable(val) (everything is observable, having observable function is misleading, mentioning "observable" is redundant, stating deep instead of observable better indicates the existence of shallow)
shallow(intitialValue, name?) // replaces observable(asFlat(val))
// Standalones  
shallowBox(initialValue?, name?) // explicitely states shallow, so there aro no doubts about behavior/defaults and to indicate the existence of deep
shallowArray(initialValue?, name?)
shallowMap(initialValues?, name?)
shallowObject(initialValues, name?)
deepBox(initialValue?, name?) // explicitely states deep, so there aro no doubts about bahivor/defaults and to indicate the existence of shallow
deepArray(initialValue?, name?)
deepMap(initialValues?, name?)
deepObject(initialValues?, name?)
// <- you can place these into mobx.observables namespace if you think it's necessary

// Decorators + Modifiers
@prop // reference
@prop.deep // @observable - autconverts array|object
@prop.shallow // @observable asFlat() - autoconverts array|object  
@prop.[type] // sticky, autoconverts to given type: shallow|deep|Box|Array|Map|Object, type safe
// Every decorator accepts name param
@prop.shallowMap("name")

// Without decorators
defineProperties(this, {
  prop1: [], // reference
  prop2: prop([], "name") // named reference
  prop3: prop.deep("name") // named @observable
  prop4: prop.shallow,
  prop5: prop.[type]
});

EDIT: If there are overloading problems between modifiers and decorators, either force user to always call the decorator/modifiers as function (without name parameter, which makes sense afterall) or move them to separate namespaces (you will never use both at the same time...you either use decorators or not - I would go for mobx.decorators and mobx.descriptors). I personally propose to do both as you will avoid possible problems in the future.
EDIT: Ehm is name neccesary in props?

[rawrmaan]

@mweststrate Just want to give you props on listening to everyone's ideas and coming to a nice conclusion. This was a really long discussion but I think the proposed API makes things even clearer than they are now while not sacrificing any of the "it just works" magic that makes MobX great.

[mweststrate]

@rawrmaan thanks! Much appreciated. I hope to get an alpha out at the end of this week, largely containing the api as described in this comment, to validate the design :)

[alexhisen]

If the auto conversion always clones, why not also clone own properties of objects with prototypes as well, not just plain objects? It's one less gotcha you have to be aware of.

[mweststrate]

@alexhisen because prototyped are considered closed boxes, and can take care of own observability. If they still would be cloned; it means that you always get a copy as soon as you have multiple references to it.

[mweststrate]

Closing this issue as [email protected] is now available, which addresses this issue. Changelog: https://github.com/mobxjs/mobx/blob/mobx3/CHANGELOG.md#300. The changes in more details: https://github.com/mobxjs/mobx/pull/725/files. Feel free to re-open if questions arise!

[jamiewinder]

@mweststrate

How does @observable.shallow work with ObservableMaps? For example, to define a shallow array property:

@observable.shallow public array = [];

How do I do the same with maps? At the moment I'm doing:

@observable.ref public map = observable.shallowMap();

I could probably just do @observable here to the exact same thing, but it feels like I should somehow be using @observable.shallow? Or is this just meant for objects and arrays?

[urugator]

@jamiewinder According to changelog:

It is now possible to pass ES6 Maps to observable / observable maps. The map will be converted to an observable map (if keys are string like)

so I quess the following should work (haven't tested)

@observable.shallow public map = new Map();

Without ES6 Map, the sticky shallow map initialized from plain object probably isn't supported, so you would have to do the conversion manually in setter.

[mweststrate]

@urugator correct. @jamiewinder if you never replace the map instance (which I consider a good practice), /* readonly */ public map = observable.shallowMap() is the most explicit I think (readonly works in typescript). Note that observable maps now expose .replace(newData) method just like observable arrays

[barberousse]

Any chance of seeing BurntCaramel's recommendation implemented?