Mobx: observableArray toString method seems fishy

[capaj]

I was debugging a bug in our app, where calling encodeUriComponent() on an observableArray produced very weird results. Turns out toString method is returning something else than the ecmascript array:

> var mobx=require('mobx')
undefined
> a = mobx.observable([])
ObservableArray {}
> encodeURIComponent(a)
'%5Bmobx.array%5D%20'
> encodeURIComponent([])
''
> a.toString()
'[mobx.array] '
> [].toString()
''
> [1].toString()
'1'
> a.push(1)
1
> a.toString()
'[mobx.array] 1'

@mweststrate/anyone why is it like this? Could we maybe match the behaviour in regular array? That would certainly fix any issues people might have when they try to encode their observableArrays into query strings.

EDIT: Looking into the source https://github.com/mobxjs/mobx/blob/2cc98ae92c56ed8f8371875436aba75657a8bab1/src/types/observablearray.ts#L383 it is indeed done like this. What is the reasoning for explicitly not matching the behaviour of ecmascript array here?

[capaj]

based on your tweet @mweststrate https://twitter.com/mweststrate/status/818785507207507968 I presume I can start working on a PR.

[mweststrate]

@capaj Yes :-D

[capaj]

PR opened: #745

[capaj]

I was thinking I'd try ObservableMap's toString as well and lo behold it has it's own special implementation which behaves different than ecma Map:

> m = new Map()
Map {}
> m.toString()
'[object Map]'
> m.set(1, 'aaa')
Map { 1 => 'aaa' }
> m.toString()
'[object Map]'
> m = new mob.ObservableMap()
ObservableMap {
  enhancer: [Function: deepEnhancer],
  name: 'ObservableMap@2',
  '$mobx': {},
  _data: {},
  _hasMap: {},
  _keys: ObservableArray {},
  interceptors: null,
  changeListeners: null }
> m.toString()
'ObservableMap@2[{  }]'
> m.set(1, 'aaa')
ObservableMap {
  enhancer: [Function: deepEnhancer],
  name: 'ObservableMap@2',
  '$mobx': {},
  _data: 
   { '1': 
      ObservableValue {
        name: '[email protected]',
        isPendingUnobservation: true,
        observers: [],
        observersIndexes: {},
        diffValue: 0,
        lastAccessedBy: 0,
        lowestObserverState: -1,
        enhancer: [Function: deepEnhancer],
        hasUnreportedChange: false,
        value: 'aaa' } },
  _hasMap: 
   { '1': 
      ObservableValue {
        name: '[email protected]?',
        isPendingUnobservation: true,
        observers: [],
        observersIndexes: {},
        diffValue: 0,
        lastAccessedBy: 0,
        lowestObserverState: -1,
        enhancer: [Function: referenceEnhancer],
        hasUnreportedChange: false,
        value: true } },
  _keys: ObservableArray {},
  interceptors: null,
  changeListeners: null }
> m.toString()
'ObservableMap@2[{ 1: aaa }]'

I think it would make sense to align that one as well, because it could bite people who would have existing codebase with ecma Map and then they'd try to replace it with ObservableMap. On the other hand I don't see where it's useful to print out keys-when I'd want to do that, I'd use just toJS from Mobx.

Also I noticed original ecma Map doesn't have toJSON() method whereas ObservableMap does have it. In this case I'd make an exception and I'd leave it there. Stringifying ecma Map is just too cumbersome without .toJSON: http://www.2ality.com/2015/08/es6-map-json.html