Can't use symbol keys in ObservableMap

[pimterry]

Any use of symbols as a key in an observable map throws an exception, complaining because symbols can't be implicitly convert into strings.

• Error messages including stacktrace:

   Cannot convert a Symbol value to a string
       at ObservableMap$$1../node_modules/mobx/lib/mobx.module.js.ObservableMap$$1._updateHasMapEntry (app.js:113461)
       at ObservableMap$$1../node_modules/mobx/lib/mobx.module.js.ObservableMap$$1.has (app.js:113392)
       at ObservableMap$$1../node_modules/mobx/lib/mobx.module.js.ObservableMap$$1.get (app.js:113518)
  

• Provide a minimal sample reproduction

   import { observable } from "mobx";
   
   const map = observable.map();
   map.get(Symbol('test'))

• Did you check this issue wasn't filed before?
  I don't think this has been filed before, but it is a bit related to Introduce support for Symbol named observable properties #1809 (but I'm interested in keys in ES6 maps specifically, not object property keys which I suspect are much more complicated)

• Elaborate on your issue. What behavior did you expect?
  I'd like to be able to use Symbols as map keys, and I don't think that's an especially uncommon thing to do.

  I think the main reason this fails is the use of the key in string templates, because `${Symbol('test')}` always throws the error above. It would work with `${Symbol('test').toString()}` instead. I haven't tested that though, so there may well be other issues here as well.

• State the versions of MobX and relevant libraries. Which browser / node / ... version?
  MobX 5.6.0, Chrome 70

[mweststrate]

It would work with ${Symbol('test').toString()} instead.

If implicit conversion is explicitly forbidden in the language, it sounds like the last thing we should do is implicitly convert symbols to strings :) And if you can do that, you can just do that in the consuming side?

So far this has been the first request for this in a couple of years. curious about your use case :)

[pimterry]

I'd argue its replacing a implicit conversion for an explicit one. I think implicit is discouraged because there's many places where you can use both symbols and strings, and accidental conversion would allow collisions. Here there's no such issue - as far as I can tell, these strings are used only as the names of observables for debugging.

My use case is more or less the exact one Symbols are designed for: totally unique keys. I have a map shared throughout much of my application (it's effectively a shared cache), and I want many different parts of the codebase to independently store data in that one map with a guarantee they'll never conflict. To do that, I just make them each define their own symbols and use those as their keys, and that guarantees they'll never accidentally see/write over each other's data.

[mweststrate]

The problem is that with implicit conversion to strings, you could just as well not use Symbols at all, as demonstrated here:

> Symbol("test") === Symbol("test")
false
> Symbol("test").toString() === Symbol("test").toString()
true
> 

So to fix this, observable map should support storing Symbolic keys, not just string ones. But looking at the code, that is already supported, the problem is only the debug names as far as I can see. Feel free to PR some unit tests, or a solution if you feel up to it :). A safeStringify(key) at the right places is probably enough to fix the issue.

[pimterry]

Ok, great! Yes that's what I was suggesting - not changing how the actual keys are stored, which should indeed work fine with symbols in theory afaict - just fixing any nearby assumptions about stringiness.

I'll try and find some time soon to open a PR, thanks.

[mweststrate]

Great! Sorry, misunderstood the issue initially :)

[mweststrate]

Released as 4.9.4 / 5.9.4

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.