Throw/warn/snapshot-itize instances passed as argument to types.optional

[xaviergonz]

I have:

• A conceptual question.
  • I've checked documentation and searched for existing issues
  • I tried the spectrum channel first
• I think something is not working as it should.
  • I've checked documentation and searched for existing issues
  • I've made sure your project is based on the latest MST version
  • Fork this code sandbox or another minimal reproduction.
  • Describe expected behavior
  • Describe observed behavior
• Feature request
  • Describe the feature and why you feel your feature needs to be generically solved inside MST
  • Are you willing to (attempt) a PR?

Not following the above template might result in your issue being closed without further notice

Today a coworker was telling me that it took them a while to find a bug related to passing an instance as argument to types.optional, since as soon as a second node of the same type was created it would crash because it was attached to the first node parent.

const someDefault = TModel1.create({...})

const TModel2 = types.create({ m1: types.optional(TModel1, someDefault) })
const a = TModel2.create({}) // ok
const b = TModel2.create({}) // boom!

My question is, should that particular case be prevented?
If so, what would the best course of action?

• A. snapshot-ize the instance - mostly non-breaking, but might loose the environment (not a big deal, since anyway children and parents have to have the same environment anyway)
• B. throw an error - breaking-ish change though?
• C. (my least favorite) just show a console warning

[mweststrate]

B sounds the most clear to me Op vr 31 aug. 2018 22:31 schreef Javier Gonzalez <notifications@github.com>:

…

I have: - A conceptual question. - I've checked documentation and searched for existing issues - I tried the spectrum channel <https://spectrum.chat/?t=dad48299-3dfc-4e10-b6da-9af1e39498a3> first - I think something is not working as it should. - I've checked documentation and searched for existing issues - I've made sure your project is based on the latest MST version - Fork this <https://codesandbox.io/s/jlnmjqr7xv> code sandbox or another minimal reproduction. - Describe expected behavior - Describe observed behavior - Feature request - Describe the feature and why you feel your feature needs to be generically solved inside MST - Are you willing to (attempt) a PR? *Not following the above template might result in your issue being closed without further notice* Today a coworker was telling me that it took them a while to find a bug related to passing an instance as argument to types.optional, since as soon as a second node of the same type was created it would crash because it was attached to the first node parent. My question is, should that particular case be prevented? If so, what would the best course of action? A. snapshot-ize the instance - mostly non-breaking, but might loose the environment (not a big deal, since anyway children and parents have to have the same environment anyway) B. throw an error C. (my least favorite) just show a console warning — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1002>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvGhPi400ZySvHerrIs7b8gMKuPZ5Jmks5uWZ0HgaJpZM4WV0WI> .

[xaviergonz]

@mweststrate upon further investigation, it seems like the current behavior is A, just that it failed sometimes when the instance was created from a function rather than passed as a direct parameter

I fixed what made it fail in #1003
I added there some thoughts there as well, with a question on how to proceed

[xaviergonz]

Released in v3.3.0