Unintended behavior when creating new models with falsy first argument

[coolsoftwaretyler]

Unintended behavior when creating new models with falsy first argument

[coolsoftwaretyler]

Hey @nithinssabu - thanks for your interest! That's great!

I'm gonna go ahead and assign this issue to you for now.

To begin, do you want to read through the issue, take a look at the original PR, and comment back with your understanding of the problem/how you want to approach it?

From there, we can get you started on a PR.

If you're feeling particularly industrious, I'd also be happy to start with a PR that has some test cases to demo your intended solution.

Let me know any way I can be helpful. I'm gonna be a little busy until Friday, but will try to be available for set up questions.

[coolsoftwaretyler]

Hey @nithinssabu - just about right, although after thinking about this a bit more, I think we should make this change backwards compatible by:

1. Keeping the existing test case intact and passing - for production specific environments
2. Adding a new test that throws an exception only when not in production. Here is an example where we test that specific environmental behavior.

If we do that, we should be able to include this in the next minor release, rather than waiting for a breaking change. I think very strictly speaking, this will break MST in some development builds, but keeping the new exception out of production will support MST v5 overall.

[coolsoftwaretyler]

If anyone is interested in grabbing this, I would be happy to discuss different approaches, help you get set up in the repo, or whatever else would make this feel accessible for first time contributors.

[nithinssabu]

I am using Mobx daily at work, but not MST. I am an experienced developer but have not used Typescript yet.

I am interested in picking this up if my experience level mentioned above is not a blocker.

[nithinssabu]

I'm gonna go ahead and assign this issue to you for now.

Thank you for assigning me. I'm so excited about this.

So, here's my understanding of the problem:

Currently when we provide an invalid name value, the model will be created with the name AnonymousModel and the properties will be empty (even if provided). This can cause confusing behaviour, especially to non-TS users.

We want to throw an exception in this case instead of creating a valid model.

I would approach it this way:

• Modify this test case to assert that an exception is thrown
• Fix the behaviour to match this assertion

[nithinssabu]

Hi @coolsoftwaretyler Please check the linked PR and see if it meets the requirements. Also, please suggest a better error message if the one I used is not good enough.

[coolsoftwaretyler]

Thanks, @nithinssabu! I will take a look today or this weekend. Appreciate your time!

[nithinssabu]

@coolsoftwaretyler Thanks for the review and merging the PR. It would be great if you can assign to me similar smaller issues that are not urgent. I would like to actively contribute.