mobx-state-tree icon indicating copy to clipboard operation
mobx-state-tree copied to clipboard
verified publisher icon mobxjs

Symbol as model key

Open Yuu6883 opened this issue 3 years ago • 4 comments

Feature request

Is your feature request related to a problem? Please describe. Trying to replace some large nested mobx observable objects with MST model, but unable to do so because MST model ignores Symbol keys when mobx itself supports observable Symbol keys.

Describe the solution you'd like Symbol as key in a model, by possibly replacing Object.keys in https://github.com/mobxjs/mobx-state-tree/blob/master/packages/mobx-state-tree/src/types/complex-types/model.ts with Reflect.ownKeys

Describe alternatives you've considered Replacing all occurrences of symbol with plain strings in my code, but need to make addition string maps because I'm also using Symbol to store a description string field in it.

Additional context

Are you willing to (attempt) a PR?

  • [ ] Yes
  • [x] No

Yuu6883 avatar Jul 27 '22 04:07 Yuu6883

Hey @Yuu6883 - thanks for filing this issue, and I'm sorry it took so long for us to get back to you.

I think this is an interesting idea, but I'm not sure when or how it'll fall on the priorities list. I'm going to label it as an enhancement, open for contribution. Since you said you weren't interested in a PR, I'd be happy to chat with anyone who comes across this and wants to give it a go.

coolsoftwaretyler avatar Jun 25 '23 21:06 coolsoftwaretyler

Hey @coolsoftwaretyler I would be happy to do this issue, It would be my first contribution to mobx-state-tree. Can you please tell me some more detail about this issue

singularvoice avatar Sep 01 '23 09:09 singularvoice

Hi @a-hassanzadeh-h - thanks for your interest in contributing (and for your other PR!).

I think to get started, I would do a few things:

  1. Write some tests that use symbols as keys on a model. Verify that they fail as expected. You might write those in packages/mobx-state-tree/__tests__/core/model.test.ts
  2. Take a look at the code that the author of this issue linked, and see if making changes there gets the tests to pass.
  3. Consider any other implications of your change, check the test suite, add tests for edge cases that might come up.

Let me know if you have specific questions once you dig in. We can troubleshoot here as needed. FYI, I am a bit busy in the coming weeks. Apologies for delays on response.

coolsoftwaretyler avatar Sep 02 '23 18:09 coolsoftwaretyler

Hey @coolsoftwaretyler, Sure. Okay, I am going to work on it, I will keep you updated if I have questions.

singularvoice avatar Sep 03 '23 06:09 singularvoice