Mobx: Road to 4.0

[mweststrate]

Hi all!

Time to start thinking about the next major of MobX. These are some of the ideas I have around them. Global goal:

_Leverage modern features in evergreen browsers to address current quirks in the MobX api_

Quirks being:

1. Observable arrays not being arrays,
2. property additions not being observed
3. Maps only supporting primitive keys and Sets not being present #800

Tools to address the quirks: levering proxies #776 and native map implementations #800

Breaking changes

• observable(object) will now make all properties (including future ones) observable. To limit / specify the set of observable properties one will have to use extendObservable
• Some utilities like isArrayLike will be deprecated
• Array.move will be deprecated
• Improve some typings #818, #817

Also fix #940 if not done before

Tasks:

• [ ] Proxy arrays, kill faux array
• [ ] Proxy maps
• [ ] Proxy objects
• [ ] Introduce sets
• [ ] Update / finetune observable api
• [ ] Verify performance
• [ ] Document migration path
• [ ] Re-evaluate existing api, remove deprecated stuff, deprecate things to be removed
• [ ] Update docs, tutorials, gotcha pages
• [ ] Double check if all issues referred to in this issue are adressed
• [ ] Clearly communicate environment dependencies of 4.0, suggest 3.0 branch for projects with strict browser requirements
• [ ] Clean up the legacy exports (for default and *)
• [ ] Decompose extras namespace for better tree shaking
• [ ] Remove spy / or spy events related to object mutations [discuss]

[stevefan1999-personal]

long live the MobX

hopefully it could defeat Redux in one day

[andykog]

bq. Proxy arrays, kill faux array
@mweststrate, do you really think of dropping IE support? :-( My granny wont be able to visit my website :-(

[mweststrate]

@andykog I think 3.0 should be maintained still, but want to prepare for the future as well. Promised for a long time now that certain issues will be fixed when proxies are available, and their support is getting more decent now :)

[marvinhagemeister]

@mweststrate Although we do have some clients who use IE11 explicitly, I do very much support going all in with Proxies for mobx. I'm really excited about them and think they will make the core a lot simpler.

[andykog]

Not everybody is so lucky that they can ignore IE users. You can't tell managers that we have to drop about 4% of customers because that will make mobx core simpler. This puts many user in a position where they have to decide wheather they gonna stick to an older mobx version or use something different for their apps. My opinion is that this is to early for switching to proxies.

[Keats]

Compatibilty of:

• proxy: http://caniuse.com/#search=proxy
• Map: https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Map

I don't think I would be able to switch immediately to v4 but I would support using proxies and Map anyway, it's not like 3.0 is going to stop working.

[mattruby]

About 5% of our traffic is still stuck on IE. Regardless, I think it's a
really cool idea to start exploring the new proxy features. I think we
should be super duper explicit about what v4 is all about. We may want to
call it mobx-next or something. I do worry that if we wanted to make
significant changes to 3.x we'd have to jump past v4. That could be a
little funky.

On Thu, Jul 6, 2017 at 2:56 AM, Vincent Prouillet [e-mail]
wrote:

Compatibilty of:

• proxy: http://caniuse.com/#search=proxy
• Map: https://developer.mozilla.org/en/docs/Web/JavaScript/
  Reference/Global_Objects/Map

I don't think I would be able to switch immediately to v4 but I would
support using proxies and Map anyway, it's not like 3.0 is going to stop
working.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/1076#issuecomment-313324108, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAIrch_yNRH0QHVYvPv7SFJWuSX2Mfz_ks5sLJMdgaJpZM4ONAsS
.

--
-Matt Ruby-
[email protected]

[andykog]

+1 for mobx-next, thought about that as well

[jamiewinder]

I could be wrong, but I think

1. Maps only supporting primitive keys and Sets not being present

can be implemented now without any loss of browser support (with a polyfilled ES6 Map)? If so, it might be worth this - and any other breaking additions which needn't affect browser support - being the basis of v4, and v5 (or mobx-next) forming the basis for the big move to using Proxy?

[stevefan1999-personal]

https://github.com/ascoders/dynamic-object

here's an object observer with proxy that features similar functionality like mobx, using almost purely Proxy. Indeed the project is written by a Chinese, hence I doubt your ability to read the documents and comments, but you guys can take it as a code reference.

[Keats]

I would be against mobx-next that sounds like it is a completely new API etc. I don't really see the issue with using v4 for breaking changes, there are polyfills for Map and maybe for proxy (https://github.com/GoogleChrome/proxy-polyfill). v3 will still work and would likely be in maintenance mode

[urugator]

Some quick notes:

• Change expr to accept options object (like computed) instead of scope
  (breaking)
• Unify naming for "this" option, in computed/reaction it's called context, in autorun/action/when/autorunAsync/spy it's called scope
  (non-breaking)
• Provide dispose(reaction object) as a second argument to reaction's sideEffect callback
  (non-breaking)

[mweststrate]

@urugator tnx!

Also: Upgrade to latest TS

[Strate]

Can we avoid "dispose" somehow, for example storing listeners to WeakMap with observable as a key?

pseudocode:

const map = new WeakMap()
observe(obj, listener) {
  map.set(obj, listener)
}

[acronoah]

Managing reactions/autorun disposal is my one difficulty with Mobx. Especially when the autorun is on something other than a component (and thus without an unmount/dispose...)

[iamdanthedev]

Having struggled recently with async computed functionality, I woukd propose it out of the box.

[andrew-luminal]

@rasdaniil +1

Better, clearer support for async, both for computed and for action, would be high on my list. I don't know how feasible it is to improve, but runInAction feels very kludgy to have to use after every await in an async action.

[mweststrate]

Check the recently added asyncAction in mobx-utils package! Removes the
clumsiness

Op do 27 jul. 2017 17:21 schreef Andrew Metcalf [e-mail]:

@rasdaniil https://github.com/rasdaniil +1

Better, clearer support for async, both for computed and for action, would
be high on my list. I don't know how feasible it is to improve, but
runInAction feels very kludgy to have to use after every await in an
async action.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/1076#issuecomment-318394878, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhFo-5r6a8qx8B2X2gRd4TUazpR5Nks5sSKsAgaJpZM4ONAsS
.

[tkrotoff]

@andykog

You can't tell managers that we have to drop about 4% of customers

MobX releases:

• v1: Oct 13, 2015
• v2: Feb 26, 2016 => 5 months
• v3: Jan 9, 2017 => 11 months
• v4 => ?

By the time MobX v4 is released, IE usage will be lower.
+ a few more months because you prefer a bit of dust on new things
+ the time it takes to switch to v4 (upgrade code, test, document, release)
=> IE will be even lower

Voilà

[aliakakis]

@stevefan1999 Redux is OK but still, I do get quite intrigued when I listen as an argument that Redux is for large apps and MobX for small/medium. Not to mention that after a while the amount of files and complexity a Redux based project has to deal with is absurd.

@mweststrate 👍 for killing faux arrays

[OshotOkill]

Good time to looking for the future. I approve the plan of switching to proxies underneath. Besides, React 16 requires Map & Set in runtime environment and it's mandatory.

[jamiewinder]

@OshotOkill - But Map and Set can be polyfilled. Proxy cannot.

[phiresky]

@jamiewinder But wouldn't the existing partial polyfill for Proxy cause a rewritten version of mobx to behave just like the current version does (i.e. new properties are not tracked)?

Then mobx4 could just have a compatibility flag that disallows adding new properties to objects and browser compatibility would stay the same.

[jamiewinder]

@phiresky I'm not really sure what the extent of the limitations of that polyfill are apart from the ones you highlighted. One that does start out is the faux array issue of MobX - observable arrays are not arrays. If MobX were to rely on a proxy polyfill, then we'd be in an ever more precarious position where an observable array may or may not 'be' an array depending on whether you're using the polyfill.

I think the switch to proxies should be absolute. i.e. if you need to support non-proxy browsers, then I'm afraid you'll have to stick to the previous version. However, I'm very likely one of those people who'll have to do this, and I'm missing some of the other features that are hoping to be added such as non-string keyed observable maps which can be added but will be a breaking change.

It'd be nice (though I wouldn't presume to ask for it) to see a MobX 4 that adds these features (and require polyfilled Map, Set), and a MobX 5 that forces you to use Proxy, but this might lead to confusion and having to maintain two active versions. So I'm not sure.

[mweststrate]

Also interesting; when will the new babel decorator implementation be
ready? Should 4.0 be based on the old or new implementation (and
semantics).

Op ma 4 sep. 2017 om 10:44 schreef jamiewinder [e-mail]:

@phiresky https://github.com/phiresky I'm not really sure what the
limitations of that polyfill apart from the ones you highlighted. One that
does start out is the faux array issue of MobX - observable arrays are not
arrays. If MobX were to rely on a proxy polyfill, then we'd be in an ever
more precarious position where an observable array may or may not 'be'
an array depending on whether you're using the polyfill.

I think the switch to proxies should be absolute. i.e. if you need to
support non-proxy browsers, then I'm afraid you'll have to stick to the
previous version. However, I'm very likely one of those people who'll have
to do this, and I'm missing some of the other features that are hoping to
be added such as non-string keyed observable maps which can be added but
will be a breaking change.

It'd be nice (though I wouldn't presume to ask for it) to see a MobX 4
that adds these features (and polyfill Map, Set), and a MobX 5 that
forces you to use Proxy, but this might lead to confusion and having to
maintain two active versions. So I'm not sure.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/1076#issuecomment-326903242, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhLyuvLuVPmNxWFW0_n_1YCJv5Bg-ks5se7h7gaJpZM4ONAsS
.

[mweststrate]

For implementation details see https://github.com/ascoders/dob, #776

[solkimicreb]

Just a quick note, because I see people mentioning only IE here. React Native is still not supporting Proxies, which is more important then IE in my opinion (other platforms are fine). Does anyone know when they plan to start supporting them?

[farwayer]

@solkimicreb As I know Proxy supported in iOS starting from 10.0. I didn't tested yet but it should be available on iOS because React Native uses system JSCore. On Android Proxies is not supported yet.

[urugator]

Make use of valueOf in comparer.structural see #1118

[solkimicreb]

About Proxies on React Native. I am pretty sure they are still not supported: https://facebook.github.io/react-native/docs/javascript-environment.html

[caesarsol]

@mweststrate Hi, what about using Proxy only in development, to check for uncorrectly used keys or unexistant ones?

That way would improve developer experience and detecting common errors, but without dropping support for old browsers!

[caesarsol]

About Maps: I actually liked how users are forced to use Map with dynamic keys, that sounds lile a good practice in general JS!

[ascoders]

I tried proxy-polyfill, but there are some unstable problems in the ie environment, and can not support deleteProperty, resulting in missing functionality.

So if 4.0 is no longer compatible ie it may be a reliable decision.

@mweststrate thanks for mentioning the library I wrote, If you can support the decorator, it may be easier to understand and more convenient to write:

@observable
class MyStore {
  name = 'ascoders' // Automatic observable
  articles = [] // Automatic observable
}

[farwayer]

@solkimicreb I tested Proxy in React Native on iOS and it works. Big disadvantage Proxy is not supported in React Native on Android and there is no plans to support it in the near future.

UPDATED: I found info JSC may be upgraded for Android in React Native in few months. But there is no real ETA though. In any case we can recommend to use custom JSC in readme if this does not happen before the 4.0 release.

Hi, what about using Proxy only in development,

@caesarsol IMO it is bad idea because something can work in dev but don't work in prod and vice versa. I prefer keep dev and prod environments as close as possible to prevent some strange errors. This is very actual for JS with tons of polyfils and translators.

[Strate]

The idea to use Proxy for dev environments to detect access to non-existing keys of objects and warn developer about it looks good for me! It will conver mos frequent developer error with mobx.

[caesarsol]

@farwayer that would be only for warning the developer, not to add functionality! So the behavior wouldn't be forked :)

@Strate thanks!

[solkimicreb]

@farwayer Thanks for the info! That's pretty good news 🙂

[mweststrate]

Other proposed changes:

• [ ] Decompose extras namespace for better tree shaking
• [ ] Drop data mutation events from spy. (Does anybody really use that? It seems so far mainly be used for the general flow; actions and reactions).

[mweststrate]

Improved plan:

FIrst split mobx into mobx and mobx-core packages. So that the future and current api are powered by the same package. Would also be great for people who hate the current mobx api with faux arrays, side-effectful getters etc etc. They still could leverage the algorithms behind mobx and build their own library on top of atoms etc.

[tkrotoff]

FYI Vue.js 3.0 (or Vue.js 2.6-next) will use ES6 Proxy: https://blog.cloudboost.io/reactivity-in-vue-js-2-vs-vue-js-3-dcdd0728dcdf

[urugator]

FIrst split mobx into mobx and mobx-core packages

I wanted to suggest something like that, but then I wasn't sure what exactly can be reused and whether some adjustments won't be required. So I thought it may be better to came up with proxy implementation first and then try to identify the common parts and split the package.

[ippa]

@farwayer @solkimicreb @mweststrate

i haven't tried it but the latest release of Expo ("pre-compiled react-native") claims to come with an updated Android JavaScriptCore including among others proxy-support ( https://blog.expo.io/expo-sdk-v22-0-0-is-now-available-7745bfe97fc6 ).

Updated the Android version of JSC to be closer to iOS 10.3’s JSC. Features like WeakMap can be used natively on Android and iOS. On Android and iOS 10+, more features like Proxies are supported.

If I've understood it correct, native proxy support will fix all of mobx quirks when it comes to observing and reacting to more complex data?

[capaj]

@strate yeah detecting an extra property on an observable should be a feature added to the current semver version of MobX. It would catch quite a few developer errors. Although there might be people who do this on purpose. Maybe do the check only in strict mode?

[mweststrate]

Proposal for a cleaned up observable API:

--- Current

@observable
@observable.ref
@observable.deep
@observable.shallow

observable.box 
observable.shallowBox
observable.object
observable.shallowObject
observable.array
observable.shallowArray
observable.map
observable.shallowMap

extendObservable
extendShallowObservable

@computed
@computed.struct
@computed.equals
@action
@action.bound

--- New

@ref // just stores an observable reference
@collection // make any collection it receives observable (1 level deep), objects, arrays, Maps, Sets. Also accepts `null` and `undefined`
@deepObservable // like current `observable`. makes anything deeply observable

ref(value, opts) // creates a boxed observable, with .get / .set
collection(value, opts) // creates an observable wrapper around collection (does not accept `null` / `undefined`)

extend(thing, props, opts) // like extendShallowObservable / extendObservable

@computed
@computed.struct
@computed.equals
@action
@action.bound

type opts = {
   deep? = false,
   name?: string
   // dunno what more...
}

[jamiewinder]

I can't say I'm keen on the renames. I was a big fan of this API change in MobX 3 as it described they describe the intent perfectly.

@ref implies reference, but nothing about its observable nature. Likewise for @collection. Having these things hang off of observable makes it obvious.

[stevefan1999-personal]

We might need mobx-compat after the observable => ref refactor

[phiresky]

I have to say I also like having all the different kinds of observable scoped in observable. Also your list is missing observable.struct?

In fact, why is computed not also scoped in observable? observable.computed would make sense :)

The thing I think would make sense to change is to prevent usage of observable() for both primitives and objects, because it's weird that observable({a: 1}) returns something that looks like {a: 1} but observable(1) creates an object with {get: () => 1, set: val => void}, I'd just forbid that.

It's also pretty confusing when starting out that observable(classInstance) creates an observable.ref instead of a deep observable.

Also I think all isMobxModifierDescriptor things work kind of confusing: observable.ref() is a completely different _kind of thing_ than observable.box (you must ref() within another observable, and the opposite for box), but only when _not used as a decorator_.

[phiresky]

I also think shallow is a far clearer name than collection. Renaming observable to deepObservable might be a good idea, not sure.

Here's how I would change the API:

observable(x) 
    -> deeply observe all properties of x. if x is a primitive, 
       null or a class instance then throw an error
@observable x = ... 
    -> deeply as above, but also allow what's basically a ref (primitives, null, class instances, etc.)


observable.shallow
   -> error, modifier are a separate API. Before, I would think
      observable.shallow({a: {}, b: {}}) would give me an object with
      observable references to a and b, but it doesn't.
@observable.shallow -> as before

observable.ref -> error, modifiers are separate API
@observable.ref -> as before

@observable.computed
get x() { ...}    -> as before

observable.computed(() => ...)  -> error, docs even say "This form of computed
                                   is not used very often"
observable.computed.boxed(() => ...)  -> `{get(), ...}` as computed() was before


// *very clearly* only do this kind of thing (completely modify the interface
   of "x") in the boxed method
boxedObservable(x) -> {get() => x, set(x) => void}
@boxedObservable -> error, no one needs this
// I also think it would be better to have this not be deep observable
// automatically, but I'm not sure.


* Modifiers


modifier.ref() -> as observable.ref() before
modifier.shallow() -> as observable.shallow before, but throw an error if any
                      primitives or null are passed. what does
                      observable.shallow("123") mean??
...

[urugator]

To me the proposal seems:

• inconsistent in naming: "deepObservable" VS "ref" and "collection" (why emphasize observable in one case, but not in another?)
• less descriptive: "collection" says nothing about observability... I understand it's collection() with default options, but it doesn't play well imo, especially in relation to deepObservable
• inconsistent in design: Why decorators favor explicitness while functional API favors configuration
  @deepObservable <=> collection(o, { deep: true })?

Counter proposal:
What about depth option (probably already discussed)? Depth seems quite self explanatory (?) and universally usable, so we could get rid of ref/shallow/deep/box:
depth: -1 (default) is deep
depth: 0 is for ref or box
depth: 1 is for shallow
depth: n shouldn't be too hard to support any arbitrary depth (?) alternatively we can treat n>1 as deep

The question is how to specify "depth" (or any option in general), so I would go for:

@observable() // brackets are mandatory
@observable({ depth: 1 })
@computed()
@computed({ equals: () => {} })

observable(o, { depth: 1, name: "name" })

/*
Dunno about extend... 
I am convinced that options object should be defined per property same as decorators.
However defining key + value + options in consistent way, without being too verbose is quite a challenge.  
So aside of current and a bit magical "descriptor/modifier" solution, there are some less magical, but more verbose alternatives:  
*/

// Object.defineProperty approach
defineProp(o, key, {
  value: val,
  depth: 1,  
})
defineProps(o, {
  key: {
    value: val,
    depth: 1
  }
})

// Helper for applying decorators
decorate(o, {
  key1: observable(), // value is taken from "o.key1" 
  key2: observable({ depth: 1 }), // value is taken from "o.key2"    
})
// we could allow "value/initialValue" option

EDIT: btw I also don't have much of a problem with current API

[spion]

Whats wrong with the current API? Its pretty good.

My paint of the bikeshed:

@observable.ref // only the reference
@observable.collection // converts refs + one level items
@observable // automatically converts assignments to deep observable
observable.(map|set|array|etc) // "shallow" collections (don't auto-convert contents)
observable(collection) // automatically converts objects/collection/subcollections, deeply

additionally, any existing tagged observables would stop deep conversion, since the consumed object knows better what should and shouldn't be (deeply) observable.

[mweststrate]

Thanks for the responses on the quick brainfart! I'll open a separate issue to discuss the above. But in short, the goal of the proposal is as follow:

1. Currently the default is deep observability, or the lazy approach
2. I notice that as a result, people tend to make to many things observable, and don't grasp the conceptual between observables references / properties, and values. This is a constant source of trouble / questions for beginners.
3. So I would like the API to encourage thinking about what actually should be observable. For example, @observable things = [] is an approach that always works, but hardly brings real understanding on how mobx works. However, when somebody uses @observable.ref things = [] _or_ readonly things = observable.array() this nicely captures the reasoning behind the state shape design.
4. Repurposing practically the same api might be very confusing during migration. Although probably removing the @observable shorthand could be enough?

[spion]

I think that rewriting https://mobx.js.org/getting-started.html for a new API proposal highlights all the problems with new proposals if there are any, very nicely :grinning:

[urugator]

Move asyncAction to mobx package, because it's a must and I don't want to import utils.
Maybe re-consider naming because there is nothing like "async action" (sort of contradiction in terms), only a series of sync actions interlaced by async operations. Maybe actions would be enought? Or sequence/series? Dunno, maybe I am just overthinking it.

[Machiaweliczny]

Maybe asyncAction can be named effects?

[mweststrate]

@urugator: Imlemented decorate https://github.com/mobxjs/mobx/pull/1321/commits/bbc31c868407ee6f3b2371d82a15551d0c23cd28

[urugator]

@mweststrate Not sure about the "class only" limitation. I mean it decorates the object, does it really need to have a constructor?

// OLOO
const Box = {
  sizes: [2],
}
decorate(Box, {
  sizes: observable
});
const box = Object.create(Box);

// or are there any issues with:
const box = decorate({
  sizes: [2],
}, {
  sizes: observable,
})

I think it would be handy to return decorated object (or class if we insist)

[mweststrate]

@urugator In principle that is possible. But at this point

const box = {
  @observable sizes: [2]
}

is also officially not supported (I think it works though). The reason that pattern isn't supported (yet) is that the decorators proposal currently in stage 3 is significantly different from the babel / ts transformations we now rely on. (It is much better) but for now I want to avoid extending the current decorator proposals.

That being said, I think the way of creating objects your are proposing is nicer then extendObservable with all the modifiers :) I'll add some unit tests and check how hard it is..

[mweststrate]

@urugator ok, that was easy: https://github.com/mobxjs/mobx/pull/1321/commits/2c88f3344573f7d0ce7657c69ba8491d036420d6

[urugator]

@urugator Hm, I thought that simply param type should change, so that user has to call decorate(Box.prototype, decorators).
It's impossible to decorate functions like this. It's not uncommon to use functions as objects in JS (static props, functors).
Having seperate decorateClass would be cleaner design wise I think (instead of type inference), but I don't see a problem in requiring object(prototype) from user.

export function decorate<T>(target: any, decorators: any) {
    invariant(isPlainObject(decorators), "Decorators should be a key value map")    
    for (let prop in decorators) {
        const decorator = decorators[prop]
        invariant(
            typeof decorator === "function",
            `Decorate: expected a decorator function for '${prop}'`
        )
        const descriptor = Object.getOwnPropertyDescriptor(target, prop)
        const newDescriptor = decorator(target, prop, descriptor)
        Object.defineProperty(target, prop, newDescriptor)
    }
    return target;
}

[mweststrate]

decorate feels like we don't need extendObservable alltogether. Or am I going to wild now :-P?

[urugator]

The main advantage of extendObservable (aside being slightly less verbose) is that everything (key, value, observability) is defined at one place, which makes it conceptually closer to decorators and therefore easier to maintain (you don't have to manage props at two places).
That's why I was wondering if we could add "initialValue" option to decorators:

const o = { 
  b,  
}
decorate(o, {
  a: observable({ value: "initial" }),
  b: observable({ value: "initial" }), // throw if b is already defined and "value" opt is passed? 
  c: observable(), // throw if c is not defined and "value" opt is not passed?
  // or maybe we don't need options object? so keep current syntax, but return "enhanced" decorator instead of "modifier"
  c: observable("initial"),
})

EDIT: Simply put, the idea is to create descriptor from passed value, when there is no descriptor and value is provided.

[mweststrate]

I don't think we can plumb even more overloads of observable into the
api and still give useful results or error messages :)

Op vr 23 feb. 2018 om 14:12 schreef urugator [e-mail]:

The main advantage of extendObservable (aside being slightly less
verbose) is that everything (key, value, observability) is defined at one
place, which makes it conceptually closer to decorators and therefore
easier to maintain (you don't have to manage props at two places).
That's why I was wondering if we could add "initialValue" option to
decorators:

const o = {
b,
}decorate(o, {
a: observable({ value: "initial" }),
b: observable({ value: "initial" }), // throw if b is already defined and "value" opt is passed?
// or maybe we don't need options object? so keep current syntax, but return "enhanced" decorator instead of "modifier"
c: observable("initial"),
})

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/1076#issuecomment-368005307, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhKdyO9ixeY1WTDyg5ZTN6V2xBMjBks5tXrkngaJpZM4ONAsS
.

[urugator]

Yea, I am just trying to find out whether it could be solved with "higher-order-decorator"

decorate(o, {  
  a: withValue("initial", observable),
  // the same
  b: (target, key) => observable(target, key, { value: "initial" })
})

function withValue(value, decorator) {  
  return function withValueDecorator(target, key) {        
     return decorator(target, key, { value }); 
  }
}

EDIT: If the above works, then removing extendObservable doesn't seem to be a problem (to me)...
EDIT: changed so that the original descriptor is always ignored

[urugator]

Or would it be weird to expose method on decorators? (computed does it already)

decorate(o, {  
  a: observable.with("initial"), 
})

[mweststrate]

@urugator I think the fact that decorate does not support initial values now is a feature (unless we kill extendObservable). The neat thing is that it kind of forces you to still do the field declarations in your classes (I think that will be the most important use case for decorators). That might sound as disadvantage at first, but the benefit is that you get proper type checking in Flow and Typescript. (Which is a problem with extendObservable in constructor which is currently the typical way to avoid decorators)

[mweststrate]

Released a first alpha of mobx4: [email protected]. Messy release notes can be found here: https://github.com/mobxjs/mobx/blob/mobx4/CHANGELOG.md

Note that the release is not complete yet (still some pending api updates, and will add some backward compatibility api's). Progress can be followed here: https://github.com/mobxjs/mobx/pull/1321

[urugator]

@mweststrate
If I would use typescript/transpiler I wouldn't need decorate in the first place don't you think? (in other words if you have to use decorate you most likely don't have an access to these "benefits", not even to field initializers!)
Really let me put decorate wherever I want, without making assumptions:

class Store {
  constructor() {
    decorate(this, {
      a: observable.with("a"); 
    })
  }
}

decorate(Store.prototype, {
  b: observable.with("b");
})

unless we kill extendObservable

I would probably kill right side modifiers altogether, even for observable ... if you want to modify behavior use decorate ... (and thats when initial value helper comes in handy)

[mweststrate]

Here is a nice sandbox to try the alpha: https://codesandbox.io/s/1oj3zzk0j7

[mweststrate]

I would probably kill right side modifiers altogether, even for observable ... if you want to modify behavior use decorate ... (and thats when initial value helper comes in handy)

That was what I was thinking about as well, but after a quick investigation many seem to be using it

The first usage of decorate in your example is something I would really like to discourage, it's goal is to be used on _types_ not on _instances_, to keep it semantically close to how decorators work, and will be working

[phiresky]

Question about the alpha: What is the difference between keys/values and Object.keys and Object.values ?

[mweststrate]

keys and values can be tracked and will react to future property
additions / removals on observable objects. See for example (
https://t.co/VdVwkK159S).

MobX5 immediately will obsolete this again (by leveraging Proxies). The
reason I added it is mainly if people need to migrate back from Mobx5
(cause, Internet Explorer), and this way MobX4 can offer feature parity.

Op di 27 feb. 2018 om 21:56 schreef phiresky [e-mail]:

Question about the alpha: What is the difference between keys/values and
Object.keys and Object.values ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/1076#issuecomment-369022859, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhFVEIFU7e0rQEi0W7DSBI_yXeYEvks5tZGvhgaJpZM4ONAsS
.

[urugator]

@mweststrate

quick investigation many seem to be using it

And what else should they be using? I obviously use it too (I don't use decorators btw), the discussion is about whether it could be replaced by decorate, which does exactly the same thing - makes existing things observable ... so that we don't have 2 ways to do the same thing.

discourage, it's goal is to be used on types not on instances

If this would be possible:

const o = { 
  @observable a: "";
}

would you still discourage:

decorate(o, {
  a: observable,
})

The decorators are not limited to classes by design. The @observable syntax is (currently).
It doesn't make sense to artifically introduce the same limitation for decorate function (or do I miss some technical limitation perhaps?).
Decorators work with objects (not with types/instances) - whether the object is used as a prototype or not is irrelevant from the perspective of decorator.

But primarily, we don't need a function for applying decorators. We need a function for making objects observable. The idea behind decorate is that we simply reuse decorators to do so ...
Or let me put it like this: Why would I want to use decorate instead of extendObservable?

Btw I realized there is no descriptor for property decorator, so that higher-order-decorator idea is wrong right? (or maybe there is for ESNext?)

[mweststrate]

@urugator let me take a step back and explain what me annoys me about the current api; it is that (extend)Observable have become as overloaded as jQuery and to all kind of juggling around with typings. Take the following examples:

Decorators

class A {
   @observable field = 3
   @observable.ref field2 = Buffer
   @computed get c1() { }
   @computed({opts}) get c2 {}
   @action.bound method() {}
}

These are concise, quite intuitive and straightforward transformations. There is nothing going on with the values and types (ok, not true for observable, but with proxies the types will be come consistent as well)

Decorate

class A {
   field = 3
   field2 = Buffer
   get c1() { }
   get c2 {}
   method() {}
}
decorate(A, {
   field: observable,
   field2: observable.ref,
   c1: computed
   c2: computed({ opts })
   method: action.bound 
})

Now this is still straightforward, has literally the same concepts, and is even pretty close to what your compiler would emit when supporting decorators. It attaches behavior to the fields, but does change their signatures.

Obviously the problem here is that the double map doesn't read nice. It's not concise anymore and a little error prone.

extendObservable / observable

const a = observable.object({
   // this is fine
   field: 3,

   // this is weird, observable.ref "packs" the Buffer into a special object, so that `object` recognizes it,
   // can unwrap the value, and also attaches the right behavior to 'field2'
   // the type checker has to be tricked into thinking `ref<T>(t: T): T`, while actual signature something like
   // `ref<T>(t: T): { value: T, "$$refWrapper": true }
   field2: observable.ref(Buffer),

   // this is fine as well
   get c1() { },

   // this again is weird, `computed` just creates a normal ComputedValue object
   // which, actually has a completely separate signature
   // but `object` detects than, and folds them into the target object
   // be aware of arrow functions though; they will have the wrong `this`
   c2: computed(function () { expr }), { opts })

   // seems ok-ish, because it doesn't mess with the types
   // but again the => trap!
   // but, action.bound again has to specially mark the create function, 
   // to signal that `object` it needs to unwrap, bind, and wrap the function again
   method: action.bound(function () { })
}

And maybe it is all not that bad as I think it is, but to me it feels that there are to many inference rules in extendObservable / object.

The typing problem is by the way probably fixable in the near future as TS is about to introduce mapped types.

So indeed, I agree that @observable should work on objects as well, my point was more or less: Ideally I would see all those decorators only operating on the definition of the object, not it's values. (indeed, not on the right hand side of assignments).

So ideally, but not sure it is realistic, I would get rid of extendObservable with it's many rules, but replace it with something that is not much more verbose

[mweststrate]

Could something like this work?

extendObservable({}, {
   field: 3
   field2: [observable.ref, Buffer],
   get c1() { },
   c2: [computed({ opts }), function() { }]
   method: [action.bound, function() { } ]
})

[jamiewinder]

Is the change for non-string keyed ES6-like observable maps (https://github.com/mobxjs/mobx/pull/800) making it into MobX 4? I think the only thing putting it off was that it was a breaking change (since adding a key of numeric 1 vs string '1' is now considered different). It seems it'd be a good segway to the proxies mode where presumably proxied ES6 maps will have this behaviour anyway.

[mweststrate]

Good one! Added it to the list

Op wo 28 feb. 2018 om 11:50 schreef Jamie Winder [e-mail]:

Is the change for non-string keyed ES6-like observable maps (#800
https://github.com/mobxjs/mobx/pull/800) making it into MobX 4? I think
the only thing putting it off was that it was a breaking change (since
adding a key of numeric 1 vs string '1' is now considered different). It
seems it'd be a good segway to the proxies mode where presumably proxied
ES6 maps will have this behaviour anyway.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/1076#issuecomment-369202017, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhMhaV0ir6AjikU9YyPO5_gxoAX91ks5tZS9pgaJpZM4ONAsS
.

[urugator]

Could something like this work?

I really don't get it. Why extendObservable needs a different way of "decorating" (array, current modifiers, whatever)? Why extendObservable can't use normal decorators as well? Then it just makes sense to rename it to decorate, because all it does is applying decorators ... thats the whole idea all along ...
To provide initial value (or any settings in general) you just need a parametrized decorator (decorator factory) ...

// weird juggling
extendObservable({}, {
  a: 5,
  b: observable.ref(Buffer),
})

// key: decorator
// initial value must be defined on object   
extendObservable({ a: 5, b: Buffer }, {
  a: observable,
  b: observable.ref, 
})

// key: decorator (created by factory)
// initial value passed to decorator factory
extendObservable({}, {
  a: observable.with(5) // returns good old decorator! 
  b: observable.ref.with(Buffer) // returns good old decorator! 
})

// so if it just applies decorators to given object...
decorate(object, decorators); // object (no class)! 

If you keep extendObservable and just introduce decorate then nothing has been solved ... the weird juggling still exist and there is just a new method (decorate), which does the same thing as extendObservable, but is limited to classes, more verbose, less expressive, more error prone and harder to maintain ... so again why should I use it instead?

[mweststrate]

I like the second example the best as it is straight forward, but I think the trickyness is the need to double declare every field (or maybe only the non observable ones? that would make it a lot more bareable)

The third example is not really an improvements, as it keeps the same type juggling (ref.with(Buffer) doesn't return a Buffer) and has again the decorator working on the values, not on the properties. Basically the same as we already have (currently you would write observable.ref(Buffer))? Maybe what we have now isn't that bad at all, but if we can approve, now is probably the time :)

But unifying decorate / extendObservable like in the second example is an interesting direction. It would turn the earlier example into:

decorate({
   field: 3,
   field2: Buffer,
   get c1() { },
   get c2() { expr },
   method() {}
}, {
   // no need to decorate 'field', observable is the default
   field2: observable.ref, // needs decorating 
   // no need to decorate 'c1', getters are assumed to be computed
   c2: computed({ opts }), // computed decorator with option
   method: action.bound // method fields need decoration to turn them into actions (otherwise they are assumed to be views)
})

The nice thing about this is that no values appear in the second argument, which keeps it very consistent with decorators that also don't work on the actual values (decorating is done before field initialization), and it indeed shouldn't make any difference whether the first argument is an object, class, etc etc, semantics would always be the same. Biggest downside is see is that declaring a lot of actions is verbose, the other variations are probably rare enough to justify needing to be 'declared twice'

(Type-checking note to self: every key in decorators arg must be in target arg, except for observable(.X) when targeting classes, because with classes the field doesn't exist before the constructor has run)

[urugator]

the decorator working on the values, not on the properties.

Decorators work on properties by definition. It simply accepts options, exactly the same way as computed ... computed.equals(fn) doesn't return fn ... does it mean it works on values instead of properties? nonsense...

Don't allow to pass anything, but decorator, to decorate. Only function which accept values (but not decorators) should be observable:

decorate(observable({
  a: 5,
  field: Buffer,  
}), {
  field: observable.ref,
})

You may preserve extendObservable, which works like observable (only values, no decorators), but "extends" existing object:

extendObservable(existing, {
  a: 5,
  field: observable.ref // NOPE! it stores decorator to field as value, use decorate instead
})

[mweststrate]

the decorator working on the values, not on the properties.

to clarify: what I mean is that if all fields are declared in the target argument, the type can be inferred correctly from that. However, once values are declared in the second argument, the second argument is required to infer the correct type, and to even achieve that the type system must be tricked into the decorator not returning a descriptor, e.g.:

// a.x can be easily inferred to a number
const a = decorate({ x: 1 }, { x: observable.ref }) 

// this api would need to check the second argument for type inference, 
// *and* make `.with` return the type of it's value, instead of the decorator with options it actually returns
const b = decorate({}, { x: observable.ref.with(1) }) 

[urugator]

a.x can be easily inferred to a number

Is it useful for people who don't use typescript/transpiler(no type checking), therefore don't have an access to decorators, therefore are the ones using decorate?

Is it doable in way that I can trade verbosity for type checking? So if the value is in the first arg, I have more verbose code, but strict typing...?

[mweststrate]

Is it useful for people who don't use typescript/transpiler(no type checking), therefore don't have an access to decorators, therefore are the ones using decorate?

No, that set of people is disjunct. Many people use flow without decorators, or typescript implicitly without decorators (when using VS code in a plain javascript project it will still use the typescript compiler to infer type information)

Is it doable in way that I can trade verbosity for type checking? So if the value is in the first arg, I have more verbose code, but strict typing...?

Yes that would be possible. But I am wondering, is the verbosity that bad? The only case where one _must_ double declare the fields, is with observable.X and computed.X.

For actions that is not strictly necessary, because extendObservable({}, { x: action(fn) }) yields type & semantically the same as decorate({ x() {} }, { x: action })). (action is more about the value itself than the property that owns it; action properties are not writable)

---

In other words, I think we could largely follow your proposal in unifying extendObservable and decorate, even without .with. extendObservable could roughly become:

extendObservable(target, values, decorators?) {
   // infer decorators from values
   for (key in values) {
      if (!(key in decorators)) {
         if (hasGetter(values, key))
            decorators[key] = computed
         if (!isFunction(values[key]))
            decorators[key] = observable
      }
   }
   // decorate!
   return decorate(Object.assign(target, values), decorators)
}

Advantages: type safe decorate & extendObservable. observable.ref etc could become purely decorators, not factories that produce a decorator application description when invoked on a value rather than a property

[urugator]

I like it, some thoughts:

• It won't work on prototypes (because values would be assigned to prototype as static prop, not lazily to instance in decorator ...)
• Could we use Object.getOwnPropertyNames/Descriptors instead of for-in? So that following is possible:

extendObservable(this, this, { // getters are not enumerable in classes
  ref: observable.ref        
  method: action,
})
// EDIT: ahh, it wont' work ...these're not own, but on prototype, right?

• observable should also accept decorators (since there are no modifiers...). I quess there is overloading problem ... so maybe move decorators to mobx.decorators ... or ... is @observable still supported or dropped in favor of .deep/.ref?
  `javascript observable({ b: Buffer, }, { b: decorators.ref // or decorators.observableRef ? })

[mweststrate]

It won't work on prototypes (because values would be assigned to prototype as static prop, not lazily to instance in decorator ...)

Decorate would work fine afaik? extendObservable not, but I think that is fine?

Could we use

Sure, this is just rough draft code :)

I quess there is overloading problem

I don't think so, when used as decorator, the second arg is always string, clearly distinguishable from
the decorator map

[urugator]

Decorate would work fine afaik? extendObservable not, but I think that is fine?

It's not like that extendObservable does't work, but decorate is simply
extendObservable(clazz.protoype, {}, decorators); do we need a special method for that?
Also I have been thinking that people may actually prefer different, perhaps more friendlier, syntax, like:

// similar to mobx-react's inject
decorate({ 
  a: observable,
  method: action,  
})(class Store {
  constructor() {
    this.a = 5;
  }
  method() {}
});

So maybe we should leave the actual decorate implementation up to users and only provide some hints in docs?
(Actually I think that "the sanest" solution is class/model factory similar to one in mobx-state-tree, but I would leave that to users as well...)

I don't think so

Then that's a good news :) Hopefully it's also compatible with new decorator proposal(s) ...

Can we shorten extendObservable to just extend or is it too ambiguous?

[mweststrate]

@urugator see https://github.com/mobxjs/mobx/pull/1365 for the impact of the new api (especially the tests section)

[mweststrate]

Hi all, if you want to experiment with the new Mobx 4 api, you can try it now!

Just install [email protected]

The changelog can be found here

Give it a try! Let me know how much the impact is of the api changes :).

You might note that this version is slower then mobx3, but that is just a side effect of the recent api refactoring, mobx4 will be faster then mobx 3 (alpha.1 is representative performance wise)

[pkieltyka]

@mweststrate very exciting! Btw, as a TypeScript user myself I have a quick question: will the "experimentalDecorators" offered in TS still work with mobx4 in the convenient form @observable field = 3 ?

[mweststrate]

Yes!

Op do 1 mrt. 2018 15:39 schreef Peter Kieltyka [e-mail]:

@mweststrate https://github.com/mweststrate very exciting! Btw, as a
TypeScript user myself I have a quick question: will the
"experimentalDecorators" offered in TS still work with mobx4 in the
convenient form @observable field = 3 ?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/1076#issuecomment-369612073, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhAr7wxtUUosn7Q1vhCp_Kvyb__CQks5taAgsgaJpZM4ONAsS
.

[urugator]

About computeds:

• remove compareStructural option, it's redundant (2 two ways to do the same thing, complicates computed impl, unnecessary dependency)
• imho there is a diference between: deep and structural equality...
  Deep means that it traverses traversable values (object/array/set/map), but uses standard (identity) comparator for leafs (non-traversable-values).
  While structural equality should try to convert everything to it's value representation first and then traverse the value if it's traversable.
  Standard way to get the value representation in JS is to call valueOf. It returns identity if not implemented - meaning that the object itself (the reference) is the value:

// Buil-in value types
console.log(new String("hello").valueOf() === "hello"); // true
console.log(new Number(5).valueOf() === 5); // true (NaN requires a special treatment)
console.log(new Date(1519916831621).valueOf() === new Date(1519916831621).valueOf()) // true
console.log(new Date(1519916831621).valueOf() === 1519916831621); // true
// Arbitrary value types
const arbitraryValueLikeObject1 = Mobx.observable.box("a");
const arbitraryValueLikeObject2 = Mobx.observable.box("a");
console.log(arbitraryValueLikeObject1.valueOf() === arbitraryValueLikeObject2.valueOf()); // true

So what I propose:

• distinct between deep and structural
• adjust naming accordingly
• adjust logic of structural comparator to rely on valueOf to (perhaps) simplify implementation and to support arbitrary value-like objects (box,Point,Vector,...)
• adjust logic of deep comparator to always use identity comparator on non-traversables

[urugator]

Not sure if it's just temporal, but I've noticed we're accessing process.env.NODE_ENV on multiple places. This has according to react's creators huge impact on performance when running on server, please change it to single call...

Unfortunately, it turns out that process.env is not a normal JavaScript object, and it is quite expensive to get a value out of it. So even when the value of NODE_ENV is set to production, just checking the environment variable so frequently adds a significant amount of time to server rendering.

[mweststrate]

remove compareStructural option

agreed

imho there is a diference between: deep and structural equality...

I don't think in mobx we don't really need to make this distinction (this can always be done in a custom equals function), because the only case where the difference would actually be different, is where an non-plain object implements a valueOf method.

Did you note that 3.6.1 / 4.0.0 had a revision of the deep-equality mechanism because it contained a bug? It now follows the underscore implementation. This means that even non-primitive objects are traversed, and things like Point & Vector should work fine as well

[mweststrate]

For the NODE_ENV: the point of it is that it is compiled away during minification (see the minified build in your node_modules). As soon as a variable is introduced to capture that value, the uglifier won't remove the code anymore. So the process.env will only be checked in a development (or in an wrongly set up product, but we can detect that)

[urugator]

This means that even non-primitive objects are traversed, and things like Point & Vector should work fine as well

Unless their internal structure is different from what their valueOf returns...
If you don't want to go this route I would at least suggest to rename it to deep ... it's shorter, it uses deepEq function, works like _.deep ... so why structural?

So the process.env will only be checked in a development (or in an wrongly set up product, but we can detect that)

I am probably missing something ... when I require("mobx") on the server (there is never any compilation step) it always uses normal non-minifed version (mobx.js) right? And this version contains these env reads or not...?

[mweststrate]

... to rename it to deep

agreed, will do

on the server

the main concern is a smaller build, not performance. When running the performance test suite on the server with and without the process.env.NODE_ENV substitution, there was no measurable difference in the speed, so if this was a problem, it seems solved by now (or mobx doesn't have enough checks yet ;-)).

[mweststrate]

Mobx 4 beta is now available! npm install [email protected]. Migration guide: https://github.com/mobxjs/mobx/wiki/Migrating-from-mobx-3-to-mobx-4

[urugator]

...so it's stil structural... :)

[mweststrate]

@urugator correct, I started renaming, but then it got weird because that would involving renaming computed.struct to computed.deep which makes it less clear what it does..

[urugator]

I thought that computed.struct is also removed ...same as computed.equals...

[mweststrate]

No, @computed.struct is the same as @computed.options({ equals: comparer.structural}). @computed.equals has been removed in favor of
.options. But struct is such a common pattern that I think it deserves
a short hand :-). The goal of the API is not to just be as small as
possible, but to make the 80% use cases as easy as possible, and make the
remaining 20% rest possible with an api that is as small as possible.

Op di 6 mrt. 2018 om 11:18 schreef urugator [e-mail]:

I thought that computed.struct is also removed ...same as computed.equals
...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/mobxjs/mobx/issues/1076#issuecomment-370733322, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABvGhGacoqvYZI-HR6acDa6LeNU97py_ks5tbmJkgaJpZM4ONAsS
.

[urugator]

I see, I thought it's @computed(options), you have a mistake in migration guide I believe, check the new API for @computed.equals(compareFn) :)

[mweststrate]

Released:

[email protected]
[email protected]
[email protected]

Which together fix some build & typescript issues :)

[mweststrate]

released [email protected] (fixing #1376)

[mweststrate]

Released 4.0.0!

[marvinhagemeister]

@mwestrate awesome! Congrats for the ride! Really happy with Mobx :)